[java-opensaml] branch maint-4 updated: OSJ-355: ConcatKDF parameter requirements too restrictive in ECDH
Brent Putman
putmanb at georgetown.edu
Thu Jun 30 03:31:12 UTC 2022
This is an automated email from the git hooks/post-receive script.
putmanb pushed a commit to branch maint-4
in repository java-opensaml.
View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=421e998d37347aaf008f50475273e7cea9258407
The following commit(s) were added to refs/heads/maint-4 by this push:
new 421e998d3 OSJ-355: ConcatKDF parameter requirements too restrictive in ECDH
421e998d3 is described below
commit 421e998d37347aaf008f50475273e7cea9258407
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Wed Jun 29 22:39:52 2022 -0400
OSJ-355: ConcatKDF parameter requirements too restrictive in ECDH
---
.../opensaml/xmlsec/derivation/impl/ConcatKDF.java | 24 +++--
.../xmlsec/derivation/impl/ConcatKDFTest.java | 107 ++++++++++++++++++++-
2 files changed, 119 insertions(+), 12 deletions(-)
diff --git a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/derivation/impl/ConcatKDF.java b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/derivation/impl/ConcatKDF.java
index bbecdcf68..25e98faca 100644
--- a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/derivation/impl/ConcatKDF.java
+++ b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/derivation/impl/ConcatKDF.java
@@ -392,8 +392,8 @@ public class ConcatKDF extends AbstractInitializableComponent
* XML Encryption 1.1.
*
* <p>
- * No syntactic validation is done on the input value. Since only whole byte-aligned values are not supported,
- * this method merely pre-pends "00" to indicate 0 padding bits.
+ * No syntactic validation is done on the input value. Since only whole byte-aligned values are supported,
+ * this method merely prepends "00" to indicate 0 padding bits.
* </p>
*
* @param value the value to process
@@ -416,7 +416,7 @@ public class ConcatKDF extends AbstractInitializableComponent
* for input to the derivation operation.
*
* <p>
- * Since only whole byte-aligned values supported, this method required input values to begin with "00",
+ * Since only whole byte-aligned values are supported, this method requires input values to begin with "00",
* indicating 0 padding bits.
* </p>
*
@@ -435,18 +435,24 @@ public class ConcatKDF extends AbstractInitializableComponent
return null;
}
+ if (trimmed.length() < 2) {
+ throw new KeyDerivationException("ConcatKDF parameter was not a valid padded hexBinary value "
+ + "(too short): " + name);
+ }
+ if (trimmed.length() % 2 != 0) {
+ throw new KeyDerivationException("ConcatKDF parameter was not a valid padded hexBinary value "
+ + "(odd number of hex digits): " + name);
+ }
+
// We only support whole byte-aligned values, so # of padding bits must always be 0
if (!trimmed.startsWith("00")) {
throw new KeyDerivationException("ConcatKDF parameter was not a valid padded hexBinary value "
+ "(non-byte-aligned): " + name);
}
- // Minimum valid padded length would be 2 bytes (4 hex digits): 1 for # of padding bits, 1+ for data
- if (trimmed.length() < 4) {
- throw new KeyDerivationException("ConcatKDF parameter was not a valid padded hexBinary value (too short): "
- + name);
- }
-
+ // As of OSJ-355, we treat "00" as a legal value, representing an empty bitstring.
+ // The following will return "" in that case, which is ok.
+
return trimmed.substring(2);
}
diff --git a/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/derivation/impl/ConcatKDFTest.java b/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/derivation/impl/ConcatKDFTest.java
index d2ef571ee..f728e6c06 100644
--- a/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/derivation/impl/ConcatKDFTest.java
+++ b/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/derivation/impl/ConcatKDFTest.java
@@ -159,7 +159,7 @@ public class ConcatKDFTest extends XMLObjectBaseTestCase {
}
@Test
- public void fromXMLObject() throws Exception {
+ public void fromXMLObject_Basic() throws Exception {
KeyDerivationMethod xmlKDM = buildXMLObject(KeyDerivationMethod.DEFAULT_ELEMENT_NAME);
xmlKDM.setAlgorithm(EncryptionConstants.ALGO_ID_KEYDERIVATION_CONCATKDF);
@@ -179,7 +179,105 @@ public class ConcatKDFTest extends XMLObjectBaseTestCase {
ConcatKDF parameter = ConcatKDF.fromXMLObject(xmlKDM);
Assert.assertNotNull(parameter);
Assert.assertTrue(parameter.isInitialized());
+
+ Assert.assertEquals(parameter.getAlgorithm(), EncryptionConstants.ALGO_ID_KEYDERIVATION_CONCATKDF);
+ Assert.assertEquals(parameter.getDigestMethod(), SignatureConstants.ALGO_ID_DIGEST_SHA256);
+
+ Assert.assertEquals(parameter.getAlgorithmID(), "AA");
+ Assert.assertEquals(parameter.getPartyUInfo(), "BB");
+ Assert.assertEquals(parameter.getPartyVInfo(), "CC");
+ Assert.assertEquals(parameter.getSuppPubInfo(), "DD");
+ Assert.assertEquals(parameter.getSuppPrivInfo(), "EE");
+ }
+
+ @Test
+ public void fromXMLObject_NullBitstrings() throws Exception {
+ KeyDerivationMethod xmlKDM = buildXMLObject(KeyDerivationMethod.DEFAULT_ELEMENT_NAME);
+ xmlKDM.setAlgorithm(EncryptionConstants.ALGO_ID_KEYDERIVATION_CONCATKDF);
+
+ ConcatKDFParams xmlParams= buildXMLObject(ConcatKDFParams.DEFAULT_ELEMENT_NAME);
+ xmlKDM.getUnknownXMLObjects().add(xmlParams);
+
+ DigestMethod xmlDigest = buildXMLObject(DigestMethod.DEFAULT_ELEMENT_NAME);
+ xmlDigest.setAlgorithm(SignatureConstants.ALGO_ID_DIGEST_SHA256);
+ xmlParams.setDigestMethod(xmlDigest);
+
+ xmlParams.setAlgorithmID(null);
+ xmlParams.setPartyUInfo(null);
+ xmlParams.setPartyVInfo(null);
+ xmlParams.setSuppPubInfo(null);
+ xmlParams.setSuppPrivInfo(null);
+
+ ConcatKDF parameter = ConcatKDF.fromXMLObject(xmlKDM);
+ Assert.assertNotNull(parameter);
+ Assert.assertTrue(parameter.isInitialized());
+
+ Assert.assertEquals(parameter.getAlgorithm(), EncryptionConstants.ALGO_ID_KEYDERIVATION_CONCATKDF);
+ Assert.assertEquals(parameter.getDigestMethod(), SignatureConstants.ALGO_ID_DIGEST_SHA256);
+
+ Assert.assertEquals(parameter.getAlgorithmID(), null);
+ Assert.assertEquals(parameter.getPartyUInfo(), null);
+ Assert.assertEquals(parameter.getPartyVInfo(), null);
+ Assert.assertEquals(parameter.getSuppPubInfo(), null);
+ Assert.assertEquals(parameter.getSuppPrivInfo(), null);
+ }
+
+ @Test
+ public void fromXMLObject_EmptyBitstrings_OSJ_355() throws Exception {
+ KeyDerivationMethod xmlKDM = buildXMLObject(KeyDerivationMethod.DEFAULT_ELEMENT_NAME);
+ xmlKDM.setAlgorithm(EncryptionConstants.ALGO_ID_KEYDERIVATION_CONCATKDF);
+
+ ConcatKDFParams xmlParams= buildXMLObject(ConcatKDFParams.DEFAULT_ELEMENT_NAME);
+ xmlKDM.getUnknownXMLObjects().add(xmlParams);
+
+ DigestMethod xmlDigest = buildXMLObject(DigestMethod.DEFAULT_ELEMENT_NAME);
+ xmlDigest.setAlgorithm(SignatureConstants.ALGO_ID_DIGEST_SHA256);
+ xmlParams.setDigestMethod(xmlDigest);
+ xmlParams.setAlgorithmID("00");
+ xmlParams.setPartyUInfo("00");
+ xmlParams.setPartyVInfo("00");
+ xmlParams.setSuppPubInfo("00");
+ xmlParams.setSuppPrivInfo("00");
+
+ ConcatKDF parameter = ConcatKDF.fromXMLObject(xmlKDM);
+ Assert.assertNotNull(parameter);
+ Assert.assertTrue(parameter.isInitialized());
+
+ Assert.assertEquals(parameter.getAlgorithm(), EncryptionConstants.ALGO_ID_KEYDERIVATION_CONCATKDF);
+ Assert.assertEquals(parameter.getDigestMethod(), SignatureConstants.ALGO_ID_DIGEST_SHA256);
+
+ Assert.assertEquals(parameter.getAlgorithmID(), null);
+ Assert.assertEquals(parameter.getPartyUInfo(), null);
+ Assert.assertEquals(parameter.getPartyVInfo(), null);
+ Assert.assertEquals(parameter.getSuppPubInfo(), null);
+ Assert.assertEquals(parameter.getSuppPrivInfo(), null);
+ }
+
+ @Test
+ public void fromXMLObject_InvalidValues() throws Exception {
+ KeyDerivationMethod xmlKDM = buildXMLObject(KeyDerivationMethod.DEFAULT_ELEMENT_NAME);
+ xmlKDM.setAlgorithm(EncryptionConstants.ALGO_ID_KEYDERIVATION_CONCATKDF);
+
+ ConcatKDFParams xmlParams= buildXMLObject(ConcatKDFParams.DEFAULT_ELEMENT_NAME);
+ xmlKDM.getUnknownXMLObjects().add(xmlParams);
+
+ DigestMethod xmlDigest = buildXMLObject(DigestMethod.DEFAULT_ELEMENT_NAME);
+ xmlDigest.setAlgorithm(SignatureConstants.ALGO_ID_DIGEST_SHA256);
+ xmlParams.setDigestMethod(xmlDigest);
+
+ xmlParams.setAlgorithmID("00AA");
+ xmlParams.setPartyUInfo("00BB");
+ xmlParams.setPartyVInfo("00CC");
+ xmlParams.setSuppPubInfo("00DD");
+ xmlParams.setSuppPrivInfo("00EE");
+
+ ConcatKDF parameter = ConcatKDF.fromXMLObject(xmlKDM);
+ Assert.assertNotNull(parameter);
+ Assert.assertTrue(parameter.isInitialized());
+
+ // Testing various invalid values
+
KeyDerivationMethod xmlKDMBad = null;
ConcatKDFParams xmlParamsBad = null;
@@ -409,6 +507,9 @@ public class ConcatKDFTest extends XMLObjectBaseTestCase {
Assert.assertEquals(ConcatKDF.unpadParam("00AA", "test"), "AA");
Assert.assertEquals(ConcatKDF.unpadParam(" 00AABBCC ", "test"), "AABBCC");
+
+ // Considered valid per OSJ-355
+ Assert.assertEquals(ConcatKDF.unpadParam("00", "test"), "");
try {
// Unsupported padding
@@ -420,13 +521,13 @@ public class ConcatKDFTest extends XMLObjectBaseTestCase {
try {
// Too short
- ConcatKDF.unpadParam("00", "test");
+ ConcatKDF.unpadParam("0", "test");
Assert.fail("Invalid value should have failed");
} catch (KeyDerivationException eA ) {
//expected
}
try {
- // Too short
+ // Odd number of hex digits
ConcatKDF.unpadParam("00A", "test");
Assert.fail("Invalid value should have failed");
} catch (KeyDerivationException eA ) {
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list