SANTUARIO-511: Implementation of the Diffie-Hellman-ES key exchange for EC and XEC keys

src/main/java/org/apache/xml/security/encryption/params/ConcatKeyDerivationParameter.java

@@ -28,12 +28,12 @@
  */
 public class ConcatKeyDerivationParameter extends KeyDerivationParameter {
 
-    String digestAlgorithm;
-    String algorithmID;
-    String partyUInfo;
-    String partyVInfo;
-    String suppPubInfo;
-    String suppPrivInfo;
+    private String digestAlgorithm;
+    private String algorithmID;
+    private String partyUInfo;
+    private String partyVInfo;
+    private String suppPubInfo;
+    private String suppPrivInfo;
 
     /**
      * Constructor ConcatKeyDerivationParameter with default SHA256 digest algorithm

src/main/java/org/apache/xml/security/encryption/params/KeyAgreementParameterSpec.java

@@ -36,14 +36,14 @@ public enum ActorType {
         RECIPIENT
     }
 
-    KeyDerivationParameter KeyDerivationParameter;
-    ActorType actorType;
-    String keyAgreementAlgorithm;
-    PublicKey originatorPublicKey;
-    PrivateKey originatorPrivateKey;
-
-    PublicKey recipientPublicKey;
-    PrivateKey recipientPrivateKey;
+    private final KeyDerivationParameter KeyDerivationParameter;
+    private final  ActorType actorType;
+    private final String keyAgreementAlgorithm;
+
+    private PublicKey originatorPublicKey;
+    private PrivateKey originatorPrivateKey;
+    private PublicKey recipientPublicKey;
+    private PrivateKey recipientPrivateKey;
 
 
     public KeyAgreementParameterSpec(ActorType actorType, String keyAgreementAlgorithm, KeyDerivationParameter keyDerivationParameter) {

src/main/java/org/apache/xml/security/encryption/params/KeyDerivationParameter.java

@@ -24,8 +24,8 @@
  * This class contains the basic parameters used for key derivation.
  */
 public class KeyDerivationParameter implements AlgorithmParameterSpec {
-    protected final String algorithm;
-    protected final int keyBitLength;
+    private final String algorithm;
+    private final int keyBitLength;
 
     public KeyDerivationParameter(String algorithm, int keyLength) {
         this.algorithm = algorithm;

src/main/java/org/apache/xml/security/keys/KeyInfo.java

@@ -42,11 +42,7 @@
 import org.apache.xml.security.keys.keyresolver.KeyResolverSpi;
 import org.apache.xml.security.keys.storage.StorageResolver;
 import org.apache.xml.security.transforms.Transforms;
-import org.apache.xml.security.utils.Constants;
-import org.apache.xml.security.utils.ElementProxy;
-import org.apache.xml.security.utils.EncryptionConstants;
-import org.apache.xml.security.utils.SignatureElementProxy;
-import org.apache.xml.security.utils.XMLUtils;
+import org.apache.xml.security.utils.*;
 import org.w3c.dom.Attr;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -367,13 +363,20 @@ public void add(DEREncodedKeyValue derEncodedKeyValue) {
     }
 
     /**
-     * Method add
+     * Method add AgreementMethod to the KeyInfo
      *
-     * @param agreementMethod
+     * @param agreementMethod the AgreementMethod to be added. The AgreementMethod must extend
+     *                        class {@link org.apache.xml.security.utils.ElementProxy}
      */
-    public void add(AgreementMethodImpl agreementMethod) {
-        appendSelf(agreementMethod);
-        addReturnToSelf();
+    public void add(AgreementMethod agreementMethod)  {
+
+        if (agreementMethod instanceof ElementProxy) {
+            appendSelf((ElementProxy)agreementMethod);
+            addReturnToSelf();
+        } else {
+            Object[] exArgs = {EncryptionConstants._TAG_AGREEMENTMETHOD, agreementMethod.getClass().getName()};
+            throw new IllegalArgumentException(I18n.translate("KeyValue.IllegalArgument", exArgs));
+        }
     }
 
     /**

src/main/java/org/apache/xml/security/keys/content/AgreementMethodImpl.java

@@ -187,9 +187,9 @@ public void setKeyDerivationMethod(KeyDerivationMethod keyDerivationMethod) {
             appendSelf((ElementProxy) keyDerivationMethod);
             addReturnToSelf();
         } else {
-            LOG.log(Level.WARNING, "KeyDerivationMethod is set but is not an instance of ElementProxy. The DOM node is lost upon serialization.");
+            LOG.log(Level.WARNING, "KeyDerivationMethod [{0}] is set but is not an instance of ElementProxy. " +
+                    "The DOM node is lost upon serialization.", keyDerivationMethod);
         }
-
     }
 
     /**

src/main/java/org/apache/xml/security/keys/content/DEREncodedKeyValue.java

@@ -39,7 +39,7 @@ public class DEREncodedKeyValue extends Signature11ElementProxy implements KeyIn
     /** JCA algorithm key types supported by this implementation. */
     private static final String[] supportedKeyTypes = { "RSA", "DSA", "EC",
             "XDH", "X25519", "X448",
-            "EdDSA", "Ed25519,", "Ed448"};
+            "EdDSA", "Ed25519", "Ed448"};
 
     /**
      * Constructor DEREncodedKeyValue

src/main/java/org/apache/xml/security/keys/derivedKey/ConcatKDF.java

@@ -41,7 +41,7 @@ public class ConcatKDF implements DerivationAlgorithm {
 
     private static final System.Logger LOG = System.getLogger(ConcatKDF.class.getName());
 
-    String algorithmURI;
+    private final String algorithmURI;
 
     /**
      * Constructor ConcatKDF with digest algorithmURI parameter such as http://www.w3.org/2001/04/xmlenc#sha256,
@@ -93,7 +93,7 @@ public byte[] deriveKey(byte[] secret, byte[] otherInfo, int offset, long keyLen
 
         int iDigestLength = digest.getDigestLength();
         if (genKeyLength / iDigestLength > (long) Integer.MAX_VALUE) {
-            LOG.log(Level.DEBUG, "Key size is to long to be derived with hash algorithm [{0}]", algorithmURI);
+            LOG.log(Level.ERROR, "Key size is to long to be derived with hash algorithm [{0}]", algorithmURI);
             throw new XMLEncryptionException("errorInKeyDerivation");
         }
         int toGenerateSize = (int) genKeyLength;

src/main/java/org/apache/xml/security/keys/derivedKey/ConcatKDFParamsImpl.java

@@ -151,4 +151,5 @@ public String getDigestMethod() {
     public String getBaseLocalName() {
         return EncryptionConstants._TAG_CONCATKDFPARAMS;
     }
+
 }

src/main/java/org/apache/xml/security/keys/derivedKey/DerivationAlgorithm.java

@@ -27,7 +27,7 @@
 public interface DerivationAlgorithm {
 
     /**
-     * Derives a key from the given secret and other info. The inital derived key is size of
+     * Derives a key from the given secret and other info. The initial derived key is size of
      * offset + keyLength.
      *
      *
@@ -36,19 +36,19 @@ public interface DerivationAlgorithm {
      * @param offset    the starting position in derived keying material of size: offset + keyLength
      * @param keyLength The length of the key to derive
      * @return The derived key
-     * @throws XMLEncryptionException
+     * @throws XMLEncryptionException if something goes wrong during the key derivation
      */
     byte[] deriveKey(byte[] secret, byte[] otherInfo, int offset,
                      long keyLength) throws XMLSecurityException;
 
 
     /**
      * Derives a key from the given secret and other info.
-     * @param secret
-     * @param otherInfo
-     * @param keyLength
+     * @param secret The "shared" secret to use for key derivation (e.g. the secret key)
+     * @param otherInfo as specified in [SP800-56A] the optional  attributes:  AlgorithmID, PartyUInfo, PartyVInfo, SuppPubInfo and SuppPrivInfo attributes  are concatenated to form a bit string “OtherInfo” that is used with the key derivation function.
+     * @param keyLength The length of the key to derive
      * @return The derived key
-     * @throws XMLSecurityException
+     * @throws XMLSecurityException if something goes wrong during the key derivation
      */
     default byte[] deriveKey(byte[] secret, byte[] otherInfo,
                      long keyLength) throws XMLSecurityException {

src/main/java/org/apache/xml/security/keys/derivedKey/KeyDerivationMethodImpl.java

@@ -26,6 +26,8 @@
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 
+import java.util.StringJoiner;
+
 /**
  * Class KeyDerivationMethodImpl is an DOM implementation of the KeyDerivationMethod
  *
@@ -96,4 +98,11 @@ public void setConcatKDFParams(ConcatKDFParamsImpl concatKDFParams) {
     public String getBaseLocalName() {
         return EncryptionConstants._TAG_KEYDERIVATIONMETHOD;
     }
+
+    @Override
+    public String toString() {
+        return new StringJoiner(", ", KeyDerivationMethodImpl.class.getSimpleName() + "[", "]")
+                .add("concatKDFParams=" + concatKDFParams)
+                .toString();
+    }
 }

src/main/java/org/apache/xml/security/utils/KeyUtils.java

@@ -47,8 +47,8 @@ public enum KeyAlgorithmType {
         EC("EC", "1.2.840.10045.2.1"),
         DSA("DSA", "1.2.840.10040.4.1"),
         RSA("RSA", "1.2.840.113549.1.1.1"),
-        XDH("XDH", ""),
-        EdDSA("EdDSA", "");
+        XDH("XDH", null),
+        EdDSA("EdDSA", null);
         private final String jceName;
         private final String oid;
 
@@ -96,13 +96,9 @@ public enum KeyType {
         SECP256R1("secp256r1", "NIST P-256,X9.62 prime256v1", KeyAlgorithmType.EC, "1.2.840.10045.3.1.7"),
         SECP384R1("secp384r1", "NIST P-384", KeyAlgorithmType.EC, "1.3.132.0.34"),
         SECP521R1("secp521r1", "NIST P-521", KeyAlgorithmType.EC, "1.3.132.0.35"),
-        C2TNB191V1("c2tnb191v1", "X9.62", KeyAlgorithmType.EC, "1.2.840.10045.3.0.5"),
-        C2TNB191V3("c2tnb191v3", "X9.62", KeyAlgorithmType.EC, "1.2.840.10045.3.0.7"),
-        C2TNB239V1("c2tnb239v1", "X9.62", KeyAlgorithmType.EC, "1.2.840.10045.3.0.11"),
-        C2TNB239V2("c2tnb239v2", "X9.62", KeyAlgorithmType.EC, "1.2.840.10045.3.0.12"),
-        C2TNB239V3("c2tnb239v3", "X9.62", KeyAlgorithmType.EC, "1.2.840.10045.3.0.13"),
-        C2TNB359V1("c2tnb359v1", "X9.62", KeyAlgorithmType.EC, "1.2.840.10045.3.0.18"),
-        C2TNB431R1("c2tnb431r1", "X9.62", KeyAlgorithmType.EC, "1.2.840.10045.3.0.20"),
+        BRAINPOOLP256R1("brainpoolP256r1", "RFC 5639", KeyAlgorithmType.EC, "1.3.36.3.3.2.8.1.1.7"),
+        BRAINPOOLP384R1("brainpoolP384r1", "RFC 5639", KeyAlgorithmType.EC, "1.3.36.3.3.2.8.1.1.11"),
+        BRAINPOOLP512R1("brainpoolP512r1", "RFC 5639", KeyAlgorithmType.EC, "1.3.36.3.3.2.8.1.1.13"),
         X25519("x25519", "RFC 7748", KeyAlgorithmType.XDH, "1.3.101.110"),
         X448("x448", "RFC 7748", KeyAlgorithmType.XDH, "1.3.101.111"),
         ED25519("ed25519", "RFC 8032", KeyAlgorithmType.EdDSA, "1.3.101.112"),

src/main/java/org/apache/xml/security/utils/XMLUtils.java

@@ -707,11 +707,14 @@ public static Element selectXencNode(Node sibling, String nodeName, int number)
     }
 
     /**
+     * Helper method to get the "number"-th element for a given local
+     * name and namespace: http://www.w3.org/2009/xmlenc11#. If element with given search parameters is not found,
+     * null is returned.
      *
-     * @param sibling
-     * @param nodeName
-     * @param number
-     * @return nodes with the given node name
+     * @param sibling the sibling node from which to start searching
+     * @param nodeName the local name of the element to search for
+     * @param number the index of the element to search for
+     * @return node with the given node name or null if not found.
      */
     public static Element selectXenc11Node(Node sibling, String nodeName, int number) {
         while (sibling != null) {

src/test/java/org/apache/xml/security/testutils/JDKTestUtils.java

@@ -18,7 +18,6 @@
  */
 package org.apache.xml.security.testutils;
 
-import org.apache.xml.security.encryption.XMLCipher;
 import java.lang.System.Logger.Level;
 import java.lang.reflect.Constructor;
 import java.security.Provider;
@@ -61,7 +60,6 @@ public class JDKTestUtils {
 
     private static  Set<String>  SUPPORTED_ALGORITHMS = Stream.of(Security.getProviders())
             .flatMap(provider -> provider.getServices().stream())
-           // .filter(s -> "Cipher".equals(s.getType()))
             .map(Provider.Service::getAlgorithm)
             .map(String::toLowerCase)
             .collect(Collectors.toSet());

src/test/java/org/apache/xml/security/testutils/KeyTestUtils.java

@@ -28,6 +28,8 @@
 import java.security.spec.X509EncodedKeySpec;
 import java.util.Base64;
 
+import static java.lang.System.Logger.Level.DEBUG;
+
 
 /**
  * The class provides testing utility methods to test XMLSEC functionality with various JDK version. Where possible
@@ -40,6 +42,7 @@
  *
  */
 public class KeyTestUtils {
+    private static final System.Logger LOG = System.getLogger(KeyTestUtils.class.getName());
     /**
      * The enum of the prepared test keys in resource folder <code>KEY_RESOURCE_PATH</code>.
      */
@@ -83,7 +86,7 @@ public static KeyPair generateKeyPair(KeyUtils.KeyType keyType) throws NoSuchAlg
         switch (keyType.getAlgorithm()){
             case EC:{
                 keyPairGenerator = provider == null ? KeyPairGenerator.getInstance(keyAlgorithm) :
-                        KeyPairGenerator.getInstance(keyType.getAlgorithm().getJceName(), provider);
+                        KeyPairGenerator.getInstance(keyAlgorithm, provider);
                 ECGenParameterSpec kpgparams = new ECGenParameterSpec(keyType.getName());
                 keyPairGenerator.initialize(kpgparams);
                 break;
@@ -101,6 +104,15 @@ public static KeyPair generateKeyPair(KeyUtils.KeyType keyType) throws NoSuchAlg
         return keyPairGenerator.generateKeyPair();
     }
 
+    public static KeyPair generateKeyPairIfSupported(KeyUtils.KeyType keyType){
+        KeyPair keyPair = null;
+        try {
+            keyPair = generateKeyPair(keyType);
+        } catch (NoSuchAlgorithmException | InvalidAlgorithmParameterException | InvalidParameterSpecException e) {
+            LOG.log(DEBUG, "Key algorithm [{0}]] is not supported! Error message: [{}]", keyType, e.getMessage());
+        }
+        return keyPair;
+    }
 
     public static PublicKey loadPublicKey(String keyName, String algorithm) throws Exception {
         byte[] keyBytes = getKeyResourceAsByteArray(keyName);

src/test/java/org/apache/xml/security/utils/DERDecoderUtilsTest.java

@@ -32,7 +32,6 @@
 import java.io.InputStream;
 import java.security.*;
 import java.security.spec.InvalidKeySpecException;
-import java.security.spec.InvalidParameterSpecException;
 import java.security.spec.X509EncodedKeySpec;
 import java.util.Base64;
 
@@ -65,18 +64,14 @@ void testGetAlgorithmIdBytesFromKey(KeyTestUtils.TestKeys testKey) throws DERDec
     }
 
     @ParameterizedTest
-    @EnumSource(value = KeyUtils.KeyType.class,
-            names = "^(?!C2TNB).*", mode = EnumSource.Mode.MATCH_ALL // exclude C2TNB keys
-    )
-    void testGetAlgorithmIdBytesForGeneratedKeys(KeyUtils.KeyType testKey) throws DERDecodingException,
-            InvalidAlgorithmParameterException, NoSuchAlgorithmException, InvalidParameterSpecException {
-        String keyAlgorithm = testKey.getAlgorithm().getJceName();
-        Assumptions.assumeTrue(JDKTestUtils.isAlgorithmSupported(keyAlgorithm, true), "Algorithm ["
-                + keyAlgorithm + "] not supported by JDK or auxiliary provider! Skipping test.");
+    @EnumSource(value = KeyUtils.KeyType.class)
+    void testGetAlgorithmIdBytesForGeneratedKeys(KeyUtils.KeyType testKey) throws DERDecodingException {
         if (!JDKTestUtils.isAlgorithmSupportedByJDK(testKey.getAlgorithm().getJceName())) {
             JDKTestUtils.registerAuxiliaryProvider();
         }
-        KeyPair keyPair = KeyTestUtils.generateKeyPair(testKey);
+        KeyPair keyPair = KeyTestUtils.generateKeyPairIfSupported(testKey);
+        Assumptions.assumeTrue(keyPair != null, "Key algorithm [" + testKey + "] not supported by JDK or auxiliary provider! Skipping test.");
+
         String oid = DERDecoderUtils.getAlgorithmIdFromPublicKey(keyPair.getPublic());
         assertEquals(testKey.getOid(), oid);
     }

src/test/java/org/apache/xml/security/utils/KeyUtilsTest.java

@@ -66,28 +66,22 @@ void generateEphemeralDHKeyPair(KeyTestUtils.TestKeys testKey) throws Exception
      * @throws Exception if the key cannot be loaded
      */
     @ParameterizedTest
-    @EnumSource(value = KeyUtils.KeyType.class,
-            names = "^(?!C2TNB).*", mode = EnumSource.Mode.MATCH_ALL // exclude C2TNB keys
-    )
+    @EnumSource(value = KeyUtils.KeyType.class)
     void generateEphemeralDHKeyPair(KeyUtils.KeyType keyType) throws Exception {
-        // if the algorithm is not supported by JDK, we need to use auxiliary provider
-        String keyAlgorithm = keyType.getAlgorithm().getJceName();
-        Assumptions.assumeTrue(JDKTestUtils.isAlgorithmSupported(keyAlgorithm, true), "Algorithm ["
-                + keyAlgorithm + "] not supported by JDK or auxiliary provider! Skipping test.");
 
         Provider testAuxiliaryProvider = JDKTestUtils.isAlgorithmSupportedByJDK(keyType.name()) ? null : JDKTestUtils.getAuxiliaryProvider();
-        KeyPair keys = KeyTestUtils.generateKeyPair(keyType);
-        Assertions.assertNotNull(keys);
+        KeyPair keyPair = KeyTestUtils.generateKeyPairIfSupported(keyType);
+        Assumptions.assumeTrue(keyPair != null, "Key algorithm [" + keyType + "] not supported by JDK or auxiliary provider! Skipping test.");
+
 
         // test DH key generation
-        KeyPair ephenmeralKeyPair = KeyUtils.generateEphemeralDHKeyPair(keys.getPublic(), testAuxiliaryProvider);
+        KeyPair ephenmeralKeyPair = KeyUtils.generateEphemeralDHKeyPair(keyPair.getPublic(), testAuxiliaryProvider);
         String ephemeralKeyOId = DERDecoderUtils.getAlgorithmIdFromPublicKey(ephenmeralKeyPair.getPublic());
 
         // test if the ephemeral key is generated with the same algorithm as the original key
         Assertions.assertNotNull(ephenmeralKeyPair);
-        Assertions.assertNotEquals(keys.getPublic(), ephenmeralKeyPair.getPublic());
-        Assertions.assertEquals(keys.getPublic().getAlgorithm(), ephenmeralKeyPair.getPublic().getAlgorithm());
+        Assertions.assertNotEquals(keyPair.getPublic(), ephenmeralKeyPair.getPublic());
+        Assertions.assertEquals(keyPair.getPublic().getAlgorithm(), ephenmeralKeyPair.getPublic().getAlgorithm());
         Assertions.assertEquals(keyType.getOid(), ephemeralKeyOId);
     }
-
 }