Closed Bug 310518 Opened 15 years ago Closed 15 years ago

SEC_ERROR_INVALID_PASSWORD is defined but not used

Categories

(NSS :: Libraries, defect, trivial)

3.10.2
defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wtc, Assigned: wtc)

Details

Attachments

(1 file)

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.