From 2ba224d3edf6cfb524b69cc88a92d1b74a21bf15 Mon Sep 17 00:00:00 2001 From: mullan Date: Tue, 23 Jun 2009 13:54:36 -0400 Subject: [PATCH] 6824440: XML Signature HMAC issue Reviewed-by: asaha --- .../implementations/IntegrityHmac.java | 76 +++++++---- .../internal/dom/DOMHMACSignatureMethod.java | 53 ++++---- .../xml/internal/security/TruncateHMAC.java | 118 ++++++++++++++++++ ...enveloping-hmac-sha1-trunclen-0-attack.xml | 16 +++ ...enveloping-hmac-sha1-trunclen-8-attack.xml | 17 +++ .../xml/crypto/dsig/GenerationTests.java | 16 ++- .../xml/crypto/dsig/ValidationTests.java | 39 +++++- ...enveloping-hmac-sha1-trunclen-0-attack.xml | 16 +++ ...enveloping-hmac-sha1-trunclen-8-attack.xml | 17 +++ 9 files changed, 311 insertions(+), 57 deletions(-) create mode 100644 test/com/sun/org/apache/xml/internal/security/TruncateHMAC.java create mode 100644 test/com/sun/org/apache/xml/internal/security/signature-enveloping-hmac-sha1-trunclen-0-attack.xml create mode 100644 test/com/sun/org/apache/xml/internal/security/signature-enveloping-hmac-sha1-trunclen-8-attack.xml create mode 100644 test/javax/xml/crypto/dsig/data/signature-enveloping-hmac-sha1-trunclen-0-attack.xml create mode 100644 test/javax/xml/crypto/dsig/data/signature-enveloping-hmac-sha1-trunclen-8-attack.xml diff --git a/src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/IntegrityHmac.java b/src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/IntegrityHmac.java index d3495bb56..85cc9e38a 100644 --- a/src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/IntegrityHmac.java +++ b/src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/IntegrityHmac.java @@ -60,8 +60,14 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { */ public abstract String engineGetURI(); + /** + * Returns the output length of the hash/digest. + */ + abstract int getDigestLength(); + /** Field _macAlgorithm */ private Mac _macAlgorithm = null; + private boolean _HMACOutputLengthSet = false; /** Field _HMACOutputLength */ int _HMACOutputLength = 0; @@ -115,14 +121,16 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { throws XMLSignatureException { try { - byte[] completeResult = this._macAlgorithm.doFinal(); - - if ((this._HMACOutputLength == 0) || (this._HMACOutputLength >= 160)) { + if (this._HMACOutputLengthSet && this._HMACOutputLength < getDigestLength()) { + if (log.isLoggable(java.util.logging.Level.FINE)) { + log.log(java.util.logging.Level.FINE, + "HMACOutputLength must not be less than " + getDigestLength()); + } + throw new XMLSignatureException("errorMessages.XMLSignatureException"); + } else { + byte[] completeResult = this._macAlgorithm.doFinal(); return MessageDigestAlgorithm.isEqual(completeResult, signature); } - byte[] stripped = IntegrityHmac.reduceBitLength(completeResult, - this._HMACOutputLength); - return MessageDigestAlgorithm.isEqual(stripped, signature); } catch (IllegalStateException ex) { throw new XMLSignatureException("empty", ex); } @@ -176,14 +184,15 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { protected byte[] engineSign() throws XMLSignatureException { try { - byte[] completeResult = this._macAlgorithm.doFinal(); - - if ((this._HMACOutputLength == 0) || (this._HMACOutputLength >= 160)) { - return completeResult; + if (this._HMACOutputLengthSet && this._HMACOutputLength < getDigestLength()) { + if (log.isLoggable(java.util.logging.Level.FINE)) { + log.log(java.util.logging.Level.FINE, + "HMACOutputLength must not be less than " + getDigestLength()); + } + throw new XMLSignatureException("errorMessages.XMLSignatureException"); + } else { + return this._macAlgorithm.doFinal(); } - return IntegrityHmac.reduceBitLength(completeResult, - this._HMACOutputLength); - } catch (IllegalStateException ex) { throw new XMLSignatureException("empty", ex); } @@ -361,6 +370,7 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { */ protected void engineSetHMACOutputLength(int HMACOutputLength) { this._HMACOutputLength = HMACOutputLength; + this._HMACOutputLengthSet = true; } /** @@ -376,12 +386,13 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { throw new IllegalArgumentException("element null"); } - Text hmaclength =XMLUtils.selectDsNodeText(element.getFirstChild(), - Constants._TAG_HMACOUTPUTLENGTH,0); + Text hmaclength =XMLUtils.selectDsNodeText(element.getFirstChild(), + Constants._TAG_HMACOUTPUTLENGTH,0); - if (hmaclength != null) { - this._HMACOutputLength = Integer.parseInt(hmaclength.getData()); - } + if (hmaclength != null) { + this._HMACOutputLength = Integer.parseInt(hmaclength.getData()); + this._HMACOutputLengthSet = true; + } } @@ -390,14 +401,13 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { * * @param element */ - public void engineAddContextToElement(Element element) - { + public void engineAddContextToElement(Element element) { if (element == null) { throw new IllegalArgumentException("null element"); } - if (this._HMACOutputLength != 0) { + if (this._HMACOutputLengthSet) { Document doc = element.getOwnerDocument(); Element HMElem = XMLUtils.createElementInSignatureSpace(doc, Constants._TAG_HMACOUTPUTLENGTH); @@ -436,6 +446,10 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { public String engineGetURI() { return XMLSignature.ALGO_ID_MAC_HMAC_SHA1; } + + int getDigestLength() { + return 160; + } } /** @@ -463,6 +477,10 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { public String engineGetURI() { return XMLSignature.ALGO_ID_MAC_HMAC_SHA256; } + + int getDigestLength() { + return 256; + } } /** @@ -490,6 +508,10 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { public String engineGetURI() { return XMLSignature.ALGO_ID_MAC_HMAC_SHA384; } + + int getDigestLength() { + return 384; + } } /** @@ -517,6 +539,10 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { public String engineGetURI() { return XMLSignature.ALGO_ID_MAC_HMAC_SHA512; } + + int getDigestLength() { + return 512; + } } /** @@ -544,6 +570,10 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { public String engineGetURI() { return XMLSignature.ALGO_ID_MAC_HMAC_RIPEMD160; } + + int getDigestLength() { + return 160; + } } /** @@ -571,5 +601,9 @@ public abstract class IntegrityHmac extends SignatureAlgorithmSpi { public String engineGetURI() { return XMLSignature.ALGO_ID_MAC_HMAC_NOT_RECOMMENDED_MD5; } + + int getDigestLength() { + return 128; + } } } diff --git a/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMHMACSignatureMethod.java b/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMHMACSignatureMethod.java index 5a9000c89..482283b61 100644 --- a/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMHMACSignatureMethod.java +++ b/src/share/classes/org/jcp/xml/dsig/internal/dom/DOMHMACSignatureMethod.java @@ -19,7 +19,7 @@ * */ /* - * Copyright 2005-2008 Sun Microsystems, Inc. All rights reserved. + * Copyright 2005-2009 Sun Microsystems, Inc. All rights reserved. */ /* * $Id: DOMHMACSignatureMethod.java,v 1.2 2008/07/24 15:20:32 mullan Exp $ @@ -58,6 +58,7 @@ public abstract class DOMHMACSignatureMethod extends DOMSignatureMethod { Logger.getLogger("org.jcp.xml.dsig.internal.dom"); private Mac hmac; private int outputLength; + private boolean outputLengthSet; /** * Creates a DOMHMACSignatureMethod with the specified params @@ -87,6 +88,7 @@ public abstract class DOMHMACSignatureMethod extends DOMSignatureMethod { ("params must be of type HMACParameterSpec"); } outputLength = ((HMACParameterSpec) params).getOutputLength(); + outputLengthSet = true; if (log.isLoggable(Level.FINE)) { log.log(Level.FINE, "Setting outputLength from HMACParameterSpec to: " @@ -101,6 +103,7 @@ public abstract class DOMHMACSignatureMethod extends DOMSignatureMethod { throws MarshalException { outputLength = new Integer (paramsElem.getFirstChild().getNodeValue()).intValue(); + outputLengthSet = true; if (log.isLoggable(Level.FINE)) { log.log(Level.FINE, "unmarshalled outputLength: " + outputLength); } @@ -135,23 +138,13 @@ public abstract class DOMHMACSignatureMethod extends DOMSignatureMethod { throw new XMLSignatureException(nsae); } } - if (log.isLoggable(Level.FINE)) { - log.log(Level.FINE, "outputLength = " + outputLength); + if (outputLengthSet && outputLength < getDigestLength()) { + throw new XMLSignatureException + ("HMACOutputLength must not be less than " + getDigestLength()); } hmac.init((SecretKey) key); si.canonicalize(context, new MacOutputStream(hmac)); byte[] result = hmac.doFinal(); - if (log.isLoggable(Level.FINE)) { - log.log(Level.FINE, "resultLength = " + result.length); - } - if (outputLength != -1) { - int byteLength = outputLength/8; - if (result.length > byteLength) { - byte[] truncated = new byte[byteLength]; - System.arraycopy(result, 0, truncated, 0, byteLength); - result = truncated; - } - } return MessageDigest.isEqual(sig, result); } @@ -171,18 +164,13 @@ public abstract class DOMHMACSignatureMethod extends DOMSignatureMethod { throw new XMLSignatureException(nsae); } } + if (outputLengthSet && outputLength < getDigestLength()) { + throw new XMLSignatureException + ("HMACOutputLength must not be less than " + getDigestLength()); + } hmac.init((SecretKey) key); si.canonicalize(context, new MacOutputStream(hmac)); - byte[] result = hmac.doFinal(); - if (outputLength != -1) { - int byteLength = outputLength/8; - if (result.length > byteLength) { - byte[] truncated = new byte[byteLength]; - System.arraycopy(result, 0, truncated, 0, byteLength); - result = truncated; - } - } - return result; + return hmac.doFinal(); } boolean paramsEqual(AlgorithmParameterSpec spec) { @@ -197,6 +185,11 @@ public abstract class DOMHMACSignatureMethod extends DOMSignatureMethod { return (outputLength == ospec.getOutputLength()); } + /** + * Returns the output length of the hash/digest. + */ + abstract int getDigestLength(); + static final class SHA1 extends DOMHMACSignatureMethod { SHA1(AlgorithmParameterSpec params) throws InvalidAlgorithmParameterException { @@ -211,6 +204,9 @@ public abstract class DOMHMACSignatureMethod extends DOMSignatureMethod { String getSignatureAlgorithm() { return "HmacSHA1"; } + int getDigestLength() { + return 160; + } } static final class SHA256 extends DOMHMACSignatureMethod { @@ -227,6 +223,9 @@ public abstract class DOMHMACSignatureMethod extends DOMSignatureMethod { String getSignatureAlgorithm() { return "HmacSHA256"; } + int getDigestLength() { + return 256; + } } static final class SHA384 extends DOMHMACSignatureMethod { @@ -243,6 +242,9 @@ public abstract class DOMHMACSignatureMethod extends DOMSignatureMethod { String getSignatureAlgorithm() { return "HmacSHA384"; } + int getDigestLength() { + return 384; + } } static final class SHA512 extends DOMHMACSignatureMethod { @@ -259,5 +261,8 @@ public abstract class DOMHMACSignatureMethod extends DOMSignatureMethod { String getSignatureAlgorithm() { return "HmacSHA512"; } + int getDigestLength() { + return 512; + } } } diff --git a/test/com/sun/org/apache/xml/internal/security/TruncateHMAC.java b/test/com/sun/org/apache/xml/internal/security/TruncateHMAC.java new file mode 100644 index 000000000..ebb424de3 --- /dev/null +++ b/test/com/sun/org/apache/xml/internal/security/TruncateHMAC.java @@ -0,0 +1,118 @@ +/* + * Copyright 2009 Sun Microsystems, Inc. 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 Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, + * CA 95054 USA or visit www.sun.com if you need additional information or + * have any questions. + */ + +/** + * @test %I% %E% + * @bug 6824440 + * @summary Check that Apache XMLSec APIs will not accept HMAC truncation + * lengths less than minimum bound + * @compile -XDignore.symbol.file TruncateHMAC.java + * @run main TruncateHMAC + */ + +import java.io.File; +import javax.crypto.SecretKey; +import javax.xml.parsers.DocumentBuilderFactory; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NodeList; + +import com.sun.org.apache.xml.internal.security.Init; +import com.sun.org.apache.xml.internal.security.c14n.Canonicalizer; +import com.sun.org.apache.xml.internal.security.signature.XMLSignature; +import com.sun.org.apache.xml.internal.security.signature.XMLSignatureException; +import com.sun.org.apache.xml.internal.security.utils.Constants; + + +public class TruncateHMAC { + + private final static String DIR = System.getProperty("test.src", "."); + private static DocumentBuilderFactory dbf = null; + private static boolean atLeastOneFailed = false; + + public static void main(String[] args) throws Exception { + + Init.init(); + dbf = DocumentBuilderFactory.newInstance(); + dbf.setNamespaceAware(true); + dbf.setValidating(false); + validate("signature-enveloping-hmac-sha1-trunclen-0-attack.xml"); + validate("signature-enveloping-hmac-sha1-trunclen-8-attack.xml"); + generate_hmac_sha1_40(); + + if (atLeastOneFailed) { + throw new Exception + ("At least one signature did not validate as expected"); + } + } + + private static void validate(String data) throws Exception { + System.out.println("Validating " + data); + File file = new File(DIR, data); + + Document doc = dbf.newDocumentBuilder().parse(file); + NodeList nl = + doc.getElementsByTagNameNS(Constants.SignatureSpecNS, "Signature"); + if (nl.getLength() == 0) { + throw new Exception("Couldn't find signature Element"); + } + Element sigElement = (Element) nl.item(0); + XMLSignature signature = new XMLSignature + (sigElement, file.toURI().toString()); + SecretKey sk = signature.createSecretKey("secret".getBytes("ASCII")); + try { + System.out.println + ("Validation status: " + signature.checkSignatureValue(sk)); + System.out.println("FAILED"); + atLeastOneFailed = true; + } catch (XMLSignatureException xse) { + System.out.println(xse.getMessage()); + System.out.println("PASSED"); + } + } + + private static void generate_hmac_sha1_40() throws Exception { + System.out.println("Generating "); + + Document doc = dbf.newDocumentBuilder().newDocument(); + XMLSignature sig = new XMLSignature + (doc, null, XMLSignature.ALGO_ID_MAC_HMAC_SHA1, 40, + Canonicalizer.ALGO_ID_C14N_OMIT_COMMENTS); + try { + sig.sign(getSecretKey("secret".getBytes("ASCII"))); + System.out.println("FAILED"); + atLeastOneFailed = true; + } catch (XMLSignatureException xse) { + System.out.println(xse.getMessage()); + System.out.println("PASSED"); + } + } + + private static SecretKey getSecretKey(final byte[] secret) { + return new SecretKey() { + public String getFormat() { return "RAW"; } + public byte[] getEncoded() { return secret; } + public String getAlgorithm(){ return "SECRET"; } + }; + } +} diff --git a/test/com/sun/org/apache/xml/internal/security/signature-enveloping-hmac-sha1-trunclen-0-attack.xml b/test/com/sun/org/apache/xml/internal/security/signature-enveloping-hmac-sha1-trunclen-0-attack.xml new file mode 100644 index 000000000..5008dae51 --- /dev/null +++ b/test/com/sun/org/apache/xml/internal/security/signature-enveloping-hmac-sha1-trunclen-0-attack.xml @@ -0,0 +1,16 @@ + + + + + + 0 + + + + nz4GS0NbH2SrWlD/4fX313CoTzc= + + + + + some other text + diff --git a/test/com/sun/org/apache/xml/internal/security/signature-enveloping-hmac-sha1-trunclen-8-attack.xml b/test/com/sun/org/apache/xml/internal/security/signature-enveloping-hmac-sha1-trunclen-8-attack.xml new file mode 100644 index 000000000..dfe0a0df9 --- /dev/null +++ b/test/com/sun/org/apache/xml/internal/security/signature-enveloping-hmac-sha1-trunclen-8-attack.xml @@ -0,0 +1,17 @@ + + + + + + 8 + + + + nz4GS0NbH2SrWlD/4fX313CoTzc= + + + + Qw== + + some other text + diff --git a/test/javax/xml/crypto/dsig/GenerationTests.java b/test/javax/xml/crypto/dsig/GenerationTests.java index ea4fd610e..13881966d 100644 --- a/test/javax/xml/crypto/dsig/GenerationTests.java +++ b/test/javax/xml/crypto/dsig/GenerationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2005-2008 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2005-2009 Sun Microsystems, Inc. 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 @@ -23,9 +23,7 @@ /** * @test - * @bug 4635230 - * @bug 6283345 - * @bug 6303830 + * @bug 4635230 6283345 6303830 6824440 * @summary Basic unit tests for generating XML Signatures with JSR 105 * @compile -XDignore.symbol.file KeySelectors.java SignatureValidator.java * X509KeySelector.java GenerationTests.java @@ -248,8 +246,14 @@ public class GenerationTests { System.out.println("* Generating signature-enveloping-hmac-sha1-40.xml"); SignatureMethod hmacSha1 = fac.newSignatureMethod (SignatureMethod.HMAC_SHA1, new HMACParameterSpec(40)); - test_create_signature_enveloping(sha1, hmacSha1, null, - getSecretKey("secret".getBytes("ASCII")), sks, false); + try { + test_create_signature_enveloping(sha1, hmacSha1, null, + getSecretKey("secret".getBytes("ASCII")), sks, false); + } catch (Exception e) { + if (!(e instanceof XMLSignatureException)) { + throw e; + } + } System.out.println(); } diff --git a/test/javax/xml/crypto/dsig/ValidationTests.java b/test/javax/xml/crypto/dsig/ValidationTests.java index 0934de7e9..0604af9bb 100644 --- a/test/javax/xml/crypto/dsig/ValidationTests.java +++ b/test/javax/xml/crypto/dsig/ValidationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2005-2007 Sun Microsystems, Inc. All Rights Reserved. + * Copyright 2005-2009 Sun Microsystems, Inc. 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 @@ -23,9 +23,7 @@ /** * @test - * @bug 4635230 - * @bug 6365103 - * @bug 6366054 + * @bug 4635230 6365103 6366054 6824440 * @summary Basic unit tests for validating XML Signatures with JSR 105 * @compile -XDignore.symbol.file KeySelectors.java SignatureValidator.java * X509KeySelector.java ValidationTests.java @@ -42,6 +40,7 @@ import javax.xml.crypto.URIDereferencer; import javax.xml.crypto.URIReference; import javax.xml.crypto.URIReferenceException; import javax.xml.crypto.XMLCryptoContext; +import javax.xml.crypto.dsig.XMLSignatureException; import javax.xml.crypto.dsig.XMLSignatureFactory; /** @@ -68,7 +67,6 @@ public class ValidationTests { "signature-enveloping-dsa.xml", "signature-enveloping-rsa.xml", "signature-enveloping-hmac-sha1.xml", - "signature-enveloping-hmac-sha1-40.xml", "signature-external-dsa.xml", "signature-external-b64-dsa.xml", "signature-retrievalmethod-rawx509crt.xml", @@ -106,7 +104,6 @@ public class ValidationTests { KVKS, KVKS, SKKS, - SKKS, KVKS, KVKS, CKS, @@ -146,6 +143,36 @@ public class ValidationTests { atLeastOneFailed = true; } + System.out.println("Validating signature-enveloping-hmac-sha1-40.xml"); + try { + test_signature("signature-enveloping-hmac-sha1-40.xml", SKKS, false); + System.out.println("FAILED"); + atLeastOneFailed = true; + } catch (XMLSignatureException xse) { + System.out.println(xse.getMessage()); + System.out.println("PASSED"); + } + + System.out.println("Validating signature-enveloping-hmac-sha1-trunclen-0-attack.xml"); + try { + test_signature("signature-enveloping-hmac-sha1-trunclen-0-attack.xml", SKKS, false); + System.out.println("FAILED"); + atLeastOneFailed = true; + } catch (XMLSignatureException xse) { + System.out.println(xse.getMessage()); + System.out.println("PASSED"); + } + + System.out.println("Validating signature-enveloping-hmac-sha1-trunclen-8-attack.xml"); + try { + test_signature("signature-enveloping-hmac-sha1-trunclen-8-attack.xml", SKKS, false); + System.out.println("FAILED"); + atLeastOneFailed = true; + } catch (XMLSignatureException xse) { + System.out.println(xse.getMessage()); + System.out.println("PASSED"); + } + if (atLeastOneFailed) { throw new Exception ("At least one signature did not validate as expected"); diff --git a/test/javax/xml/crypto/dsig/data/signature-enveloping-hmac-sha1-trunclen-0-attack.xml b/test/javax/xml/crypto/dsig/data/signature-enveloping-hmac-sha1-trunclen-0-attack.xml new file mode 100644 index 000000000..5008dae51 --- /dev/null +++ b/test/javax/xml/crypto/dsig/data/signature-enveloping-hmac-sha1-trunclen-0-attack.xml @@ -0,0 +1,16 @@ + + + + + + 0 + + + + nz4GS0NbH2SrWlD/4fX313CoTzc= + + + + + some other text + diff --git a/test/javax/xml/crypto/dsig/data/signature-enveloping-hmac-sha1-trunclen-8-attack.xml b/test/javax/xml/crypto/dsig/data/signature-enveloping-hmac-sha1-trunclen-8-attack.xml new file mode 100644 index 000000000..dfe0a0df9 --- /dev/null +++ b/test/javax/xml/crypto/dsig/data/signature-enveloping-hmac-sha1-trunclen-8-attack.xml @@ -0,0 +1,17 @@ + + + + + + 8 + + + + nz4GS0NbH2SrWlD/4fX313CoTzc= + + + + Qw== + + some other text + -- GitLab