Add test vectors for PRF extension

[emlun]

Closes #2088.

---

Preview | Diff

[zacknewman]

Do you think it makes sense to add something about the client not sending this data to the server? Admittedly my experience with PRF is related to password managers. The server must never know your "password", or in this case the symmetric encryption key; therefore it's essential that the client does not send results. hmac-secret is encrypted, so the fact that gets sent to the server is not a problem.

A more explicit example is Bitwarden. The server does most of the RP operations including sending the PublicKeyCredentialRequestOptions to use. Their client apps also handle the client-side RP operations. Only the client must access the PRF output in order to decrypt the vault; therefore when sending the response to the server, it necessarily omits prf from the extensions it sends back to the server. It of course sends hmac-secret still since that's encrypted and necessary in order for the server to complete the ceremony—most notably asserting the signature is valid.

Is it "obvious" that the client should not send this data seeing how if the server had this info it's now at best devolved into a shared secret?

[emlun]

Do you think it makes sense to add something about the client not sending this data to the server?
[...]
Is it "obvious" that the client should not send this data seeing how if the server had this info it's now at best devolved into a shared secret?

Hm. I wanted to say that yes, this should be obvious enough in the use cases where this is relevant, and that there are other use cases where you actually do want to send the PRF outputs to the server. But we do since recently have this step in both §7.1. Registering a New Credential and §7.2. Verifying an Authentication Assertion:

4. Let clientExtensionResults be the result of calling credential.getClientExtensionResults().

which perhaps complicates the matter a bit. The Relying Party Operations sections also don't really make a distinction between client-side and server-side parts of the RP, rather treating both as one entity, so it also doesn't seem quite appropriate to simply add something simple like ", omitting any properties that should not be sent to the server" either.

Maybe the right way to go is to just call out the client-side-only use case in the description of AuthenticationExtensionsPRFOutputs.results, and warn to not accidentally send them back to the server if that is undesired for the use case.

[zacknewman]

Yeah, it's a tough call. Normally no distinction is made between client and server RP operations; however this is one exception (for password managers at least) where it's important to not follow the ceremony criteria where one "blindly" sends getClientExtensionResults or toJSON. Like you said, there are other use cases of PRF that may want the info to be sent to the server; so it seems "weird" to caution about one set of use cases that doesn't universally apply.

Maybe the right way to go is to just call out the client-side-only use case in the description of AuthenticationExtensionsPRFOutputs.results, and warn to not accidentally send them back to the server if that is undesired for the use case.

That's where I am leaning too: a brief cautionary note that mentions one may want to omit this information when sending getClientExtensionResults or toJSON to the server when the server should not have this sensitive data.

[agl]

Suggested change

	- <code>h&apos;c4172e982e9097c39a6c0cb720cb375b92e3fcad154a63e43a93f1096b1e1973' = SHA-256(UTF-8("WebAuthn PRF test vectors") || 0x01)</code>
	- <code>h'c4172e982e9097c39a6c0cb720cb375b92e3fcad154a63e43a93f1096b1e1973' = SHA-256(UTF-8("WebAuthn PRF test vectors") || 0x01)</code>

[emlun]

Thanks, but no - this renders incorrectly for similar reasons as noted in PR #2175. Bikeshed replaces ' with ' in the output, but replaces ' with "’" (U+2019) here.

Although now that you pointed it out, maybe it should be ' on both sides in that case...

[agl]

(I did not confirm the hmac_secret values in detail; i.e. by recomputing the encryptions and decryptions.)

[Firehed]

Is it "obvious" that the client should not send this data seeing how if the server had this info it's now at best devolved into a shared secret?

From my eyes, it is not - though I'll readily admit that's more due to unfamiliarity with the use-cases than going through the actual spec. While it's out of scope on this PR to add more info about use-cases, I think it would be a worthwhile addition to the spec to go into more details there. MasterKale's gist provides a bit more context, but I think more detail about what party manages and/or retains what data would go a long way towards keeping implementations on the successful path.

When it comes to cryptography, and especially handling of key material (or inputs to derive it), I'd recommend erring strongly on the side of overly-explicit.

That's where I am leaning too: a brief cautionary note that mentions one may want to omit this information when sending getClientExtensionResults or toJSON to the server when the server should not have this sensitive data.

It may be too late to materially change the mechanics, but if this is the case, it seems to me that the sensitive data should have a different access path that's outside of those APIs. In particular, toJSON is really intended to dump the whole thing to the server for processing (or, at least, that's how I plan on using it once there's enough browser support); default-including data in there that should not leave the client is asking for problematic implementations.

[zacknewman]

When it comes to cryptography, and especially handling of key material (or inputs to derive it), I'd recommend erring strongly on the side of overly-explicit.

I agree especially since it would be a quick remark.

It may be too late to materially change the mechanics, but if this is the case, it seems to me that the sensitive data should have a different access path that's outside of those APIs. In particular, toJSON is really intended to dump the whole thing to the server for processing (or, at least, that's how I plan on using it once there's enough browser support); default-including data in there that should not leave the client is asking for problematic implementations.

A harsh rebuttal would be that any RP that "accidentally" sends sensitive data is likely not qualified to do whatever it is they are trying to do (e.g., password manager). If/when I implement my own passkey-only password manager; then on the client, I would use toJSON and then remove the data I don't want (e.g., prf) to send. I think there are other reasons one may want to remove certain data from toJSON anyway. For example I would personally remove all superfluous data (e.g., id and rawId from RegistrationResponseJSON and publicKey, publicKeyAlgorithm, and authenticatorData from AuthenticatorAttestationResponseJSON) since that puts more work on the server to perform inter-data matching for those that are already parsing the attestationObject which already contains all of that info.

I should add that for my general-purpose server-side WebAuthn library I'm writing I always error when deserializing the client extensions when prf exists as a way to force downstream users to omit that info. Obviously my library is not "general-purpose" enough for the use cases @emlun is referring to, but I'm OK with that.

[Firehed]

A harsh rebuttal would be that any RP that "accidentally" sends sensitive data is likely not qualified to do whatever it is they are trying to do (e.g., password manager).

I agree, but that doesn't tend to stop it in practice. Mistakes happen.

I think there are other reasons one may want to remove certain data from toJSON anyway. [...]

Indeed! However, once you go down that path, you can rapidly end up at a point where you're doing enough customization to the format that you're better off not bothering with toJSON at all and manually assembling the parts of the format you do want (as you have to do in practice today).

Which is in no way to suggest that your approach is wrong or invalid, but I want to keep in mind the common-case usage of that API. If there's a chance to avoid a pretty major security footgun before that API has widespread support, it's probably wise to do so.

Maybe my concerns are overstated. I suspect that most parties will ignore the extension entirely, and (as you suggest) it'll probably only get used by RPs that actually do need it and would understand the security implications.

In any case, the addition of test vectors for this looks great and I'm excited to see them added. Maybe it'd be better to spin this aspect off into a separate issue?

[emlun]

Yes, let's move that discussion to #2177 (thanks @zacknewman!).

[selfissued]

Approved during the 9-Oct-24 call