-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Credential ID uniqueness expectations are inconsistent/vague #579
Comments
For the RP's who have implemented or are going to implement ONLY credentialID based lookup, uniqueness of the credentialID is very important. I would suggest it to be minimum of 16 bytes. At the same time, we probably should refrain away from how its generated which allows flexibility and inventiveness for the authenticators. |
Actually, that's a lot more requirements than I found with a quick search, I only found the reference in 4.2.1. @akshayu It's not possible to guarantee this as the authenticator does not have the required context and vendors do not know how other vendors generate Credential IDs. Collisions are always possible. @emlun There is little point to that probability requirement in that form as it cannot be verified or guaranteed. I am not an expert in encryption theory so I can be wrong but since each authenticator uses a unique wrapping key, the uniqueness can be designed for the value being wrapped but this does not guarantee the resulting key handle has the same uniqueness across different authenticators. If this is correct, a vendor cannot even guarantee it for his own authenticators let alone globally. I only see one sane way to implement this: generate 150 random bits and add them to the wrapped value. It will work but is a huge waste of space. How about a 32 byte minimum length of a value generated by a cryptographic operation? |
I was using the 2017-08-11 WD, perhaps we had different versions? Anyway, those are all good points; I retract suggestion (2), and then (1) isn't really necessary anymore. However I think it's a good idea to instead explicitly allow RPs to refuse registering a duplicate credential ID. Authenticators are already required to generate a new credential ID for every call to new Allowing RPs to refuse duplicate credential IDs would also eliminate the risk of DoS attacks that register the same credential ID a billion times to break an RP that looks the user up in a table keyed with the credential ID. |
I agree on the explicitly allowing to refuse a duplicate. I would still add a minimum length requirements and the requirement to use a cryptographic operation. The first one will create lots of bits, I think the second one will guarantee at least as much entropy as the encryption key (again, not a cryptography expert). |
I agree the RP SHOULD refuse a duplicate credential id. I would make that a MUST but some may only be indexing credential id by user id for starred credentials so might not be able to easily tell if it is duplicate. Collisions in UUID https://en.wikipedia.org/wiki/Universally_unique_identifier are relatively well understood. A 16 byte value is probably sufficient. If the problem is a weak random number generator more bits probably won't help. Encrypting a random value won't add to the entropy, encrypting a non random value would add entropy from the encryption key assuming that the key changes. How best to generate it probably depends on the RNG and the quality of its sources of entropy. Being overly specific may force people to make suboptimal choices for their platform. Some may have a source of cosmic background radiation and others not so much. The point is as random as possible and RP reject duplicates if they detect collisions. |
Totally agree with @ve7jtb suggestion. |
So 16 bytes length, fine by me. What about the entropy? |
If RPs are recommended that they SHOULD refuse duplicate credential IDs I think it's not necessary to specify any minimum length or entropy for the credential ID. The worst that would happen is a bad user experience with badly designed authenticators, and that's on the authenticator designer if that's worth ignoring the SHOULD clause. |
In cases of UAF, the credential Id (called keyId) is unique in the scope of aaguid (called AAID). We cannot guarantee that the credential Ids are unique across all authenticators. For usability, the server may check duplication of credential Ids in the scope aaguid instead of looking up all records. |
@Kieun Does UAF generate a new credential ID if registration is retried? |
@emlun Yes. Credential Ids are generated randomly by authenticators during registration. |
Why would it not be up to the RP to decide if they want to accept duplicate credential IDs? Just key off the user id too now. We've added it especially for you :) |
@christiaanbrand As long as it is explicitly allowed that's fine, the current wording does not allow it. |
Of course the RP can already choose no accept it or not, but if you really lawyer it up on §6.1 it technically doesn't allow the RP to refuse duplicate credential IDs. :) I see two main reasons for recommending against accepting duplicates:
|
Actually... apparently someone already thought of this. The last paragraph of 6.1. Registering a new credential reads:
How embarassing to have missed that... It might however be worthwile to make this one of the formal algorithm steps. |
This is an editorial fix. Making the step a formal part of the algorithm step can help clarify things. Assigning the issue to PR milestone. @equalsJeffH @nadalin @YubicoDemo for tracking. |
moving to CR milestone given @emlun's #579 (comment) and PR #709 being open. |
) * move the credentialId uniqueness handling to the formal alg steps. Close #579 * be more precise about what ceremony we mean
As pointed out in #558, the requirements on the uniqueness of the credential ID are not completely clear.
§4.1. PublicKeyCredential Interface reads:
§4.2.1 Information about Public Key Credential (interface AuthenticatorAttestationResponse), point
attestationObject
reads:§5. WebAuthn Authenticator model reads:
§5.2.1. The authenticatorMakeCredential operation reads:
(All emphasis added)
In summary, the uniqueness of the credential ID is specified as
I suggest that
a concrete requirement is given instead of "with a high probability" - for example "with a probability greater than 1 - 2^150" (this value(edit: 150 bits of entropy) would mean a collision chance < 1E-9 at 1E18 credential IDs generated).The text was updated successfully, but these errors were encountered: