bcgit / bc-java

Bouncy Castle Java Distribution (Mirror)
https://www.bouncycastle.org/java.html
MIT License
2.27k stars 1.12k forks source link

[RFE] handle Signature.getParameters without throwing UnsupportedOperationException with EC Keypair #770

Closed space88man closed 3 years ago

space88man commented 4 years ago

Version: BouncyCastle 1.66 from maven central

JDK8u >= 252 sun.security.pkcs10.PKCS10 (used by keytool) has changed its implementation of encodeAndSign to call Signature.getParameters(). BC will throw an UnsupportedOperationException using the -certreq subcommand with a EC keypair. In contrast, SunEC returns null.

This used to work with <= JDK8u242 (PKCS10.java did not use the method Signature.getParameters())

How to reproduce:

  1. Create bc.properties file so BC replaces SunEC

    # bc.properties
    security.provider.3=org.bouncycastle.jce.provider.BouncyCastleProvider
  2. Create keypair:

    # this uses SunEC; BC will work too because during signing of TBSCertificate
    # getParameters() is not called
    keytool -keystore 0.p12 -storetype PKCS12 -storepass password -genkeypair -alias ecpair \
        -keyalg EC -keysize 256 -dname CN=ecpair
  3. Generate CSR

    # use sun.security.tools.keytool.Main instead of keytool to read properties file
    # - don't think that keytool can consume the properties file directly
    #
    java -Djava.security.properties=bc.properties  sun.security.tools.keytool.Main -v -keystore 0.p12 \
        -storetype PKCS12 -storepass password -certreq -alias ecpair -file ecpair.req
    keytool error: java.lang.UnsupportedOperationException
    java.lang.UnsupportedOperationException
        at java.security.SignatureSpi.engineGetParameters(SignatureSpi.java:404)
        at java.security.Signature$Delegate.engineGetParameters(Signature.java:1431)
        at java.security.Signature.getParameters(Signature.java:1006)
        at sun.security.pkcs10.PKCS10.encodeAndSign(PKCS10.java:244)
        at sun.security.tools.keytool.Main.doCertReq(Main.java:1470)
        at sun.security.tools.keytool.Main.doCommands(Main.java:979)
        at sun.security.tools.keytool.Main.run(Main.java:370)
        at sun.security.tools.keytool.Main.main(Main.java:363)

To correlate line numbers with source code: the specific JDK I am using is Fedora 32's java-1.8.0-openjdk-1.8.0.265.b01-1.fc32.x86_64, whose matching source is java-1.8.0-openjdk-src-1.8.0.265.b01-1.fc32.x86_64. The code snippet in question, which happened around u252 is:

    File is: sun/security/pkcs10/PKCS10.java
    196     /**
    197      * Create the signed certificate request.  This will later be
    198      * retrieved in either string or binary format.
    199      *
    200      * @param subject identifies the signer (by X.500 name).
    201      * @param signature private key and signing algorithm to use.
    202      * @exception IOException on errors.
    203      * @exception CertificateException on certificate handling errors.
    204      * @exception SignatureException on signature handling errors.
    205      */
    206     public void encodeAndSign(X500Name subject, Signature signature)
    207     throws CertificateException, IOException, SignatureException {
    208         DerOutputStream out, scratch;
    209         byte[]          certificateRequestInfo;
    210         byte[]          sig;
    211 
    212         if (encoded != null)
    213             throw new SignatureException("request is already signed");
    214 
    215         this.subject = subject;
    216 
    217         /*
    218          * Encode cert request info, wrap in a sequence for signing
    219          */
    220         scratch = new DerOutputStream();
    221         scratch.putInteger(BigInteger.ZERO);            // PKCS #10 v1.0
    222         subject.encode(scratch);                        // X.500 name
    223         scratch.write(subjectPublicKeyInfo.getEncoded()); // public key
    224         attributeSet.encode(scratch);
    225 
    226         out = new DerOutputStream();
    227         out.write(DerValue.tag_Sequence, scratch);      // wrap it!
    228         certificateRequestInfo = out.toByteArray();
    229         scratch = out;
    230 
    231         /*
    232          * Sign it ...
    233          */
    234         signature.update(certificateRequestInfo, 0,
    235                 certificateRequestInfo.length);
    236         sig = signature.sign();
    237         sigAlg = signature.getAlgorithm();
    238 
    239         /*
    240          * Build guts of SIGNED macro
    241          */
    242         AlgorithmId algId = null;
    243         try {
    244             AlgorithmParameters params = signature.getParameters();
    245             algId = params == null
    246                     ? AlgorithmId.get(signature.getAlgorithm())
    247                     : AlgorithmId.get(params);
    248         } catch (NoSuchAlgorithmException nsae) {
    249             throw new SignatureException(nsae);
    250         }
    251 
    252         algId.encode(scratch);     // sig algorithm
    253         scratch.putBitString(sig);                      // sig
    254 
    255         /*
    256          * Wrap those guts in a sequence
    257          */
    258         out = new DerOutputStream();
    259         out.write(DerValue.tag_Sequence, scratch);
    260         encoded = out.toByteArray();
    261     }

Line 244: it's not handling the case where getParameters() throws UnsupportedOperationException.

This can also be reproduced in JDK 11(java-11-openjdk-11.0.8.10-2.fc32.x86_6).

Note:

space88man commented 4 years ago

With JDK 11 , the error looks like:

$ java -cp lib/bcprov-jdk15on-1.66.jar -Djava.security.properties=bc.properties \
    sun.security.tools.keytool.Main -v  -keystore 0.p12 -storetype PKCS12 -storepass password \
    -certreq  -alias ecpair -file ecpair.req
keytool error: java.lang.UnsupportedOperationException
java.lang.UnsupportedOperationException
    at java.base/java.security.SignatureSpi.engineGetParameters(SignatureSpi.java:405)
    at java.base/java.security.Signature$Delegate.engineGetParameters(Signature.java:1453)
    at java.base/java.security.Signature.getParameters(Signature.java:1029)
    at java.base/sun.security.pkcs10.PKCS10.encodeAndSign(PKCS10.java:235)
    at java.base/sun.security.tools.keytool.Main.doCertReq(Main.java:1578)
    at java.base/sun.security.tools.keytool.Main.doCommands(Main.java:1093)
    at java.base/sun.security.tools.keytool.Main.run(Main.java:398)
    at java.base/sun.security.tools.keytool.Main.main(Main.java:391)

...so the JDK folks must have switched to using Signature.getParameters() in recent builds.

space88man commented 4 years ago

Possibly BC should return null like SunEC (as it does for RSA in rsa/DigestSignatureSpi.java) ?

public final AlgorithmParameters getParameters()

Returns the parameters used with this signature object.

If this signature has been previously initialized with parameters (by calling the setParameter method),
this method returns the same parameters. If this signature has not been initialized with parameters,
this method may return a combination of default and randomly generated parameter values if the underlying
signature implementation supports it and can successfully generate them. Otherwise, null is returned.

Returns:
    the parameters used with this signature, or null
Since:
    1.4
See Also:
    setParameter(AlgorithmParameterSpec) 
dghgit commented 3 years ago

I've modified this, and a few others to return null. Will be in 167b06 on https://www.bouncycastle.org/betas