DER encoding error for an enumerated type with a value of zero

SilleBille commented 5 years ago

This issue was migrated from Pagure Issue #3025.Originally filed by awood on 2018-05-24

Creating a CRL entry with a CRL Reason of "unspecified" (which has a value of 0) results in an encoding error when the JDK attempts to generate the CRL due to the way JSS encodes ASN.1 enumerations in DerOutputStream. Instead of encoding the enumeration as 0x0A0100, the value portion of the TLV is omitted and it's encoded as 0x0A00.

From what I can tell from X.690 (https://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf), "The encoding of an enumerated value shall be that of the integer value with which it is associated" and for integers "The encoding of an integer value shall be primitive. The contents octets shall consist of one or more octets."

Steps to Reproduce:

Compile and run the attached test case:

% javac -cp ~/.m2/repository/org/mozilla/jss/4.4.0/jss-4.4.0.jar EnumerationZeroTest.java

% java -cp ~/.m2/repository/org/mozilla/jss/4.4.0/jss-4.4.0.jar:. -Djava.library.path=/usr/lib64/jss EnumerationZeroTest

SilleBille commented 5 years ago

Posted by jmagne on 2018-05-29:

Looks like the attached test file is not being found by the system for me..

SilleBille commented 5 years ago

Posted by awood on 2018-05-30:

If you can't get the test working, I'll attach the output.

Full encoded extension: 30090603551d1504020a00  Encoded CRL Reason: 0a00    Reason value: 0

is the output line that demonstrates the problem. Basically it just shows that an enumeration value of zero isn't being encoded like the others and ultimately that causes a DER decoding error in the JDK classes.

I think the problem is in the putEnumerated method of DerOutputStream.

SilleBille commented 5 years ago

Posted by jmagne on 2018-05-30:

Thanks for the additional attachment, but I was referring to the fact that when I try to download the file, they system is telling me it's not found.

SilleBille commented 5 years ago

Posted by awood on 2018-05-30:

import org.mozilla.jss.JSSProvider;
import org.mozilla.jss.netscape.security.util.BitArray;
import org.mozilla.jss.netscape.security.util.DerInputStream;
import org.mozilla.jss.netscape.security.util.DerValue;
import org.mozilla.jss.netscape.security.x509.AuthorityKeyIdentifierExtension;
import org.mozilla.jss.netscape.security.x509.CRLExtensions;
import org.mozilla.jss.netscape.security.x509.CRLNumberExtension;
import org.mozilla.jss.netscape.security.x509.CRLReasonExtension;
import org.mozilla.jss.netscape.security.x509.Extension;
import org.mozilla.jss.netscape.security.x509.KeyIdentifier;
import org.mozilla.jss.netscape.security.x509.RevocationReason;
import org.mozilla.jss.netscape.security.x509.RevokedCertImpl;
import org.mozilla.jss.netscape.security.x509.RevokedCertificate;
import org.mozilla.jss.netscape.security.x509.X500Name;
import org.mozilla.jss.netscape.security.x509.X509CRLImpl;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.math.BigInteger;
import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.Security;
import java.security.cert.CertificateFactory;
import java.security.cert.X509CRL;
import java.security.interfaces.RSAPublicKey;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Date;
import java.util.List;

/** Class to demonstrate DER encoding failure when using an ASN.1 enumerated type with a value of zero.
 *
 *  RFC 5280's section 5.3.1 lists the valid values for certificate revocation codes:
 *
 *  CRLReason ::= ENUMERATED {
 *       unspecified             (0),
 *       keyCompromise           (1),
 *       cACompromise            (2),
 *       affiliationChanged      (3),
 *       superseded              (4),
 *       cessationOfOperation    (5),
 *       certificateHold         (6),
 *            -- value 7 is not used
 *       removeFromCRL           (8),
 *       privilegeWithdrawn      (9),
 *       aACompromise           (10) }
 *
 */
public class EnumerationZeroTest {
    static {
        System.loadLibrary("jss4");
        Security.addProvider(new JSSProvider());
    }

    private EnumerationZeroTest() {

    }

    public static String toHex(byte[] bytes) {
        StringBuilder sb = new StringBuilder(bytes.length * 2);
        for (byte b : bytes) {
            sb.append(String.format("%02x", b));
        }

        return sb.toString();
    }

    /**
     * Calculate the KeyIdentifier for an RSAPublicKey and place it in an AuthorityKeyIdentifier extension.
     *
     * Java encodes RSA public keys using the SubjectPublicKeyInfo type described in RFC 5280.
     * <pre>
     * SubjectPublicKeyInfo  ::=  SEQUENCE  {
     *   algorithm            AlgorithmIdentifier,
     *   subjectPublicKey     BIT STRING  }
     *
     * AlgorithmIdentifier  ::=  SEQUENCE  {
     *   algorithm               OBJECT IDENTIFIER,
     *   parameters              ANY DEFINED BY algorithm OPTIONAL  }
     * </pre>
     *
     * A KeyIdentifier is a SHA-1 digest of the subjectPublicKey bit string from the ASN.1 above.
     *
     * param key the RSAPublicKey to use
     * return an AuthorityKeyIdentifierExtension based on the key
     * throws IOException if we can't construct a MessageDigest object.
     */
    public static AuthorityKeyIdentifierExtension buildAuthorityKeyIdentifier(RSAPublicKey key)
        throws IOException {
        try {
            MessageDigest d = MessageDigest.getInstance("SHA-1");

            byte[] encodedKey = key.getEncoded();

            DerInputStream s = new DerValue(encodedKey).toDerInputStream();
            // Skip the first item in the sequence, AlgorithmIdentifier.
            // The parameter, startLen, is required for skipSequence although it's unused.
            s.skipSequence(0);
            // Get the subjectPublicKey bit string
            BitArray b = s.getUnalignedBitString();
            byte[] digest = d.digest(b.toByteArray());

            KeyIdentifier ki = new KeyIdentifier(digest);
            return new AuthorityKeyIdentifierExtension(ki, null, null);
        }
        catch (NoSuchAlgorithmException e) {
            throw new IOException("Could not find SHA1 implementation", e);
        }
    }

    /**
     * Output the DER encoding of a CRLExtension for examination
     */
    public static void outputExtension(CRLReasonExtension ext) throws Exception {
        ByteArrayOutputStream resultBytesOut = new ByteArrayOutputStream();
        ext.encode(resultBytesOut);

        byte[] encodedBytes = resultBytesOut.toByteArray();
        System.out.print("Full encoded extension: " + toHex(encodedBytes));
        Extension reasonExt = new Extension(new DerValue(encodedBytes));
        System.out.print("\tEncoded CRL Reason: " + toHex(reasonExt.getExtensionValue()));
        DerValue reasonValue = new DerValue(reasonExt.getExtensionValue());
        System.out.println("\tReason value: " + reasonValue.getEnumerated());
    }

    /**
     * Build a CRL using JSS
     * param useZero whether or not to try creating a CRLEntry with the reason set to "unspecified"
     * return an X509CRL object
     * throws Exception if anything goes wrong
     */
    public static X509CRL buildCrl(boolean useZero) throws Exception {
        try {
            KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA");
            generator.initialize(2048);
            KeyPair kp = generator.generateKeyPair();

            List<RevokedCertificate> revokedCerts = new ArrayList<>();
            for (int i = 0; i < 10; i++) {
                // 7 is an unused value in the enumeration
                if (i == 7 || (i == 0 && !useZero)) {
                    continue;
                }

                CRLReasonExtension reasonExt = new CRLReasonExtension(RevocationReason.fromInt(i));
                outputExtension(reasonExt);

                CRLExtensions entryExtensions = new CRLExtensions();
                entryExtensions.add(reasonExt);

                revokedCerts.add(
                    new RevokedCertImpl(BigInteger.valueOf((long) i), new Date(), entryExtensions));
            }

            CRLExtensions crlExtensions = new CRLExtensions();
            crlExtensions.add(new CRLNumberExtension(BigInteger.ONE));
            crlExtensions.add(buildAuthorityKeyIdentifier((RSAPublicKey) kp.getPublic()));

            X500Name issuer = new X500Name("CN=Test");

            Date now = new Date();

            Calendar calendar = Calendar.getInstance();
            calendar.add(Calendar.DAY_OF_MONTH, 365);
            Date until = calendar.getTime();

            X509CRLImpl crlImpl = new X509CRLImpl(
                issuer,
                now,
                until,
                revokedCerts.toArray(new RevokedCertificate[] {}),
                crlExtensions
            );

            crlImpl.sign(kp.getPrivate(), "SHA256withRSA");

            CertificateFactory cf = CertificateFactory.getInstance("X.509");
            return (X509CRL) cf.generateCRL(new ByteArrayInputStream(crlImpl.getEncoded()));
        }
        catch (GeneralSecurityException | IOException e) {
            throw new RuntimeException(e);
        }
    }

    public static void main(String[] args) {
        try {
            X509CRL crl = buildCrl(false);

            System.out.println(crl.toString());

            buildCrl(true);  // will throw exception
        }
        catch (Exception e) {
            e.printStackTrace();
        }
    }
}

SilleBille commented 5 years ago

Posted by jmagne on 2018-05-30:

Proposed simple patch to address this:

Was able to test the encoding to see that is matches what we expect for a enum value of zero: as follows:

reason 0: 300a0603551d1504030a0100 reason 08:: 300a0603551d1504030a0108

I didn't get far enough to test it with the sun classes, but should be easy to see if they accept it.

diff --git a/base/util/src/netscape/security/util/DerOutputStream.java b/base/util/src/netscape/security/util/DerOutputStream.java index df16d0b38..25bd3b81f 100644 --- a/base/util/src/netscape/security/util/DerOutputStream.java +++ b/base/util/src/netscape/security/util/DerOutputStream.java -205,7 +205,7 public class DerOutputStream write((byte) i); } } else {

// positive case
// positive case or 0 case for (length = 4; length > 0; --length) { if ((i & bytemask) != 0) break; -217,7 +217,12 public class DerOutputStream putLength(length + 1); write(0x00); } else {
putLength(length);
if (i == 0) { //handle when value is 0
putLength(1);
write((byte) i);
} else {
putLength(length);
} } // unrolled loop switch (length) {

SilleBille commented 5 years ago

Posted by mharmsen on 2018-06-22:

commit 06eacad918e745d632067deea398f14ce9da29ac (HEAD -> JSS_4_4_BRANCH, origin/JSS_4_4_BRANCH) Author: Jack Magne Date: Fri Jun 15 14:53:53 2018 -0700

SilleBille commented 5 years ago

Posted by jmagne on 2018-06-22:

Last comment is incorrect. This patch did not make it in the first commit of the other ticket since it was not a blocker and we wanted some more testing on it.

SilleBille / pki

DER encoding error for an enumerated type with a value of zero #462