fix for MakeCredential Response

[nob3rise]

Add some fixes for MakeCredential Response test in Conformance Tool test.

[lyokato]

ありがとうございます。

ざっと確認しましたが、効率化などに関する部分は問題ないかと思いました。

しかし、プロトコル部分に関する修正は以下の2点かと思いますが、意図が読めないところがありました。プロトコルに関する修正は、W3C仕様書のどの記述に基づいての修正なのか参照先を頂きたいです。あるいは他のライブラリとのInterOpで失敗する場合はどういった失敗が発生するのか具体的な情報を頂きたいです。

AttestationObjectのフォーマット変更

WebAuthnKit-iOS/WebAuthnKit/Sources/Authenticator/Attestation.swift

Lines 57 to 60 in 8301b13

	let dict = SimpleOrderedDictionary<Int>()
	dict.addString(0x01, "packed")
	dict.addBytes(0x02, self.authData.toBytes())
	dict.addStringKeyMap(0x03, self.attStmt)

CBORのMapフォーマットの部分で、キーに当たるStringをIntに変更しているようですが、このIntの各数値はどの仕様を参照されたものでしょうか?

自分も改めてこのあたりを確認してみましたが、キーは文字列で合っているように見えますし、
https://www.w3.org/TR/webauthn/#sctn-attestation

Yubicoのライブラリなども確認してみましたが、やはり文字列がキーになっています。
https://github.com/Yubico/python-fido2/blob/master/test/test_ctap2.py#L166-L168

makeCredentialのパラメータ修正

WebAuthnKit-iOS/WebAuthnKit/Sources/Client/Operation/ClientCreateOperation.swift

Line 256 in 8301b13

hash: self.options.challenge,

hash計算した値ではなく、サーバーから受け取ったchallenge値をそのまま渡すように修正されていますが、こちらもどの情報を参照されての修正なのかを明示頂きたいです。

こちらも仕様を確認いたしましたが、
https://www.w3.org/TR/webauthn/#op-make-cred

こちらのパラメタータに

hash
The hash of the serialized client data, provided by the client.

と記述され、"serialized client data"のリンク先の説明には

Hash of the serialized client data
This is the hash (computed using SHA-256) of the JSON-serialized client data, as constructed by the client.

と記述されていますし、サーバーから受け取ったchallengeではなく、JSONを作ったあとhashかけたものを渡すという仕様で間違いないように見えます。

また、同様にYubico実装など他のライブラリを追っても、同様の処理をしていますし、ここを変更してしまうと多くのサーバー実装との疎通が失敗してしまいます

https://github.com/Yubico/python-fido2/blob/7376b989a4008955cdd11926b3c22279c7b5481d/fido2/client.py#L460

---

というわけで、以下確認いただきたく

• プロトコル部分の修正2点はあらためて確認お願いしたいです
• 自分の方が見落としているところがあるかもしれませんが、その場合、前述のように修正の際に参照された仕様の具体的な箇所を明記いただきたいです
• 確認されたのはConfirmation Toolでの失敗だけでしょうか?
• 修正されたコードで、何かサーバーサイド実装とのInterOpなどは確認済でしょうか
• された場合は、サーバーサイドのwebauthnの処理はどのライブラリを利用されたかなど詳しく教えて頂きたいです

Confirmation Toolでの確認は、自分もさぼっていたところですので、あらためて準備して確認してみようと思います。少し時間はかかるかもしれませんが。

[nob3rise]

レビューありがとうございます。

プロトコルの変更についての情報が不足しており申し訳ありません。

AttestationObjectのフォーマット変更

こちらについてはFIDOのCTAPの仕様書の情報を参考にしています。

上記の仕様書の5.1 makeCredentialにレスポンスデータの詳細を示す表があります。この中にMember nameという項目があり、それぞれの名前の下にインデックスの数値が書かれています。

また、仕様書の6.2 ResponseのEXAMPLE 6にmakeCredentialのレスポンスデータ例が載っているのですが、こちらでもキーがString値ではなくInt値になっていることが確認できます。

ご紹介頂いたpython-fido2のソース中でもキー名はInt値で定義されており、toStringしたときにはString値で返してくれるようです。

makeCredentialのパラメータ修正

こちらもFIDOのCTAPの仕様書の情報を参考にしています。

仕様書の5.1 makeCredentialのリクエスト値を表で示している箇所でclientDataHashのDefinitionがさらっと次のように説明してあります。

Hash of the ClientData contextual binding specified by host. See [WebAuthn].

当初私もJSONを自分で作ってSHAハッシュを計算するのだと思っていましたが、何度やっても上手くいかず、仕様書を読み返して見たところ上記の記述を見つけて、実装してみたところ上手く行きました。

尚、これらの動作確認はConformance Toolでのみ行っており、実際のWebAuthnサーバーとクライアントでの動作確認はできていません。
ただ、Conformance ToolでmakeCredentialのテストケースをパスするためにはこの２箇所の修正が必要だったことと、仕様書上の記述より今回の修正が正なのではないかと考えています。

[lyokato]

情報ありがとうございます！
状況よく理解できました。ご指摘のポイントも非常にわかりやすかったです。

自分の方でもあとで該当箇所、読み込んでみたいと思いますが、
とりあえずFIDO2 CTAP側の仕様ということですね。

ちなみに現時点で自分がConfirmationToolについての知見がないので申し訳ありませんが、
実行されたテストというのはAuthenticatorに該当する部分のみのテストでしょうか?

本来WebAuthnのフローは

Server <-> Client <-> Authenticator

の三者間のフローですが、このSwiftライブラリはInternalなAuthenticatorのみ現在対応していて、
Client部分とAuthenticatorの２つの両方のコンポーネントを内包しています。

ConfirmationToolによる該当のテストがどのように行われたかを確認したいと考えています

1. Authenticator部分のみに対してのvalidationなのか
2. Client部分も含めての振舞いに対するvalidationをしてくれるのか

CTAPの仕様はClientとAuthenticator間の通信仕様ですので、
「Client <-> Authenticatorの通信データはご指摘のように修正する必要がある、ただしAuthenticatorからの情報を受け取ったClientがサーバーにデータを渡す際に、W3C側の仕様に基づいてなんらかの変換を行う」という必要があるかないかを調査したいためです。

また、既存のサーバー実装とのInterOpもあらためてしたいと思います。
自分も1年前に既存のサーバー側ライブラリいくつかとInterOpをして、問題なかったので現在の実装にしてありました。しかし当時はConfirmationToolに対応したライブラリが存在しませんでした。
現在ですとwebauthn4jなど公式のテスト通過済みのライブラリもいくつかあると思いますので、
あらためて確認しようと思います。

---

というわけで以下のように進めたいと思います

• 自分の方でもご指摘のCTAPの仕様を読み込んでみます
• ConfirmationToolをこちらでも準備して確認してみます
• 公式certifiedなサーバーサイドライブラリとのInterOpを確認します

これで問題なさそうでしたら、いただいた修正をそのまま適用したいと思います。

どこかで引っかかったら、また相談/確認させてください。

ご指摘ありがとうございました！

[nob3rise]

ご確認ありがとうございます。
説明不足で申し訳ありません、お察しの通り私が進めていたのはClientとAuthenticator間の通信になります。

確かにYahoo Tech Blogの情報を見ると、ServerとClient間の通信ではキー名はStringで渡すのが正しいようで、WebAuthnKit-iOSの実装が誤っているということではないようです。失礼いたしました。

私はConformanceToolをClientに見立てて、WebAuthnKit-iOSを使ったAuthenticatorの動作確認をしています。
また、私が実施しているConformanceToolのテストでは実際のサーバに通信は行われていないため、ご質問の回答としては
Authenticator部分のみに対してのvalidationになると思います。

私の方でもこのまま確認を続けますので、WebAuthnKit-iOSがCTAP対応するために必要な個所が他に見つかりましたらプルリクエストを改めて上げさせて頂きます。
今回の修正についてはそのまま取り込むと、ClientとServer間の通信に悪影響を及ぼす恐れがありますので提示して頂きました方針で進めて頂ければ問題無いと思います。

今後ともよろしくお願いいたします。