search

google / CTAP2-test-tool

Test tool for CTAP2 authenticators
Apache License 2.0
57 stars 26 forks source link

client_pin_new_requirements_set_pin and client_pin_new_requirements_change_pin pin padding is to 64 bytes not 32 #89

Open ve7jtb opened 3 years ago

ve7jtb commented 3 years ago

In CTAP2.1 the max pin length is 63 bytes and is padded out to 64 bytes.
In CTAP2.0 "The decrypted padded newPin should be of at least 64 bytes length" So 64 bytes is what should be tested for.

Just for fun CTAP2.0 doesn't define errors. For CTAP2.1 the error for too short is CTAP1_ERR_INVALID_PARAMETER someone needs to speak up if they want that changed.

For paddedNewPin being longer than 64 bytes nether spec mentions an error.

I will see about fixing that in CTAP2.1

ve7jtb commented 3 years ago

Related CTAP2.1 issue https://github.com/fido-alliance/fido-2-specs/issues/1115

kaczmarczyck commented 3 years ago

LGTM and thanks for the spec fix for longer padded PINs.

Here's my reasoning for the test. CTAP 2.0 says:

The decrypted padded newPin should be of at least 64 bytes length and authenticator determines actual PIN length by looking for first 0x00 byte which terminates the PIN.

Therefore, CTAP 2.1 seemed like a clarification to me. The sentence above seems to imply at least one byte of padding, and 2.1 makes this explicit. Also, this sentence speaks about "padded" first, which is correctly stated to be 64 bytes at least, and then later talks about "actual" length. This test is checking for the "actual" PIN length.

For more anecdotal evidence, I think this is the "majority" interpretation on my small sample of tested devices. To be fair, I always found this sentence strange. Because "looking for the first" byte implies that it's important from what direction you start looking.

kaczmarczyck commented 3 years ago

@ve7jtb The title has a get_assertion_empty_user_id, but doesn't mention it. Is this by mistake?

ve7jtb commented 3 years ago

I fixed the title. Cut and paste error.

Perhaps it is the error description that confused me "Accepted a PIN with padding of length 32"

should that be "Accepted a PIN with padding of length less than 64" I guess practically the only length less than 64 is 32 that you can test.

kaczmarczyck commented 3 years ago

Oh, I thought this is about actual PIN length vs padded PIN length. Your last error message sounds more like a padded PIN length problem. Are you saying the test is incorrect, or just the message misleading?