479029 - OCSP Response signature cert found invalid if issuer is trusted only for SSL

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Rob Stradling]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.7pre) Gecko/2009021718 Minefield/3.0.7pre
Build Identifier: Trunk

This problem occurs with the latest Firefox CVS code, but I'm taking an educated guess that it's an NSS issue.  I've not managed to figure out how to test it with the NSS command-line tools, but OpenSSL believes that the OCSP Response is OK:

$ openssl ocsp -issuer TBSX509CASGC.crt -no_nonce -cert sha256.tbs-internet.com.crt -url http://ocsp.tbs-x509.com -VAfile TBSX509CASGC.crt
Response verify OK
sha256.tbs-internet.com.crt: good
        This Update: Feb 18 11:06:48 2009 GMT
        Next Update: Feb 19 11:06:48 2009 GMT

Reproducible: Always

Steps to Reproduce:
1. Tick "When an OCSP server connection fails, treat the certificate as invalid"
2. Browse to https://sha256.tbs-internet.com

Actual Results:  
Secure Connection Failed
An error occurred during a connection to sha256.tbs-internet.com.
Invalid OCSP signing certificate in OCSP response.
(Error code: sec_error_ocsp_invalid_signing_cert)

Expected Results:  
Successful navigation to the HTTPS page, with no OCSP errors encountered/displayed.

Comment 1

[Rob Stradling]

Attachment: Issuing CA and OCSP Signing Certificate

Comment 2

[Rob Stradling]

Attachment: Site certificate

Comment 3

[Rob Stradling]

Attachment: OCSP Response

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Rob, what is the effect on this bug of the two intermediate CA certificates 
that were discussed in bug 477186?  
Can this problem be reproduced without those certs in the DB?
Can it be reproduced WITH those certs in the DB?

Comment 5

[Rob Stradling]

I just grabbed the very latest Firefox CVS + NSS HEAD code again.

(1) http://crt.comodoca.com/UTNAddTrustServerCA.crt
(2) http://crt.comodoca.com/UTNAddTrustServer_CA.crt
(3) http://crt.comodoca.com/AddTrustUTNServerCA.crt

With none of these 3 cross-certs installed, I get the sec_error_ocsp_invalid_signing_cert error.

With (1) or (2) or (3) installed, I get the sec_error_ocsp_invalid_signing_cert error.

With (1) and (2) installed, I get the sec_error_ocsp_invalid_signing_cert error.

With (3) plus at least one of (1) or (2) installed, I get the following error instead...
"Secure Connection Failed
sha256.tbs-internet.com uses an invalid security certificate.
The certificate is not trusted because the issuer certificate is unknown.
(Error code: sec_error_unknown_issuer)"
...so I think my statement "I cannot reproduce
any of the problems I listed in Comment #2" in Bug #477186 Comment #27 was a bit premature.
With this setup, I also get the same error for other sites with similar certificate chains that only use RSA/SHA-1.

If, following that last test, I tick the "This certificate can identify websites" trust purpose for at least 1 of the installed cross-certs, I get sec_error_ocsp_invalid_signing_cert error again instead of sec_error_unknown_issuer.

Nelson, I'll await your response before I consider posting any further follow-ups to this bug or to Bug #477186.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

There are several relevant NSS test tools for this, all of which 
were probably built when you built your own NSS + FF from the trunk.
One such tool is vfyserv.  The command 
   vfyserv -o -d DB sha256.tbs-internet.com
where "DB" is the path name of the directory containing a set of cert/key 
DB files and also a copy of the nssckbi shared library
will do an https connection with the server and do an OCSP check on the 
server's cert.  

Running that test, I found several distinct problems.  I will file bugs 
about one or more of them.  The issue immediately relevant, I think, is 
not the type of signature algorithm used in any cert or CRL or OCSP response
but rather is apparently an incorrect test for a trusted issuer cert for 
OCSP signatures.  It's an issue with the cert chain for the cert used to 
sign the OCSP repsonse.  More details to follow.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

It seems that the fix for bug 381317, while a big improvement, was still
not 100% right.  It seems that the code that verifies a cert chain for 
the usage "certUsageVerifyCA" makes an arbitrary choice of which of the 
three sets of trust flags to check.  That's the wrong thing to do for 
OCSP responder chains.  Patch forthcoming.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Attachment: Patch v1 for NSS Trunk

This patch recognizes that the trust testing done for the usage named
certUsageAnyCA is fundamentally the right test for CA certs that are 
being used to sign ocsp responses, and changes the OCSP code to use that
usage.  See 
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/certhigh/certvfy.c&rev=1.67&mark=734-735,761-773#726

It also fixes some bugs that prevented that usage from being used with 
certain of the old CERT_VerifyCert* functions.

I'd like Alexei and/or Bob to review this.

Comment 9

[Rob Stradling]

Thanks Nelson.

With that patch applied, I can no longer reproduce any of the sec_error_ocsp_invalid_signing_cert errors I mentioned in comment #5.

I still see the sec_error_unknown_issuer error when I have "(3) plus at least one of (1) or (2) installed", but your comment #6 seems to hint that you'll be filing a separate bug about this.

Comment 10

[Rob Stradling]

Further to Comment #9:
> I can no longer reproduce any of the sec_error_ocsp_invalid_signing_cert
> errors...

However...

With that patch applied, (1) installed (with no trust purposes) and (3) *not* installed, I get sec_error_ocsp_invalid_signing_cert when trying to connect to https://www.interfimo.fr/

If I untick "When an OCSP server connection fails, treat the certificate as invalid", I can connect to https://www.interfimo.fr/ OK (even though the site certificate and the OCSP Response are signed by the same Issuing CA Certificate).

Alternatively, if I tick "This certificate can identify web sites" for (1), I no longer get sec_error_ocsp_invalid_signing_cert when connecting to https://www.interfimo.fr/.

Alternatively, if I install (3) as well (with no trust purposes), I no longer get sec_error_ocsp_invalid_signing_cert when connecting to https://www.interfimo.fr/.

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

Rob, 
- Which (if any) of the observations in your comment 10 are describing 
new behaviors that appear to be the result of the patch?
- Which are behaviors that are seen with or without the patch?

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

Rob, I think that perhaps your comment 10 is actually documentation in 
support of bug 479508, yes?

Comment 13

[Alexei Volkov]

Comment on attachment 363256 [details] [diff] [review]
Patch v1 for NSS Trunk

Changes look ok to me, but the patch is incomplete. There are two more functions that need to be changed: in CERT_VerifyCAForCertUsages needs to have the same changes  as you have made in line 752; in pkix_pl_OcspResponse_VerifySignature line 891 please make the change
-            certUsage = certUsageVerifyCA;
+            certUsage = certUsageAnyCA;

Comment 14

[Alexei Volkov]

Comment on attachment 363256 [details] [diff] [review]
Patch v1 for NSS Trunk

I guess the patch should get r+ and integrated, but required additional work.

Comment 15

[Rob Stradling]

In reply to comment #11:
> Rob, 
> - Which (if any) of the observations in your comment 10 are describing
> new behaviors that appear to be the result of the patch?

Comment #10 describes the behaviour I saw with the current trunk FF + trunk NSS code + this patch applied.

> - Which are behaviors that are seen with or without the patch?

With the current trunk FF + trunk NSS code, and *without* this patch applied, I see...

> > With that patch applied, (1) installed (with no trust purposes) and (3)
> > *not* installed, I get sec_error_ocsp_invalid_signing_cert when trying to
> > connect to https://www.interfimo.fr/

Same behaviour.

> > If I untick "When an OCSP server connection fails, treat the certificate as
> > invalid", I can connect to https://www.interfimo.fr/ OK (even though the
> > site certificate and the OCSP Response are signed by the same Issuing CA
> > Certificate).

Same behaviour.

> > Alternatively, if I tick "This certificate can identify web sites" for (1),
> > I no longer get sec_error_ocsp_invalid_signing_cert when connecting to
> > https://www.interfimo.fr/.

I do see sec_error_ocsp_invalid_signing_cert, which makes sense (given your description of what the patch fixes - comments #7 and #8).

> > Alternatively, if I install (3) as well (with no trust purposes), I no
> > longer get sec_error_ocsp_invalid_signing_cert when connecting to
> > https://www.interfimo.fr/.

I see sec_error_unknown_issuer instead.  I can't explain why your patch gets around this problem.

Comment 16

[Rob Stradling]

In reply to comment #12:
> Rob, I think that perhaps your comment 10 is actually documentation in
> support of bug 479508, yes?

Very probably, at least for some of the tests I described.

On a related note, here's one more test that seems to match up with your "Loops are terminated when the total chain length reaches an arbitrary maximum length, currently 20 certs" comment from bug 479308...

With both (1) and (3) installed, and with "This certificate can identify web sites" ticked for both of them, the FF Certificate Viewer shows the following Certificate Hierarchy when I browse to https://www.interfimo.fr :

UTN-USERFirst-Hardware
 AddTrust External CA Root
  UTN-USERFirst-Hardware
   AddTrust External CA Root
    UTN-USERFirst-Hardware
     AddTrust External CA Root
      UTN-USERFirst-Hardware
       AddTrust External CA Root
        UTN-USERFirst-Hardware
         AddTrust External CA Root
          UTN-USERFirst-Hardware
           AddTrust External CA Root
            UTN-USERFirst-Hardware
             AddTrust External CA Root
              UTN-USERFirst-Hardware
               AddTrust External CA Root
                UTN-USERFirst-Hardware
                 AddTrust External CA Root
                  TBS X509 CA business
                   www.interfimo.fr

Comment 17

[Rob Stradling]

> On a related note, here's one more test that seems to match up with your "Loops
> are terminated when the total chain length reaches an arbitrary maximum length,
> currently 20 certs" comment from bug 479308...

Sorry, I meant bug 479508.

Comment 18

[Robert Relyea]

Comment on attachment 363256 [details] [diff] [review]
Patch v1 for NSS Trunk

r+ the changes look correct. I did not verify if they were complete.

Alexi did identify some areas that need to be updated as well (presumably in the pkix code path).

bob

Comment 19

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v2 for NSS trunk (checked in)

This patch incorporates Alexei's suggested changes.

Comment 20

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 364259 [details] [diff] [review]
patch v2 for NSS trunk (checked in)

Alexei, please confirm that this patch contains the additional changes you suggested.

Comment 21

[Alexei Volkov]

Comment on attachment 364259 [details] [diff] [review]
patch v2 for NSS trunk (checked in)

r=alexei. I think the patch is complete.

Comment 22

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 364259 [details] [diff] [review]
patch v2 for NSS trunk (checked in)

Committed on trunk
certhigh/certvfy.c;    new revision: 1.68; previous revision: 1.67
certhigh/ocsp.c;       new revision: 1.57; previous revision: 1.56
libpkix/pkix_pl_nss/pki/pkix_pl_ocspresponse.c; new revision: 1.17

Comment 23

[Kai Engert [:KaiE:]]

Another sample site for this bug is https://www.softballoutlet.com/

Comment 24

[Kai Engert [:KaiE:]]

This bug is marked as "resolved fixed", but the status whiteboard still says "[must address comment 13 before resolving as fixed]".

Is comment 13 addressed?
Someone who understand this bug please clarify, or reopen, or remove the status whiteboard. Thanks.

Comment 25

[Nelson Bolyard (seldom reads bugmail)]

Yes, comment 13 was addressed.

Comment 27

[Johnathan Nightingale [:johnath]]

Dan/Sam - is this something we can take on branch? It is causing deployed EV certs to incorrectly downgrade to DV. On the other hand, I don't know if it requires special tags to be built, for us to take NSS changes post-release.

Comment 28

[Samuel Sidler (old account; do not CC)]

We'd need a new NSS tag. And we aren't going to take that for 1.9.0.8 which is code-frozen. Maybe for 1.9.0.9?

Comment 29

[Nelson Bolyard (seldom reads bugmail)]

We're expecting to release NSS 3.12.3 any day now, before 1 April, maybe
on Mozilla's 11th open source anniversary.

Comment 30

[Daniel Veditz [:dveditz]]

If you get us a tag this week that pretty much corresponds with opening the tree for 1.9.0.10 checkins. We should be able to fit this in to that release easily.

You probably want this for Firefox 3.5 too, right?

Comment 31

[Kai Engert [:KaiE:]]

FYI, I think NSS 3.12.3 won't come with working CRL fetching support. This means upgrading to NSS 3.12.3 will introduce regression bug 474606, unless a workaround (like the one proposed in bug 485052) is added to Firefox, too.

Comment 32

[Daniel Veditz [:dveditz]]

That sounds scary. I think we can't block a 1.9.0.x release for this until we have something workable.

Comment 33

[Samuel Sidler (old account; do not CC)]

And by that, Dan means that we need to have an updated NSS that fixes bug 474606.

Comment 34

[Nelson Bolyard (seldom reads bugmail)]

The problem described in bug 474606 is unrelated to this bug.  
Let's not discuss those problems here.

Comment 35

[Rob Stradling]

I've just marked this bug as blocking bug 485052 (for reasons first explained in bug 477244 comment 53).

Since bug 485502 is already "blocking1.9.1+", this bug also needs to be "blocking1.9.1+".  Since this bug is already RESOLVED FIXED in NSS, I trust that there will be no objections.

Comment 36

[Mike Beltzner [:beltzner, not reading bugmail]]

If this was fixed in the NSS tag we recently took on the 191 branch, can someone please mark it fixed1.9.1?

Comment 37

[Johnathan Nightingale [:johnath]]

This change was included in kai's 3_12_3_RC0 landing as far as I can tell.  ( http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f160224c5221 )

Marking fixed1.9.1