1167820 - sec_error_bad_der - pkix: incorrect handling of directory name constraints

Status: RESOLVED INVALID

(Core :: Security: PSM, defect)

Description

[James Dingwall]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150523043448

Steps to reproduce:

Created a self signed CA, intermediate CA and web server SSL certificate with EJBCA.  OpenSSL displays the name constraints in the CA and intermediate as:
            X509v3 Name Constraints: 
                Permitted:
                  DirName: C = GB, L = City, O = example.com
                  email:example.com
                  DNS:example.com
                  DNS:.example.com

The server certificate is for www.example.com.


Actual results:

Accessing the site with Firefox 38 displays the error below.  Other tools (curl, chromium-browser connect ok and display the expected response)

Secure Connection Failed

An error occurred during a connection to www.example.com. security library: improperly formatted DER-encoded message. (Error code: sec_error_bad_der)

    The page you are trying to view cannot be shown because the authenticity of the received data could not be verified.
    Please contact the website owners to inform them of this problem.


Expected results:

The web server content is displayed in the browser.

Comment 1

[James Dingwall]

The problem seems to be in security/pkix/lib/pkixnames.cpp where there I believe there is an inverted logic test where both the included and excluded directory name constraints are matched for success where actually a match failure is desirable for the exclusions.  I don't believe that simply inverting the test for == Success is a correct solution (although it seems to prove this is where the problem lies in my testing as changing it will allow access specifically to my site) since that would allow all classes of error with the certificate to pass and would seem to cause an error for the case that there are no name constraints since that is the first test in CheckPresentedIDConformsToNameConstraintsSubtrees.

  rv = CheckPresentedIDConformsToNameConstraintsSubtrees(
         presentedIDType, presentedID, nameConstraints,
         NameConstraintsSubtrees::permittedSubtrees);
  if (rv != Success) {
    return rv;
  }

  rv = CheckPresentedIDConformsToNameConstraintsSubtrees(
         presentedIDType, presentedID, nameConstraints,
         NameConstraintsSubtrees::excludedSubtrees);
  if (rv != Success) {
    return rv;
  }

...

With some further debugging messages added it seems that CheckPresentedIDConformsToNameConstraintsSubtrees returns in the do { ... } while { ... } returns here so the function didn't succesfully complete so perhaps that indicates a bug with the certificate generation?

    // GeneralSubtree ::= SEQUENCE {
    //      base                    GeneralName,
    //      minimum         [0]     BaseDistance DEFAULT 0,
    //      maximum         [1]     BaseDistance OPTIONAL }
    Reader subtree;
    rv = ExpectTagAndGetValue(subtrees, der::SEQUENCE, subtree);
    if (rv != Success) {
      return rv;
    }

Providing a set of certificates which demonstrates this issue would be possible if that would help.

Comment 2

[James Dingwall]

Examining the EJBCA source code (6_2_0_ce) it appears that the excluded name constraints is added to the certificate even if they are empty.  Looking at RFC 5280 this seems to be permitted:

      NameConstraints ::= SEQUENCE {
           permittedSubtrees       [0]     GeneralSubtrees OPTIONAL,
           excludedSubtrees        [1]     GeneralSubtrees OPTIONAL }

      GeneralSubtrees ::= SEQUENCE SIZE (1..MAX) OF GeneralSubtree

i.e. there are one or more trees in the name constraints if it is present

      GeneralSubtree ::= SEQUENCE {
           base                    GeneralName,
           minimum         [0]     BaseDistance DEFAULT 0,
           maximum         [1]     BaseDistance OPTIONAL }

      BaseDistance ::= INTEGER (0..MAX)

i.e. there may be 0 or more fields within the included/excluded subtree if it is present which may mean that the do {} while {} of CheckPresentedIDConformsToNameConstraintsSubtrees should be while {} do {}.  I don't have a full understanding of the specification syntax so my interpretation may be incorrect.

Comment 3

[James Dingwall]

After re-examining everything I think I'm going to eat my hat and say this is not an FF issue.

1. re-reading the grammar for name constraints shows that the workings of CheckPresentedIDConformsToNameConstraintsSubtrees is correct, if the OPTIONAL GeneralSubtrees sequence is there it must have a minimum length of 1 entry (but an entry can be 0 length?)

2. CheckPresentedIDConformsToNameConstraintsSubtrees has a case statement to see if it is evaluating the permitted/excluded case to adjust the return value accordingly.

I will try to patch the EJBCA code to not add the constraints when they are empty or alternatively to populate the excluded constraints list with a dummy value.

Comment 4

[James Dingwall]

Comparing the pkix behaviour to what I think is the equivalent in OpenSSL suggests OpenSSL doesn't follow the RFC / is more liberal as https://git.openssl.org/?p=openssl.git;a=blob;f=crypto/x509v3/v3_ncons.c;h=315bd3c294c35ba1faa5b3ac719b3368d3b2e3e8;hb=HEAD contains a check at line 233 that there are > 0 entries and does not return an error for the == 0 entries case.

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Sounds like the certificates being generated are invalid wrt RFC 5280.