When CERT_AddTempCertToPerm fails with TOKEN_NOT_LOGGED_IN, it *has* added the cert with RANDOM trust bits

UNCONFIRMED
Assigned to

Status

NSS
Libraries
P2
normal
UNCONFIRMED
8 years ago
4 years ago

People

(Reporter: David Woodhouse, Assigned: Robert Relyea)

Tracking

3.12.6
3.12.9
x86_64
Linux

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-GB; rv:1.9.2.7) Gecko/20100803 Fedora/3.6.7-1.fc13 Firefox/3.6.7
Build Identifier: nss-3.12.6-12.fc13.x86_64

We seem to be required to log in, just to deal with trust on CA certificates.

Discovered in Evolution, where the certificate manager UI started failing unexpectedly. It's even nastier there -- a call to CERT_AddTempCertToPerm() fails, but has actually *succeeded* in adding the cert to the database, just with the wrong permissions. 

See Evolution bug: https://bugzilla.gnome.org/show_bug.cgi?id=626066

Reproducible: Always

Actual Results:  
 (with SQL database)


[dwmw2@i7 tmp]$ certutil -d sql:`pwd` -N
Enter a password which will be used to encrypt your keys.
The password should be at least 8 characters long,
and should contain at least one non-alphabetic character.

Enter new password: 
Re-enter password: 
[dwmw2@i7 tmp]$ certutil -d sql:`pwd` -A -i ../class3.crt -n class3 -t TC,C,C
Enter Password or Pin for "NSS Certificate DB":


Expected Results:  
 (with DBM database)

[dwmw2@i7 tmp]$ certutil -d `pwd` -N
Enter a password which will be used to encrypt your keys.
The password should be at least 8 characters long,
and should contain at least one non-alphabetic character.

Enter new password: 
Re-enter password: 
[dwmw2@i7 tmp]$ certutil -d `pwd` -A -i ../class3.crt -n class3 -t TC,C,C
[dwmw2@i7 tmp]$ certutil -d `pwd` -L

Certificate Nickname                                         Trust Attributes
                                                             SSL,S/MIME,JAR/XPI

class3                                                       CT,C,C
That's correct.  Editing the trust in the sqk DB requires a login, because
the trust info is authenticated. 

This bug report says the cert was added with the "wrong" permissions.
I think you mean the "wrong" trust flags.  Without authentication, 
there should be NO new trust flags.  Is that what you saw? 
If so, then this is all working as designed and intended, not a bug.
If not, please tell us what trust flags you saw.
(Reporter)

Comment 2

8 years ago
The trust flags are random, I think.

See the patch at https://bugzilla.gnome.org/show_bug.cgi?id=626066#c18

When we call CERT_AddTempCertToPerm() and it fails with SEC_ERROR_TOKEN_NOT_LOGGED_IN, it *has* added the CA with random trust bits. Authenticating and then calling CERT_AddTempCertToPerm() fails, because the cert already exists. We have to just call CERT_ChangeCertTrust for the newly-added cert.

I've just retested this with the above patches applied. After asking Evolution to import a cert, I get asked for the password to log in to the token. I run certutil at that point, before entering the password, and the cert has *already*
been added. In the latest test it was added as follows, but I've seen various different bits set:
CAcert Class 3 Root - Root CA                                CT,c,C
OK, well, I think adding a cert with NO trust bits while unauthenticated
is thought to be acceptable, but adding a cert with RANDOM trust bits 
should never be acceptable, authenticated or otherwise.  

Also, at one time, the act of adding a cert that was already present was
not deemed to be a failure.  The reasoning was "you wanted this cert to 
be in the DB/token, and now it is, so you've succeeded".  If that is no 
longer true, then something has regressed, IMO.
(Reporter)

Comment 4

8 years ago
Indeed. In the meantime, given that I need to fix things for an Evolution release today, I think the best option is this; please advise:

		srv = CERT_AddTempCertToPerm (tmpCert,
					     nickname,
					     &trust);

		/* If we aren't logged into the token, then what *should*
		   happen is the above call should fail, and we should
		   authenticate and then try again. But see NSS bug #595861.
		   With NSS 3.12.6 at least, the above call will fail, but
		   it *will* have added the cert to the database, with
		   random trust bits. We have to authenticate and then set
		   the trust bits correctly. And calling
		   CERT_AddTempCertToPerm() again doesn't work either -- it'll
		   fail even though it arguably ought to succeed (which is
		   probably another NSS bug).
		   So if we get SEC_ERROR_TOKEN_NOT_LOGGED_IN, we first try
		   CERT_ChangeCertTrust(), and if that doesn't work we hope
		   we're on a fixed version of NSS and we try calling
		   CERT_AddTempCertToPerm() again instead.
		*/
		if (srv != SECSuccess &&
		    PORT_GetError() == SEC_ERROR_TOKEN_NOT_LOGGED_IN &&
		    e_cert_db_login_to_slot (NULL, PK11_GetInternalKeySlot())) {
			srv = CERT_ChangeCertTrust (CERT_GetDefaultCertDB (),
						    tmpCert, &trust);
			if (srv != SECSuccess)
				srv = CERT_AddTempCertToPerm (tmpCert,
							      nickname,
							      &trust);
		}
		if (srv != SECSuccess) {
			set_nss_error (error);
			return FALSE;
		}
Assignee: nobody → rrelyea
Priority: -- → P2
Summary: sql database requires password to set trust on CA certificates → When CERT_AddTempCertToPerm fails with TOKEN_NOT_LOGGED_IN, it *has* added the cert with RANDOM trust bits
Target Milestone: --- → 3.12.9
Version: unspecified → 3.12.6
David, 
At present, I can't extemporaneously advise you on the "best" workaround.
Thinking aloud, if you delete the cert and then re-add it, that might work 
with broken and unbroken versions of NSS.
(Reporter)

Comment 6

8 years ago
Is there any particular reason to prefer that approach over "fix the permissions, and if that fails then someone must have fixed NSS so it wasn't added in the first place, so try adding it again", as in comment 4?
(Reporter)

Comment 7

8 years ago
s/permissions/trust bits/ of course; sorry.
Not really.  Whatever does the job with the widest variety of versions of NSS
and types of NSS DB is probably best.
(Assignee)

Comment 9

8 years ago
re comment 4: 

yes, that's the best way to handle the problem.It will not only work with sql databases, but with other tokens that require auth from the get go.

(I believe this is the code we have in certutil).

bob
You need to log in before you can comment on or make changes to this bug.