Closed Bug 310518 Opened 15 years ago Closed 15 years ago
_ERROR _INVALID _PASSWORD is defined but not used
We have two error codes with the same meaning: SEC_ERROR_BAD_PASSWORD and SEC_ERROR_INVALID_PASSWORD. Fortunately, SEC_ERROR_INVALID_PASSWORD is defined but not used. See http://lxr.mozilla.org/mozilla/search?string=SEC_ERROR_INVALID_PASSWORD Should we update our documentation and source code comments to say SEC_ERROR_INVALID_PASSWORD is deprecated and you should use SEC_ERROR_BAD_PASSWORD instead? Anyone remember if SEC_ERROR_INVALID_PASSWORD was ever used?
Hmm... perhaps they are different. One can interpret SEC_ERROR_INVALID_PASSWORD as a password that does not meet PKCS #11 or FIPS 140-2 requirements such as it is not UTF-8, it is too short, or it doesn't contain three or more classes of characters. What do you think?
Two other password related error codes that are defined but not used: SEC_ERROR_RETRY_PASSWORD http://lxr.mozilla.org/mozilla/search?string=SEC_ERROR_RETRY_PASSWORD SEC_ERROR_RETRY_OLD_PASSWORD http://lxr.mozilla.org/mozilla/search?string=SEC_ERROR_RETRY_OLD_PASSWORD
Severity: normal → trivial
I looks like the 'retry' portions were part of the old libsec ui code, since it's really an application policy whether it is going to retry the password. SEC_ERROR_BAD_PASSWORD and SEC_ERROR_INVALID_PASSWORD looks like the former is a password that was incorrect, and the latter is a password that doesn't meet the requirements of a password. They map to CKR_PIN_INCORRECT and CKR_PIN_INVALID respectively. Currently pk11err.c maps both to SEC_ERROR_BAD_PASSWORD. I think we should change that mapping for the general case (that is PK11_MapError() should return SEC_ERROR_INVALID_PASSWORD for CKR_PIN_INVALID). I was initially worried about applications checking the SEC_ERROR_BAD_PASSWORD and doing something different (like not retrying), but I see NSS does has the same logic in not retrying CKR_PIN_INCORRECT in it's loop. I verified the C_Login can only return CKR_BAD_PASSWORD. PKCS #11 has another define CKR_PIN_LEN_RANGE, which seems to be a special case of CKR_PIN_INVALID (CKR_PIN_INVALID means you used invalid characters in your password, CKR_PIN_LEN_RANGE means your password was too short or too long. ). These definitiona are all suppositions based on which functions return which error codes and the error code name, none of these are define explicitly in the PKCS #11 spec AFAIK.
Should we add a new SEC_ERROR_PASSWORD_LEN_RANGE error code?
Attachment #197951 - Flags: review?(rrelyea)
Bob, PKCS #11 v2.20 says CKR_PIN_INVALID and CKR_PIN_LEN_RANGE can only be returned by C_InitPIN and C_SetPIN. I think C_InitToken can also return these two error codes if the pPin parameter is the initial SO PIN and pPin has invalid characters in it or is too long or too short, but these two error codes are not documented as possible return values of C_InitToken.
Comment on attachment 197951 [details] [diff] [review] Improve CKR_PIN_XXX error mapping r=relyea.
Attachment #197951 - Flags: review?(rrelyea) → review+
Patch checked in. I don't know what to do about SEC_ERROR_RETRY_PASSWORD and SEC_ERROR_RETRY_OLD_PASSWORD. Should we update our documentation to deprecate them?
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Summary: Deprecate SEC_ERROR_INVALID_PASSWORD → SEC_ERROR_INVALID_PASSWORD is defined but not used
Target Milestone: --- → 3.11
You need to log in before you can comment on or make changes to this bug.