signtool reports Corrupt JAR file for cert chain problem

Status

NSS
Tools
P3
major
16 years ago
8 years ago

People

(Reporter: Roland Titze, Assigned: Nelson Bolyard (seldom reads bugmail))

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: See comment 31)

Attachments

(5 attachments, 2 obsolete attachments)

(Reporter)

Description

16 years ago
The signtool (Version 1.3) reports an error during verification of the signed
patch 106541-21.jar although Suns jarsigner reports no verification problem.

> signtool | head -3

Netscape Signing Tool 1.3 - a signing tool for jar files

> md5 106541-21.jar
MD5 (106541-21.jar) = 9d7ec1e4ee9613237594d674fda364fd

> signtool -v 106541-21.jar > signtool.out 2>&1

> egrep '(NOT VERIFIED|Corrupt)' signtool.out
    NOT VERIFIED   106541-21/SUNWarcx/reloc/usr/lib/pics/sparcv9/libc_pic.a
      (reason: Corrupt JAR file)

The file 106541-21.jar is available at:
http://sunsolve.Sun.COM/pub-cgi/show.pl?target=patches/patch-access

The problem occurs on different Solaris releases, so I assume its not OS
release dependent (I have used Solaris 9 for this test).

Comment 1

16 years ago
Ian,

Could you take a look at this signtool bug?  Thanks.
Assignee: wtc → ian.mcgreer
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P1
Target Milestone: --- → 3.6

Comment 2

16 years ago
Created attachment 93452 [details]
pretty-print of intermediate cert


The chain appears to be three deep, this is the intermediate (signed the cert
which signed the jar).

Comment 3

16 years ago
This is a bug in the cert library, not signtool.

The cert chain is three deep.  I attached the intermediate cert above.  It is a
valid CA cert with the keyUsage and basicConstraints extensions present.  The
nsCertType extension is not present.

In CERT_DecodeDERCertificate, NSS sets default bits for the nsCertType extension
if it is not present.  The code is here:

        /* if no extension, then allow any ssl or email (no ca or object
         * signing)
         */
        cert->nsCertType = NS_CERT_TYPE_SSL_CLIENT | NS_CERT_TYPE_SSL_SERVER |
            NS_CERT_TYPE_EMAIL;

        /* if the basic constraint extension says the cert is a CA, then
           allow SSL CA and EMAIL CA and Status Responder */
        if ((basicConstraintPresent == PR_TRUE)
            && (basicConstraint.isCA)) {
                cert->nsCertType |= NS_CERT_TYPE_SSL_CA;
                cert->nsCertType |= NS_CERT_TYPE_EMAIL_CA;
                cert->nsCertType |= EXT_KEY_USAGE_STATUS_RESPONDER;
        } else if (CERT_IsCACert(cert, NULL) == PR_TRUE) {
                cert->nsCertType |= EXT_KEY_USAGE_STATUS_RESPONDER;
        }

When CERT_VerifyCertChain is called, NSS checks that the nsCertType bit for the
requested usage is present.  In this case, we are verifying the signer of a cert
which signed a JAR file, so were are looking for the object signing CA bit. 
That bit is not set in the code above, so the verification fails.

Bob, Nelson, is there any reason why the code above does not set the
NS_CERT_TYPE_OBJECT_SIGNING_CA bit?
Component: Tools → Libraries
OS: Solaris → All
Hardware: Sun → All
Version: unspecified → 3.0

Comment 4

16 years ago
Created attachment 93456 [details] [diff] [review]
add objectSigningCA to defaults


After making this change and downloading the Sun root cert, the JAR verified
with signtool.	Any reason not to do this?

Comment 5

16 years ago
Yes, there is. CA's which do not explicitly have Signer authority are considered
invalid signers.

The reason for this was object Signing was considered a high assurance
operation. When it was added, there was already an existing infrastructure out
there for SSL and EMAIL. We did not want to automatically accept certs that were
authorized for SSL and email purposes as object signers, so we purposefully
require the CA to include the object signing extension in the CA itself and the
certs.

If you wish to use your own CA you will either need to include the extension (if
you are chaining), or explicitly trust the CA cert itself for object signing.

bob

Comment 6

16 years ago
This is an intermediate cert.  Even if the root was marked trusted as an object
signing CA, the verification would fail before reaching it.  Someone who receive
the JAR file will most likely not have the intermediate in her database, only
the root.

IINM, the purpose of usage extensions is to _restrict_ the possible uses of a
certificate.  If such an extension is not present, the cert should be considered
valid for all possible usages (provided the user trusts the CA that signed it).
 In this case, the intermediate CA cert is a perfectly valid CA, with no usage
restrictions.  I don't think it should be necessary for a user to have to
explicitly trust it for any particular usage, and it's confusing that it will
work for SSL and S/MIME, but not object signing.

Also, the only usage extension with any notion of "object signing CA" is the
nsCertType extension, which is considered to be deprecated (I thought).  It
doesn't seem correct to me that NSS should require this extension to be present,
only that it should honor it if it is.

Comment 7

16 years ago
Yes, you are describing the case we are protecting against.

Verisign signs several intermediate CA's with the intention of those CA handling
SSL or S/MIME. These Intermediates may be issued to other companies. Because
Verisign may have signed those CA before the object signing feature went in, and
because the object signing feature is 1) netscape specific, 2) a high assurance
feature, and 3) 'new', the decision was made to give the object signing trust
the opposite of other trusts. You need to opt in at every stage rather than opt out.

So the long and short of it is the code is operating as designed. Whether or not
the design is reasonable would be another discussion.

Comment 8

16 years ago
Okay, I understand the problem a little better now.  So let's go back to the
beginning: how do we fix this bug?  We could:

1) Keep existing semantics, requiring users to obtain and trust any intermediate
object signing CA certs that don't have the nsCertType extension;

or

2) Change the semantics to include object signing CA by default when nsCertType
is not present.

This isn't necessarily a unique problem.  What if a new extendedKeyUsage bit was
added?  What would that mean for all certs signed before that bit was defined
that did not have the extension?  Would they be valid for that usage or not?
(Assignee)

Comment 9

16 years ago
Ian, my understanding of these options matches yours.  These options,
when present, reduce the capabilities of the cert.  They don't add to
the capabilities of the cert.  I've not heard the explanation Bob 
gave before this.  

It seems to me that, now that we have such a small market share, we're 
no longer in a position to drive CAs to issue certs our way.  If we don't
work with properly issued standard certs, we only reduce our own market
share.

Comment 10

16 years ago
To elaborate on what I said in comment #8:

If a CA issues certs with no usage restrictions, I assume that means the cert
has unrestricted use, now and for as long as it is valid.  CA's are still in
control if new usages appear: they could have the policy of always including the
usage extensions, with all the current bits turned on (this will prevent them
from being co-opted by new bits appearing in the future).  Also, they could
alleviate this problem by issuing certs with shorter validity periods.

If, in the past, CA's were caught by surprise when new usages appeared, I don't
think that is our problem (as implementors).  In the case you described,
Verisign should have used the SSL and S/MIME bits, instead of issuing certs
without the extension, and then assuming new usages would be automatically
restricted.

Otherwise, the question I posed in comment #8 becomes a difficult one for
implementors to answer, and we run into problems like this one.
Target Milestone: 3.6 → 3.5.1

Updated

16 years ago
Target Milestone: 3.5.1 → 3.6

Comment 11

16 years ago
There are two ways to grant signing capability to a key:  Netscape's "object 
signing" and the newer "code signing" extended key usage.

The Netscape "object signing method" which uses the old Netscape certtype 
extension has always required that each CA in the certificate chain has a 
Netscape certtype extension that contains the NS_CERT_TYPE_OBJECT_SIGNING_CA 
bit. We should *not* change this. If the user's certificate has a netscape 
certtype extension that contains the "object signing" bit, the CAs in the chain 
must follow the Netscape rules.

The second method is to include the "id-kp-codeSigning" OID in the standard 
extended key usage extension.  The rules for processing this extension do not 
require that the CA contains a similar extension.  That is, the path processing 
description (RFC 3280) does not include any requirement to have this OID in 
every level of the chain.

I suggest that we handle "object signing" and "code signing" as two separate 
capabilities, that are validated in their own way. As a final step, we can 
allow certs that validate for either one to satify the requirements for 
the "object signing" USAGE passed into the VerifyCert function.

This will allow certificates issued under the new EKU rules to work, while 
maintaining the existing rules for Netscape certtype processing.

Comment 12

16 years ago
Moving to 3.7.

I agree with Terry's proposal.  We are handling the netscapeCertType extension
correctly (though it has a strange definition).  But we should also verify, in
an independent channel, whether or not the EKU says this cert is a valid
codeSigner.  If so, we should allow it to verify JARs.
Target Milestone: 3.6 → 3.7

Comment 13

15 years ago
Moved to target milestone 3.8 because the original
NSS 3.7 release has been renamed 3.8.
Target Milestone: 3.7 → 3.8

Comment 14

15 years ago
Created attachment 116481 [details] [diff] [review]
separate code paths for EKU/nsCertType verification


This patch attempt to implement Terry's proposal.  The current
certUsageObjectSigning is not changed.	A new certUsageCodeSigning is added. 
This latter usage will verify when a cert (or a cert in its chain) does not
have the nsCertType extension, as long as the chain can validate on the basis
of the EKU extension.  When CERT_VerifyCert is called with
certUsageObjectSigning, a failure will lead to an attempt to verify using
certUsageCodeSigning.
Attachment #93456 - Attachment is obsolete: true

Updated

15 years ago
Attachment #116481 - Flags: review?(relyea)
(Assignee)

Comment 15

15 years ago
Several comments on this patch:

1. This patch changes the behavior of CERT_VerifyCert, but not
CERT_VerifyCertificate.  

2. I think creating a second externally visible usage value is undesirable 
for several reasons:
a) code/object signing is conceptually a signel usage.  The fact that there are
two (or more) separate tests required to test a cert for this usage should not
be reflected as a second usage, IMO.  
b) a program that calls this method with the new usage value will have the 
same problem that existing programs have with the existing usage, namely, only
one of the two necessary tests will be performed.  

I think the right fix will somehow do both necessary tests with just a single
usage value, and will work for both CERT_VerifyCert and CERT_VerifyCertificate.

I seem to recall that in NSS 3.3, the presence of this cert extension caused
the cert type flags in the CERTCertificate to be updated to appear as if the
Netscape cert type extension had been present.  Perhaps a similar tactic is
needed with the newer stan cert type.

Comment 16

15 years ago
1: You're right, CERT_VerifyCertificate was overlooked.

2: I agree, but I don't see an obvious.  What I thought was to create an
ObjectSigning usage that would work with both extensions, and a CodeSigning
usage that would work with only the keyUsage extension.  Perhaps at some point
we would want to deprecate support for the nsCertType extension, so from then on
only the CodeSigning usage would be supported.

What would you propose as an alternative?  If we don't make a new public
SECCertUsage, the only way (that I can see) to trigger different behaviors in
CERT_VerifyCert is to change the function signature.

Regarding the cert type flags in the CERTCertificateStr, they're still there,
nothing changed in NSS 3.4.  Part of the problem (as the initial comments on
this bug show) is that they are given default settings even when the nsCertType
extension is _not_ present.  A cert that has key usage that allows cert signing
but no nsCertType extension will have cert->nsCertType set to all CA bits _but_
objectSigning.  This is apparently the desired behavior for the handling the
nsCertType extension, but is not how the codeSigning bit of EKU works.
(Assignee)

Comment 17

15 years ago
After re-reading Terry's comment 11, I see that your patch was trying to 
implement exactly what he suggested.  Assuming that is the right goal,
the only shortcoming of the patch above is that it overlooks
CERT_VerifyCertificate and its variants.  

But I'm not convinced we want to have two separate usages here.  If we do,
then we have to somehow make it crystal clear how to decide which one to use.
Alternatively, if one of them is intended only for "internal" use, then 
perhaps CERT_Verify* (the exported version of this function) should disallow
an external caller to set this usage.  I'll talk to Terry more about this.

Updated

15 years ago
Target Milestone: 3.8 → 3.9

Comment 18

15 years ago
The proposed patch seems to allow nsCertType to be used in end-entity certs to 
satisfy the requirements for the new code-signing usage.  I think this is 
wrong. Validating for object signing must use nsCertType and validating for 
code signing must use the EKU extension.  I think these are currently merged 
together at some point into a single field in the cert structure.  This may 
need to be changed so that the nsCertType and EKU cases can be distinguished.

Nelson has suggested that a single usage (the current object signing usage) 
can be used instead of separate passes. I think this can be done by 
remembering which extension in the end-entity certificate (nsCertType or EKU) 
statisfied the requirement.  This state information is used to compute the 
requirements for the CAs in the chain: either OBJECT_SIGNING_CA (for 
nsCertType) or nothing (for EKU).
(Assignee)

Comment 19

15 years ago
After further consideration, I think that

1) the new certUsageCodeSigner usage for the EKU "code signing" extension,
separate from the certUsageObjectSigner usage that detects the Netscape
cert type extension, is appropriate because the two usages have VERY
different rules.  It is MUCH easier to get a EKU code signing cert than to
get a Netscape object signing cert.  There is no way for a root CA that is
trusted for code signing to restrict the privilege of issuing code signing
leaf certs to some (not all) of its subordinate CAs. 

2) I do NOT believe that a cert chain that satisfies the loose conditions
for the certUsageCodeSigner should also implicitly be valid for the
stricter certUsageObjectSigner usage.  Applications that have historically
wanted and asked for the stricter usage should not all now find themselves
being used with certs that satisfy only the new looser usage rules.

3) A cert chain that satisfies the strict rules of validity for 
certUsageObjectSigner usage could also implicitly be valid for the less
strict certUsageCodeSigner usage.  But an application that wants to allow
either can use CERT_VerifyCertificate and ask for either one.  So there
doesn't seem to be any reason to have either usage also imply the other.

4)  A specific code review comment.  There are several places where the code was
changed similar to this change:

-    if ( issuerCert->nsCertType & caCertType ) {
+    if ( (issuerCert->nsCertType & caCertType) == caCertType ) {

I believe that change was made so that the result would be true when
"caCertType" was zero.  But I believe it also changes the behavior
in an unintended way when caCertType is a mask with more than one
bit set.  So, I think that in all such cases, the new line of code
should be like this:

+    if ( (issuerCert->nsCertType & caCertType) || !caCertType ) {

More comments may be forthcoming.

Comment 20

15 years ago
Comment on attachment 116481 [details] [diff] [review]
separate code paths for EKU/nsCertType verification

rejecting patch until we come agreement on the high level.
Attachment #116481 - Flags: review?(rrelyea0264) → review-

Comment 21

14 years ago
Julien, please reassign to the appropriate Sun
engineer to investigate this bug.
Assignee: bugz → julien.pierre.bugs
Target Milestone: 3.9 → 3.10

Comment 22

13 years ago
This bug has been sitting for a few years.

Do we have any agreement on how to handle this case now ?
Nelson, you mentioned that more comments may be forthcoming . Do you have any
other words of wisdom on this ?

Comment 23

13 years ago
I think Nelson's proposal in comment 19 is adequate. I'm not sure what
objections existed at the time I rejected the patch. It seems that we have 2
issues here:

1) Historic evaluation of the Netscape extension. We need to preserve this
historic value.

2) Supporting the current industry standard extension.

I believe Nelson's proposal does both of these.... I'm not sure if the patch
implements the proposal.

bob
(Assignee)

Comment 24

13 years ago
My thinking on this subject had continued to evolve.  I no longer think it 
is very important to preserve the old Netscape "object signing" design 
(in which signer certs and all its parent CA certs must have an object
signing OID in their EKU extensions) separately from the newer "code signing"
design (in which only the signer's own cert needs the code signing OID in
its EKU, and CA certs require no EKU).  

Regarding the EKU extension, RFC 3280 says: 
"In general, this extension will appear only in end entity certificates. "

Now, that's not MUST or even SHOULD, but it is still widely regarded as 
indicating that standards compliant CAs ought not to put EKUs in CA certs.  

To succesfully argue for the preservation of the old Object Signing feature
separately from code signing, one would need to show that there is any NSS
user (product) that wants it.
AFAIK, the only products to ever use the "object signing" design have 
been Netscape and Mozilla browsers, and the latter do not require signing
of *any* downloads.  There is no present indication that anyone in mozilla
prefers object signing to code signing.  So, there is little motivation for 
CAs to support the old Netscape Object Signing approach by issuing special 
CA certs with Netscape object Signing EKUs in them.  

At this point, it is probably sufficient to treat object signing and code
signing as equivalent.  Indeed, we do not have separate trust bits for the
two.  Judicious control over the object/code signing trust flag for CA 
certs is required, as always.  

My point in this comment is that a single object/code signing usage probably
suffices.  We could even relax the rules for object signing so that they
no longer required the CAs to have EKUs and became equivalent to code signing
certs.  

Comment 25

13 years ago
> AFAIK, the only products to ever use the "object signing" design have 
> been Netscape and Mozilla browsers, and the latter do not require signing
> of *any* downloads. 

I believe this is a 'switch' today. By default this switch is 'off' (don't
require signing). There is a push in Mozilla to make the default 'on' in a
particular release. I'm including doug, who seems to have inheritted that effort.

> There is no present indication that anyone in mozilla
> prefers object signing to code signing.  So, there is little motivation for 
> CAs to support the old Netscape Object Signing approach by issuing special 
> CA certs with Netscape object Signing EKUs in them.

I'm not sure which Mozilla is planning on using. Your statement could very well
be true, but I'd like to get it verified. I know the plan is Mozilla code would
be signed by certs that only Mozilla (not IE) would recognize. I don't know if
Mozilla will also accept binaries signed by general extensions as well.

bob

Comment 26

13 years ago
The plan is that for FireFox 1.5 we would require that extension installations
be signed.  We would disable unsigned xpinstall's (xpi's).
(Assignee)

Updated

13 years ago
QA Contact: bishakhabanerjee → jason.m.reid

Comment 27

13 years ago
Retargetting for 3.11 .
Target Milestone: 3.10 → 3.11

Comment 28

12 years ago
We would not stop the 3.11 release for this bug. Lowering to P2.
Priority: P1 → P2
(Assignee)

Comment 29

12 years ago
(In reply to comment #26)
> The plan is that for FireFox 1.5 we would require that extension installations
> be signed.  We would disable unsigned xpinstall's (xpi's).

Doug, please update us on this plan.  Is it still on target?

Comment 30

12 years ago
for reasons outside my control, the effort to make xpi signed has been dropped for 1.5.

Updated

12 years ago
Target Milestone: 3.11 → 3.12
(Assignee)

Comment 31

12 years ago
This bug started small, and evolved into a big discussion of object signing
vs. code signing.  But putting aside that issue, I see two problems being
reported in this bug report:

1) Signtool signed the JAR without complaint, but signtool cannot verify 
the signature it generated, because the signer's cert chain is inadequate 
for object signing.  IOW, signtool should have complained when the signature
was generated that the signature was going to be unverifiable.  That would
have avoided putting up a "signed" file in a public place, where everyone
who tried it would find its signature invalid.

2) The error message from signtool when reporting the unverifiable signature
is misleading, useless nonsense.  It should report the facts, signature not
verified due to inadequate cert/key usage, not "Jar file corrupt."

Both of those 2 problems can be fixed without having to address the 
object signing vs code signing issue.  I think we should fix these two 
issues soon.  I think mozilla.org will make little progress in signing
their XPIs until we fix this.
Blocks: 251298
(Assignee)

Updated

12 years ago
QA Contact: jason.m.reid → libraries
(Assignee)

Updated

12 years ago
Summary: "signtool -v 106541-21.jar" reports Corrupt JAR file → signtool reports Corrupt JAR file for cert chain problem
(Assignee)

Updated

12 years ago
Component: Libraries → Tools
Priority: P2 → P3
QA Contact: libraries → tools
(Assignee)

Updated

12 years ago
Whiteboard: See comment 31

Updated

11 years ago
Assignee: julien.pierre.boogz → biswatosh.chakraborty

Updated

11 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 32

11 years ago
We've recently had another inquiry from a programmer who got himself
a code signing cert from a CA that is trusted to issue them, and cannot
sign his XPI/JAR file with signtool because signtool requires an object
signing cert.  

AFAIK, THERE ARE *NO* MORE VALID INTERMEDIATE OBJECT SIGNING CAs, anywhere.

The old "Netscape Object Signing" definition of cert chains is DEAD.
Developers who want to acquire object signing certs (whose intermediate
CAs have the special object signing EKU) simply cannot do so any more.
The inability to obtain object signing certs makes this a MAJOR bug.

SO, I now propose to change the implementation of certUsageObjectSigner 
so that it verifies chains according to the looser code signing chain
criteria, which require the special EKU only in the EE cert, and require
that the chain lead to a root trusted for code/object signing.  This 
proposal would also fix bug 321156 (which is really a duplicate of this bug).
This change will make it so that signtool, and mozilla's XPI signature 
verification, will simply work as expected by all people who sign code.

Bob, Julien, and Wan-Teh: Please express your agreement or disagreement
with this proposal.  Anyone else on the CC list is also welcome to 
"weigh in" on this matter.  
Assignee: biswatosh.chakraborty → nelson
Blocks: 321156
Severity: normal → major
Status: ASSIGNED → NEW

Comment 33

11 years ago
I agree with this proposal.
Unsetting target milestone in unresolved bugs whose targets have passed.
Target Milestone: 3.12 → ---

Comment 35

8 years ago
Created attachment 463621 [details] [diff] [review]
drop Netscape Object Signing requirements to match RFC's for how it is currently used, ie XPI signing

The consensus for this bug appears to be:
The old "Netscape Object Signing" definition of cert chains is DEAD.
XPI signing validation checking uses object signing verification, although it should not be so strict (bug 321156).

This patch is meant to no longer require intermediate certs used for certUsageObjectSigner to have a code signing EKU or trust bit set, only that they correctly chain to a Root with a code signing EKU or trust bit.
Attachment #463621 - Flags: review?(wtc)

Updated

8 years ago
Attachment #463621 - Flags: review?(wtc) → review?(nelson)
Comment on attachment 463621 [details] [diff] [review]
drop Netscape Object Signing requirements to match RFC's for how it is currently used, ie XPI signing

Your patch doesn't follow NSS's coding conventions, which require lines 
longer than 80 columns to be wrapped.  If that was the only problem, I'd 
just fix it myself before commiting it.  but ...

Doesn't this patch allow non-CA certs to issue code signing certs?
I think that instead of 

+           requiredCertType = 0;

You want something like:

            requiredCertType = NS_CERT_TYPE_OBJECT_SIGNING_CA |
                               NS_CERT_TYPE_EMAIL_CA |
                               NS_CERT_TYPE_SSL_CA;

In addition to the code change, you'll need to attach test cases that check
to ensure that the patch does NOT 
- allow code signing certs to be issued by non-CA certs
- allow ordinary email certs or SSL server certs, which lack any KU extension
or EKU extension, to be used as code signing certs.

Comment 37

8 years ago
Comment on attachment 463621 [details] [diff] [review]
drop Netscape Object Signing requirements to match RFC's for how it is currently used, ie XPI signing

diff --git a/security/nss/lib/certdb/certdb.c b/security/nss/lib/certdb/certdb.c
--- a/security/nss/lib/certdb/certdb.c
+++ b/security/nss/lib/certdb/certdb.c
@@ -1243,17 +1243,19 @@ CERT_KeyUsageAndTypeForCertUsage(SECCert
            requiredCertType = NS_CERT_TYPE_EMAIL_CA;
            break;
          case certUsageEmailRecipient:
            requiredKeyUsage = KU_KEY_CERT_SIGN;
            requiredCertType = NS_CERT_TYPE_EMAIL_CA;
            break;
          case certUsageObjectSigner:
            requiredKeyUsage = KU_KEY_CERT_SIGN;
-           requiredCertType = NS_CERT_TYPE_OBJECT_SIGNING_CA;
+           requiredCertType = NS_CERT_TYPE_OBJECT_SIGNING_CA |
+                               NS_CERT_TYPE_EMAIL_CA |
+                               NS_CERT_TYPE_SSL_CA;
            break;
          case certUsageAnyCA:
          case certUsageVerifyCA:
          case certUsageStatusResponder:
            requiredKeyUsage = KU_KEY_CERT_SIGN;
            requiredCertType = NS_CERT_TYPE_OBJECT_SIGNING_CA |
                NS_CERT_TYPE_EMAIL_CA |
                    NS_CERT_TYPE_SSL_CA;
diff --git a/security/nss/lib/certhigh/certvfy.c b/security/nss/lib/certhigh/certvfy.c
--- a/security/nss/lib/certhigh/certvfy.c
+++ b/security/nss/lib/certhigh/certvfy.c
@@ -789,17 +789,18 @@ cert_VerifyCertChainOld(CERTCertDBHandle
             * it was given permission by its signer to be a CA.
             */
            /*
             * if basicConstraints says it is a ca, then we check the
             * nsCertType.  If the nsCertType has any CA bits set, then
             * it must have the right one.
             */
            if (!isca || (issuerCert->nsCertType & NS_CERT_TYPE_CA)) {
-               isca = (issuerCert->nsCertType & caCertType) ? PR_TRUE : PR_FALSE;
+               isca = (( issuerCert->nsCertType & caCertType) || !caCertType )
+                       ? PR_TRUE : PR_FALSE;
            }
 
            if (  !isca  ) {
                PORT_SetError(SEC_ERROR_CA_CERT_INVALID);
                LOG_ERROR_OR_EXIT(log,issuerCert,count+1,0);
            }
 
            /* make sure key usage allows cert signing */
@@ -1191,17 +1192,17 @@ CERT_VerifyCertificate(CERTCertDBHandle 
         }
         if ( CERT_CheckKeyUsage(cert, requiredKeyUsage) != SECSuccess ) {
             if (PR_TRUE == requiredUsage) {
                 PORT_SetError(SEC_ERROR_INADEQUATE_KEY_USAGE);
             }
             LOG_ERROR(log,cert,0,requiredKeyUsage);
             INVALID_USAGE();
         }
-        if ( !( certType & requiredCertType ) ) {
+        if ( ( certType & requiredCertType ) != requiredCertType ) {
             if (PR_TRUE == requiredUsage) {
                 PORT_SetError(SEC_ERROR_INADEQUATE_CERT_TYPE);
             }
             LOG_ERROR(log,cert,0,requiredCertType);
             INVALID_USAGE();
         }
 
         /* check trust flags to see if this cert is directly trusted */
@@ -1398,17 +1399,17 @@ CERT_VerifyCert(CERTCertDBHandle *handle
        EXIT_IF_NOT_LOGGING(log);
        requiredKeyUsage = 0;
        requiredCertType = 0;
     }
     if ( CERT_CheckKeyUsage(cert, requiredKeyUsage) != SECSuccess ) {
        PORT_SetError(SEC_ERROR_INADEQUATE_KEY_USAGE);
        LOG_ERROR_OR_EXIT(log,cert,0,requiredKeyUsage);
     }
-    if ( !( certType & requiredCertType ) ) {
+    if ( ( certType & requiredCertType ) != requiredCertType ) {
        PORT_SetError(SEC_ERROR_INADEQUATE_CERT_TYPE);
        LOG_ERROR_OR_EXIT(log,cert,0,requiredCertType);
     }
 
     /* check trust flags to see if this cert is directly trusted */
     if ( cert->trust ) { /* the cert is in the DB */
        switch ( certUsage ) {
          case certUsageSSLClient:

Comment 38

8 years ago
Created attachment 464853 [details] [diff] [review]
drop Netscape Object Signing requirements to match RFC's for how it is currently used, ie XPI signing - v2

Test cases forthcoming
Attachment #463621 - Attachment is obsolete: true
Attachment #464853 - Flags: review?(nelson)
Attachment #463621 - Flags: review?(nelson)

Comment 39

8 years ago
Created attachment 466347 [details]
test cases

Test cases to show that patch does NOT:
- allow code signing certs to be issued by non-CA certs
- allow ordinary email certs or SSL server certs, which lack any KU extension
or EKU extension, to be used as code signing certs.

Comment 40

8 years ago
Created attachment 466350 [details]
example files for test cases

Zip file of supporting XPI and certificates used in test cases
You need to log in before you can comment on or make changes to this bug.