385946 - Can't import certificate into cert database in FIPS mode (certutil).

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Slavomir Katuscak]

I tried following steps:

1. Create DB.
certutil -d "${TEST_DB}" -N -f "${PWD_FILE}"

2. Set DB to FIPS mode.
modutil -dbdir ${TEST_DB} -fips true -force

3. Generate self-signed certificate.
certutil -d "${TEST_DB}" -S -s "${CERT_NAME}" -n "${TEST_NAME}" -x -t "C,C,C" -f "${PWD_FILE}" -z "${NOISE_FILE}"

4. Export certificate.
certutil -d "${TEST_DB}" -L -n "${TEST_NAME}" -r -o testcert.crt

5. Remove certificate from DB.
certutil -d "${TEST_DB}" -F -n "${TEST_NAME}" -f "${PWD_FILE}"

6. Import certificate.
certutil -d "${TEST_DB}" -A -n "${TEST_NAME}" -t ",," -i testcert.crt

Last step failed with this error message:
certutil: could not add certificate to token or database: Error adding certificate to database.

Import is OK when FIPS is disabled.
PKCS#12 format export/import using pk12util is OK.

Found on Mac OS X, reproduced on Solaris.

Comment 1

[Julien Pierre]

I have a suspicion that this is related to bug 396999 . I will try to reproduce this problem.

Comment 2

[Julien Pierre]

Hmm, maybe not. There was no other token involved here.

Comment 3

[Julien Pierre]

I reproduced the problem . It can be reduced to the following 3 steps.
1) certutil -d . -N
2) modutil -dbdir . -fips true -force
3) certutil -d . -A -n nickname -t ,,, -i cert.der

Any existing cert will do. I'm going to debug this next. I hope this problem only affects certutil, otherwise it is fairly severe.

Comment 4

[Julien Pierre]

This happens deep in Stan code, which calls C_CreateObject. That returns with CKR_USER_NOT_LOGGED_IN .

The stack is :

=>[1] import_object(tok = 0x80aad80, sessionOpt = (nil), objectTemplate = 0x8046288, otsize = 9U), line 246 in "devtoken.c"
  [2] nssToken_ImportCertificate(tok = 0x80aad80, sessionOpt = (nil), certType = NSSCertificateType_PKIX, id = 0x80af760, nickname = 0x808bab0 "boom", encoding = 0x80af768, issuer = 0x80af770, subject = 0x80af778, serial = 0x80af780, email = (nil), asTokenObject = 1), line 568 in "devtoken.c"
  [3] PK11_ImportCert(slot = 0x80a90f8, cert = 0x80b4968, key = 0, nickname = 0x808bab0 "boom", includeTrust = 0), line 903 in "pk11cert.c"
  [4] AddCert(slot = 0x80a90f8, handle = 0x80a9728, name = 0x808bab0 "boom", trusts = 0x8046999 ",,,", inFile = 0x808c3f0, ascii = 0, emailcert = 0, pwdata = 0x804668c), line 188 in "certutil.c"
  [5] certutil_main(argc = 10, argv = 0x8046738, initialize = 1), line 2320 in "certutil.c"
  [6] main(argc = 10, argv = 0x8046738), line 2453 in "certutil.c"

There is no opportunity to authenticate in there. PK11_ImportCert doesn't have wincx, and it is a public function. So, the only fix is for the application to authenticate.

certutil has the following code to do the import :

	if (!PK11_IsFriendly(slot)) {
	    rv = PK11_Authenticate(slot, PR_TRUE, pwdata);
	    if (rv != SECSuccess) {
		SECU_PrintError(progName, "could not authenticate to token or database");
		GEN_BREAK(SECFailure);
	    }
	}

	rv =  PK11_ImportCert(slot, cert, CK_INVALID_HANDLE, name, PR_FALSE);
	if (rv != SECSuccess) {
	    SECU_PrintError(progName, "could not add certificate to token or database");
	    GEN_BREAK(SECFailure);
	}

In other words, it tries to authenticate to all non-friendly tokens before importing. The problem is that the FIPS token is still marked friendly, because it allows reading token certs without being logged in. But the FIPS token does not allow writing token certs without being logged in. Unfortunately, we don't have separate read-friendly / write-friendly bits to make that distinction. So I'm not sure what test we could replace it with.

One easy fix is to just unconditionally log in before importing. That would fix certutil. But there is a possibility that other applications have used this same test too, and will still be broken.

Another fix - which I would prefer, would be to mark the FIPS token as unfriendly. The fact that it is marked friendly has already caused other problems. I think bug 396999 would also be resolved if the FIPS token was marked unfriendly.

Comment 5

[Julien Pierre]

Attachment: Change test in certutil

I don't really like this test very much, because it has intimate knowledge of our FIPS and softoken properties, which is dynamically loaded and can change over time.

I would just as soon make the PK11_Authenticate unconditional, but that would cause an extra login where none was previously required to add certs to the non-FIPS softoken. This patch will cause authentication to all tokens before import, except for this one case.

Comment 6

[Robert Relyea]

Comment on attachment 301786 [details] [diff] [review]
Change test in certutil

r+ rrelyea. 

This is a better test than PK11_IsFriendly() when importing certificates.

Comment 7

[Robert Relyea]

The other (probably better) option is to unconditionally require a password when importing certificates. If you are using the new shared DB, even the non-fips token needs to be authenticated when importing trusted certs and keys.

http://wiki.mozilla.org/NSS_Shared_DB#Special_coding_for_MACed_entries

This is one of the reasons we still allow old applications to open the legacy database so that they can keep the semantic of writing to a database without a password.

An early survey was done when the above was proposed to determine if there were any callers that actually modified the database without authenticating. I believe certutil was the only one, so it was amended to authenticate when importing trusted certs, one may argue we should have just went for any certs.

bob

Comment 8

[Julien Pierre]

Thanks for the link, Bob. I saw the auth check for the trust and was wondering why it was different.

I checked this in to the trunk. I will ask Nelson for review for the branch.

Checking in certutil.c;
/cvsroot/mozilla/security/nss/cmd/certutil/certutil.c,v  <--  certutil.c
new revision: 1.126; previous revision: 1.125
done

Comment 9

[Julien Pierre]

Thanks, Nelson. I checked the patch in to NSS_3_11_BRANCH :

Checking in certutil.c;
/cvsroot/mozilla/security/nss/cmd/certutil/certutil.c,v  <--  certutil.c
new revision: 1.97.2.11; previous revision: 1.97.2.10
done