#3025 DER encoding error for an enumerated type with a value of zero
Closed: migrated 3 years ago by dmoluguw. Opened 5 years ago by awood.

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:
1. 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
EnumerationZeroTest.java


Metadata Update from @mharmsen:
- Custom field component adjusted to None
- Custom field feature adjusted to None
- Custom field origin adjusted to None
- Custom field proposedmilestone adjusted to None
- Custom field proposedpriority adjusted to None
- Custom field reviewer adjusted to None
- Custom field type adjusted to None
- Custom field version adjusted to None
- Issue assigned to jmagne
- Issue set to the milestone: 10.5

5 years ago

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

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

jss-output.txt

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.

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.

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();
        }
    }
}

Metadata Update from @mharmsen:
- Issue priority set to: blocker

5 years ago

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) {

Metadata Update from @jmagne:
- Issue priority set to: None (was: blocker)

5 years ago

Metadata Update from @mharmsen:
- Custom field rhbz adjusted to https://bugzilla.redhat.com/show_bug.cgi?id=1560682

5 years ago

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

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.

Dogtag PKI is moving from Pagure issues to GitHub issues. This means that existing or new
issues will be reported and tracked through Dogtag PKI's GitHub Issue tracker.

This issue has been cloned to GitHub and is available here:
https://github.com/dogtagpki/pki/issues/3143

If you want to receive further updates on the issue, please navigate to the
GitHub issue and click on Subscribe button.

Thank you for understanding, and we apologize for any inconvenience.

Metadata Update from @dmoluguw:
- Issue close_status updated to: migrated
- Issue status updated to: Closed (was: Open)

3 years ago

Login to comment on this ticket.

Metadata
Attachments 2
Attached 5 years ago View Comment
Attached 5 years ago View Comment