From a4c249610e7c0fcc7fd863f14f51c96abc5ca39f Mon Sep 17 00:00:00 2001 From: Weijun Wang Date: Fri, 26 Feb 2021 16:49:44 +0000 Subject: [PATCH] 8259535: ECDSA SignatureValue do not always have the specified length Reviewed-by: mullan --- .../implementations/ECDSAUtils.java | 17 +- .../implementations/SignatureECDSA.java | 17 +- .../dsig/internal/dom/DOMSignatureMethod.java | 10 +- .../xml/internal/security/ShortECDSA.java | 192 ++++++++++++++++++ 4 files changed, 225 insertions(+), 11 deletions(-) create mode 100644 test/jdk/com/sun/org/apache/xml/internal/security/ShortECDSA.java diff --git a/src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/ECDSAUtils.java b/src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/ECDSAUtils.java index 41a100ad7e8..719af4c0508 100644 --- a/src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/ECDSAUtils.java +++ b/src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/ECDSAUtils.java @@ -42,14 +42,15 @@ public final class ECDSAUtils { * The JAVA JCE ECDSA Signature algorithm creates ASN.1 encoded (r, s) value * pairs; the XML Signature requires the core BigInteger values. * - * @param asn1Bytes - * @return the decode bytes + * @param asn1Bytes the ASN.1 encoded bytes + * @param rawLen the intended length of decoded bytes for an integer. + * If -1, choose one automatically. + * @return the decoded bytes * @throws IOException * @see 6.4.1 DSA * @see 3.3. ECDSA Signatures */ - public static byte[] convertASN1toXMLDSIG(byte asn1Bytes[]) throws IOException { - + public static byte[] convertASN1toXMLDSIG(byte asn1Bytes[], int rawLen) throws IOException { if (asn1Bytes.length < 8 || asn1Bytes[0] != 48) { throw new IOException("Invalid ASN.1 format of ECDSA signature"); } @@ -72,7 +73,13 @@ public final class ECDSAUtils { for (j = sLength; j > 0 && asn1Bytes[offset + 2 + rLength + 2 + sLength - j] == 0; j--); //NOPMD - int rawLen = Math.max(i, j); + int maxLen = Math.max(i, j); + + if (rawLen < 0) { + rawLen = maxLen; + } else if (rawLen < maxLen) { + throw new IOException("Invalid signature length"); + } if ((asn1Bytes[offset - 1] & 0xff) != asn1Bytes.length - offset || (asn1Bytes[offset - 1] & 0xff) != 2 + rLength + 2 + sLength diff --git a/src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/SignatureECDSA.java b/src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/SignatureECDSA.java index 40df4338f42..8c2931bb9f9 100644 --- a/src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/SignatureECDSA.java +++ b/src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/SignatureECDSA.java @@ -32,6 +32,7 @@ import java.security.PublicKey; import java.security.SecureRandom; import java.security.Signature; import java.security.SignatureException; +import java.security.interfaces.ECPrivateKey; import java.security.spec.AlgorithmParameterSpec; import com.sun.org.apache.xml.internal.security.algorithms.JCEMapper; @@ -54,6 +55,9 @@ public abstract class SignatureECDSA extends SignatureAlgorithmSpi { /** Field algorithm */ private Signature signatureAlgorithm; + /** Length for each integer in signature */ + private int signIntLen = -1; + /** * Converts an ASN.1 ECDSA value to a XML Signature ECDSA Value. * @@ -61,14 +65,15 @@ public abstract class SignatureECDSA extends SignatureAlgorithmSpi { * pairs; the XML Signature requires the core BigInteger values. * * @param asn1Bytes + * @param rawLen * @return the decode bytes * * @throws IOException * @see 6.4.1 DSA * @see 3.3. ECDSA Signatures */ - public static byte[] convertASN1toXMLDSIG(byte asn1Bytes[]) throws IOException { - return ECDSAUtils.convertASN1toXMLDSIG(asn1Bytes); + public static byte[] convertASN1toXMLDSIG(byte asn1Bytes[], int rawLen) throws IOException { + return ECDSAUtils.convertASN1toXMLDSIG(asn1Bytes, rawLen); } /** @@ -179,8 +184,7 @@ public abstract class SignatureECDSA extends SignatureAlgorithmSpi { protected byte[] engineSign() throws XMLSignatureException { try { byte jcebytes[] = this.signatureAlgorithm.sign(); - - return SignatureECDSA.convertASN1toXMLDSIG(jcebytes); + return SignatureECDSA.convertASN1toXMLDSIG(jcebytes, signIntLen); } catch (SignatureException ex) { throw new XMLSignatureException(ex); } catch (IOException ex) { @@ -203,6 +207,11 @@ public abstract class SignatureECDSA extends SignatureAlgorithmSpi { } try { + if (privateKey instanceof ECPrivateKey) { + ECPrivateKey ecKey = (ECPrivateKey)privateKey; + signIntLen = (ecKey.getParams().getCurve().getField().getFieldSize() + 7) / 8; + // If not ECPrivateKey, signIntLen remains -1 + } if (secureRandom == null) { this.signatureAlgorithm.initSign((PrivateKey) privateKey); } else { diff --git a/src/java.xml.crypto/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignatureMethod.java b/src/java.xml.crypto/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignatureMethod.java index 5b4fdb9af91..c86ca09c719 100644 --- a/src/java.xml.crypto/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignatureMethod.java +++ b/src/java.xml.crypto/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignatureMethod.java @@ -21,7 +21,7 @@ * under the License. */ /* - * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved. */ /* * $Id: DOMSignatureMethod.java 1854026 2019-02-21 09:30:01Z coheigea $ @@ -35,6 +35,7 @@ import javax.xml.crypto.dsig.spec.SignatureMethodParameterSpec; import java.io.IOException; import java.security.*; import java.security.interfaces.DSAKey; +import java.security.interfaces.ECPrivateKey; import java.security.spec.AlgorithmParameterSpec; import java.security.spec.MGF1ParameterSpec; import java.security.spec.PSSParameterSpec; @@ -518,7 +519,12 @@ public abstract class DOMSignatureMethod extends AbstractDOMSignatureMethod { // If signature is in ASN.1 (i.e., if the fallback algorithm // was used), convert the signature to the P1363 format if (asn1) { - return SignatureECDSA.convertASN1toXMLDSIG(sig); + int rawLen = -1; + if (key instanceof ECPrivateKey) { + ECPrivateKey ecKey = (ECPrivateKey)key; + rawLen = (ecKey.getParams().getCurve().getField().getFieldSize() + 7) / 8; + } + return SignatureECDSA.convertASN1toXMLDSIG(sig, rawLen); } else { return sig; } diff --git a/test/jdk/com/sun/org/apache/xml/internal/security/ShortECDSA.java b/test/jdk/com/sun/org/apache/xml/internal/security/ShortECDSA.java new file mode 100644 index 00000000000..85df37dce25 --- /dev/null +++ b/test/jdk/com/sun/org/apache/xml/internal/security/ShortECDSA.java @@ -0,0 +1,192 @@ +/* + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8259535 + * @summary ECDSA SignatureValue do not always have the specified length + * @modules java.xml.crypto + */ + +import org.w3c.dom.Document; +import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; + +import javax.xml.crypto.dsig.*; +import javax.xml.crypto.dsig.dom.DOMSignContext; +import javax.xml.crypto.dsig.keyinfo.KeyInfo; +import javax.xml.crypto.dsig.keyinfo.KeyInfoFactory; +import javax.xml.crypto.dsig.keyinfo.X509Data; +import javax.xml.crypto.dsig.keyinfo.X509IssuerSerial; +import javax.xml.crypto.dsig.spec.C14NMethodParameterSpec; +import javax.xml.crypto.dsig.spec.TransformParameterSpec; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.dom.DOMResult; +import javax.xml.transform.dom.DOMSource; +import java.io.StringReader; +import java.math.BigInteger; +import java.security.Provider; +import java.security.Security; +import java.security.*; +import java.security.spec.AlgorithmParameterSpec; +import java.security.spec.PKCS8EncodedKeySpec; +import java.util.*; + +public class ShortECDSA { + + public static final String XML = "\n" + + "\tValue\n" + + ""; + + private static final String PRIVATE_KEY_BASE_64 = + "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgP6yNCRsISznuzY4D" + + "0cwkBjgV8uu2lQ2tCPxdam7Fx9OhRANCAAS33BazN06tOnXsLYatPvmkrEVDyRWj" + + "yzxlCU+en8PPJ4ETUGRhz8h1fAELEkKS0Cky5M61oQVyiSxHaXhunH29"; + + private static final XMLSignatureFactory FAC = + XMLSignatureFactory.getInstance("DOM"); + + public static void main(String[] args) throws Exception { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setNamespaceAware(true); + Document document = factory.newDocumentBuilder(). + parse(new InputSource(new StringReader(XML))); + + PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec( + Base64.getDecoder().decode(PRIVATE_KEY_BASE_64)); + KeyFactory kf = KeyFactory.getInstance("EC"); + PrivateKey privateKey = kf.generatePrivate(keySpec); + + // Remove SunEC so neither its ASN.1 or P1363 format ECDSA will be used + Security.removeProvider("SunEC"); + Security.addProvider(new MyProvider()); + + Document signedDocument = XmlSigningUtils.signDocument(document, privateKey); + NodeList nodeList = signedDocument.getElementsByTagName("SignatureValue"); + byte[] sig = Base64.getMimeDecoder().decode( + nodeList.item(0).getFirstChild().getNodeValue()); + if (sig.length != 64) { + System.out.println("Length: " + sig.length); + System.out.println(HexFormat.ofDelimiter(":").formatHex(sig)); + throw new RuntimeException("Failed"); + } + } + + public static class XmlSigningUtils { + + public static Document signDocument(Document document, PrivateKey privateKey) + throws Exception { + DOMResult result = new DOMResult(); + TransformerFactory.newInstance().newTransformer() + .transform(new DOMSource(document), result); + Document newDocument = (Document) result.getNode(); + FAC.newXMLSignature(buildSignedInfo(), buildKeyInfo()) + .sign(new DOMSignContext(privateKey, newDocument.getDocumentElement())); + return newDocument; + } + + private static SignedInfo buildSignedInfo() + throws NoSuchAlgorithmException, InvalidAlgorithmParameterException { + return FAC.newSignedInfo( + FAC.newCanonicalizationMethod(CanonicalizationMethod.EXCLUSIVE, (C14NMethodParameterSpec) null), + FAC.newSignatureMethod(SignatureMethod.ECDSA_SHA256, null), + List.of(FAC.newReference( + "", + FAC.newDigestMethod(DigestMethod.SHA256, null), + List.of(FAC.newTransform(Transform.ENVELOPED, (TransformParameterSpec) null)), null, null))); + } + + private static KeyInfo buildKeyInfo() { + KeyInfoFactory keyInfoFactory = FAC.getKeyInfoFactory(); + X509IssuerSerial x509IssuerSerial = keyInfoFactory + .newX509IssuerSerial("CN=Me", BigInteger.ONE); + X509Data x509Data = keyInfoFactory.newX509Data(Collections.singletonList(x509IssuerSerial)); + return keyInfoFactory.newKeyInfo(Collections.singletonList(x509Data)); + } + } + + // Only provide SHA256withECDSA, no P1363 format. This triggers the convertASN1toXMLDSIG translation. + public static class MyProvider extends Provider { + MyProvider() { + super("MyProvider", "1", "My provider"); + put("Signature.SHA256withECDSA", MyECDSA.class.getName()); + } + } + + public static class MyECDSA extends SignatureSpi { + + // Hardcoded signature with short r and s + public static byte[] hardcoded; + + static { + hardcoded = new byte[68]; + // Fill in with a number <128 so it will look like a + // correct ASN.1 encoding for positive numbers. + Arrays.fill(hardcoded, (byte) 0x55); + hardcoded[0] = 0x30; // SEQUENCE OF + hardcoded[1] = 66; // 2 [tag,len,31] components + hardcoded[2] = hardcoded[35] = 2; // Each being an INTEGER... + hardcoded[3] = hardcoded[36] = 31; // ... of 31 bytes long + } + + protected void engineInitVerify(PublicKey publicKey) { + } + + protected void engineInitSign(PrivateKey privateKey) { + } + + protected void engineUpdate(byte b) { + } + + protected void engineUpdate(byte[] b, int off, int len) { + } + + protected byte[] engineSign() { + return hardcoded; + } + + protected boolean engineVerify(byte[] sigBytes) { + return false; + } + + protected void engineSetParameter(String param, Object value) { + } + + protected Object engineGetParameter(String param) { + throw new UnsupportedOperationException(); + } + + protected void engineSetParameter(AlgorithmParameterSpec params) { + } + + protected AlgorithmParameters engineGetParameters() { + throw new UnsupportedOperationException(); + } + + protected int engineSign(byte[] outbuf, int offset, int len) { + System.arraycopy(hardcoded, 0, outbuf, offset, hardcoded.length); + return hardcoded.length; + } + } +}