Support for the ECDH KeyAgreement method algorithm (#264)

* Support for the ECDH KeyAgreement method algorithm * Fix typos, regenerate example keystores using keytool/ * Clean double spaces, NPE validation for KeyAgreement * XDH and EdEC key length validation for AlgorithmSuiteValidator * Add check for KeyAgreement --------- Co-authored-by: RIHTARSIC Joze <[email protected]>
apache · Jan 25, 2024 · 7ab6a7e · 7ab6a7e
1 parent 7fb8bfc
commit 7ab6a7e
Show file tree

Hide file tree

Showing 44 changed files with 1,000 additions and 258 deletions.
diff --git a/parent/pom.xml b/parent/pom.xml
@@ -17,7 +17,8 @@
   specific language governing permissions and limitations
   under the License.
 -->
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
 
     <parent>
@@ -48,7 +49,7 @@
         <wsdl4j.version>1.6.3</wsdl4j.version>
         <woodstox.version>6.6.0</woodstox.version>
         <xz.version>1.9</xz.version>
-        <xmlsec.version>4.0.1</xmlsec.version>
+        <xmlsec.version>4.0.2-SNAPSHOT</xmlsec.version>
         <xmlunit.version>2.9.1</xmlunit.version>
         <!-- OSGi related properties -->
         <wss4j.osgi.import />
@@ -107,6 +108,11 @@
                 <artifactId>junit-jupiter-engine</artifactId>
                 <version>${junit.version}</version>
             </dependency>
+            <dependency>
+                <groupId>org.junit.jupiter</groupId>
+                <artifactId>junit-jupiter-params</artifactId>
+                <version>${junit.version}</version>
+            </dependency>
             <dependency>
                 <groupId>wsdl4j</groupId>
                 <artifactId>wsdl4j</artifactId>

diff --git a/policy/pom.xml b/policy/pom.xml
@@ -17,7 +17,8 @@
   specific language governing permissions and limitations
   under the License.
 -->
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
 
     <parent>

diff --git a/policy/src/main/java/org/apache/wss4j/policy/model/AbstractSymmetricAsymmetricBinding.java b/policy/src/main/java/org/apache/wss4j/policy/model/AbstractSymmetricAsymmetricBinding.java
@@ -91,7 +91,7 @@ public int hashCode() {
     }
 
     protected void parseNestedSymmetricAsymmetricBindingBasePolicy(
-        Policy nestedPolicy,  AbstractSymmetricAsymmetricBinding asymmetricBindingBase
+        Policy nestedPolicy, AbstractSymmetricAsymmetricBinding asymmetricBindingBase
     ) {
         Iterator<List<Assertion>> alternatives = nestedPolicy.getAlternatives();
         //we just process the first alternative

diff --git a/pom.xml b/pom.xml
@@ -17,7 +17,8 @@
   specific language governing permissions and limitations
   under the License.
 -->
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
 
     <groupId>org.apache.wss4j</groupId>

diff --git a/ws-security-common/pom.xml b/ws-security-common/pom.xml
@@ -233,6 +233,11 @@
             <artifactId>junit-jupiter-engine</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-params</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.slf4j</groupId>
             <artifactId>slf4j-reload4j</artifactId>

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/ConfigurationConstants.java b/ws-security-common/src/main/java/org/apache/wss4j/common/ConfigurationConstants.java
@@ -737,6 +737,21 @@ protected ConfigurationConstants() {
      */
     public static final String ENC_KEY_TRANSPORT = "encryptionKeyTransportAlgorithm";
 
+    /**
+     * Defines the Agreement method algorithm to derive encryption key.
+     * The default algorithm is:
+     * "http://www.w3.org/2009/xmlenc11#ECDH-ES"
+     *
+     * <p/>
+     * The application may set this parameter using the following method:
+     * <pre>
+     *      call.setProperty(ConfigurationConstants.ENC_KEY_AGREEMENT_METHOD,
+     *          WSConstants.AGREEMENT_METHOD_ECDH_ES);
+     * </pre>
+     *
+     */
+    public static final String ENC_KEY_AGREEMENT_METHOD = "encryptionKeyAgreementMethod";
+
     /**
      * Parameter to define which parts of the request shall be encrypted.
      * <p/>

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/EncryptionActionToken.java b/ws-security-common/src/main/java/org/apache/wss4j/common/EncryptionActionToken.java
@@ -27,6 +27,7 @@ public class EncryptionActionToken extends SignatureEncryptionActionToken {
     private boolean encSymmetricEncryptionKey = true;
     private String mgfAlgorithm;
     private String symmetricAlgorithm;
+    private String keyAgreementMethodAlgorithm;
     private String keyTransportAlgorithm;
     private boolean getSymmetricKeyFromCallbackHandler;
 
@@ -54,6 +55,12 @@ public String getKeyTransportAlgorithm() {
     public void setKeyTransportAlgorithm(String keyTransportAlgorithm) {
         this.keyTransportAlgorithm = keyTransportAlgorithm;
     }
+    public String getKeyAgreementMethodAlgorithm() {
+        return keyAgreementMethodAlgorithm;
+    }
+    public void setKeyAgreementMethodAlgorithm(String keyAgreementMethodAlgorithm) {
+        this.keyAgreementMethodAlgorithm = keyAgreementMethodAlgorithm;
+    }
     public boolean isGetSymmetricKeyFromCallbackHandler() {
         return getSymmetricKeyFromCallbackHandler;
     }

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/WSS4JConstants.java b/ws-security-common/src/main/java/org/apache/wss4j/common/WSS4JConstants.java
@@ -108,6 +108,18 @@ protected WSS4JConstants() {
         "http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p";
     public static final String KEYTRANSPORT_RSAOAEP_XENC11 =
         "http://www.w3.org/2009/xmlenc11#rsa-oaep";
+    public static final String KEYWRAP_AES128 =
+            "http://www.w3.org/2001/04/xmlenc#kw-aes128";
+    public static final String KEYWRAP_AES192 =
+            "http://www.w3.org/2001/04/xmlenc#kw-aes192";
+    public static final String KEYWRAP_AES256 =
+            "http://www.w3.org/2001/04/xmlenc#kw-aes256";
+    public static final String KEYWRAP_TRIPLEDES =
+            "http://www.w3.org/2001/04/xmlenc#kw-tripledes";
+    public static final String KDF_CONCAT =
+            "http://www.w3.org/2009/xmlenc11#ConcatKDF";
+    public static final String AGREEMENT_METHOD_ECDH_ES =
+            "http://www.w3.org/2009/xmlenc11#ECDH-ES";
     public static final String TRIPLE_DES =
         "http://www.w3.org/2001/04/xmlenc#tripledes-cbc";
     public static final String AES_128 =

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/AlgorithmSuite.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/AlgorithmSuite.java
@@ -37,7 +37,7 @@ public class AlgorithmSuite {
 
     private Set<String> encryptionMethods = Collections.emptySet();
     private Set<String> keyWrapAlgorithms = Collections.emptySet();
-
+    private Set<String> keyAgreementAlgorithms = Collections.emptySet();
     private Set<String> derivedKeyAlgorithms = Collections.emptySet();
 
     private int maximumSymmetricKeyLength = 256;
@@ -116,6 +116,17 @@ public Set<String> getKeyWrapAlgorithms() {
         return keyWrapAlgorithms;
     }
 
+    public void addKeyAgreementMethodAlgorithm(String keyAgreementAlgorithm) {
+        if (keyAgreementAlgorithms.isEmpty()) {
+            keyAgreementAlgorithms = new HashSet<>();
+        }
+        keyAgreementAlgorithms.add(keyAgreementAlgorithm);
+    }
+
+    public Set<String> getKeyAgreementMethodAlgorithms() {
+        return keyAgreementAlgorithms;
+    }
+
     public void addDerivedKeyAlgorithm(String derivedKeyAlgorithm) {
         if (derivedKeyAlgorithms.isEmpty()) {
             derivedKeyAlgorithms = new HashSet<>();
@@ -191,4 +202,4 @@ public void setMinimumEllipticCurveKeyLength(int minimumEllipticCurveKeyLength)
         this.minimumEllipticCurveKeyLength = minimumEllipticCurveKeyLength;
     }
 
-}
+}
diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/AlgorithmSuiteValidator.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/AlgorithmSuiteValidator.java
@@ -21,16 +21,17 @@
 
 import java.security.PublicKey;
 import java.security.cert.X509Certificate;
-import java.security.interfaces.DSAPublicKey;
-import java.security.interfaces.ECPublicKey;
-import java.security.interfaces.RSAPublicKey;
+import java.security.interfaces.*;
 import java.util.Set;
 
 import javax.xml.crypto.dsig.Reference;
 import javax.xml.crypto.dsig.Transform;
 import javax.xml.crypto.dsig.XMLSignature;
 
 import org.apache.wss4j.common.ext.WSSecurityException;
+import org.apache.xml.security.exceptions.DERDecodingException;
+import org.apache.xml.security.utils.DERDecoderUtils;
+import org.apache.xml.security.utils.KeyUtils;
 
 /**
  * Validate signature/encryption/etc. algorithms against an AlgorithmSuite policy.
@@ -138,6 +139,19 @@ public void checkEncryptionKeyWrapAlgorithm(
         }
     }
 
+    public void checkKeyAgreementMethodAlgorithm(
+            String keyAgreementMethodAlgorithm
+    ) throws WSSecurityException {
+        Set<String> keyAgreementMethodAlgorithms = algorithmSuite.getKeyAgreementMethodAlgorithms();
+        if (!keyAgreementMethodAlgorithms.isEmpty()
+                && !keyAgreementMethodAlgorithms.contains(keyAgreementMethodAlgorithm)) {
+            LOG.warn(
+                    "The Key agreement method does not match the requirement"
+            );
+            throw new WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
+        }
+    }
+
     public void checkSymmetricEncryptionAlgorithm(
         String symmetricAlgorithm
     ) throws WSSecurityException {
@@ -216,11 +230,50 @@ public void checkAsymmetricKeyLength(
                 throw new WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
             }
         } else {
-            LOG.warn(
-                "An unknown public key was provided"
-            );
+            // Try with last supported key types EdEC and XDH
+            int keySize = getEdECndXDHKeyLength(publicKey);
+            if (keySize < algorithmSuite.getMinimumEllipticCurveKeyLength()
+                    || keySize > algorithmSuite.getMaximumEllipticCurveKeyLength()) {
+                LOG.warn(
+                        "The asymmetric key length does not match the requirement"
+                );
+                throw new WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
+            }
+        }
+    }
+
+    /**
+     * A generic method to determinate key length for keys x25519, x448, ed25519 and ed448 keys. Method does not rely on
+     * any specific implementation of the key, but uses OID to determine the key type.
+     *
+     * @param publicKey the public key to check the key length
+     * @return the key length in bits
+     * @throws WSSecurityException if the key is not  EdEC or XDH or if length can not be determined
+     */
+    private int getEdECndXDHKeyLength(PublicKey publicKey) throws  WSSecurityException {
+        String keyAlgorithmOId;
+        try {
+            keyAlgorithmOId = DERDecoderUtils.getAlgorithmIdFromPublicKey(publicKey);
+        } catch (DERDecodingException e) {
+            LOG.warn("Can not parse the public key to determine key size!", e);
+            throw new WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
+        }
+        KeyUtils.KeyType keyType = KeyUtils.KeyType.getByOid(keyAlgorithmOId);
+        if (keyType == null) {
+            LOG.warn("An unknown public key was provided");
             throw new WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
         }
+
+        return switch (keyType) {
+            case ED25519, X25519 -> 256;
+            case ED448, X448 -> 456;
+            default -> {
+                LOG.warn(
+                        "An unknown public key was provided"
+                );
+                throw new WSSecurityException(WSSecurityException.ErrorCode.INVALID_SECURITY);
+            }
+        };
     }
 
     /**

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java
@@ -1454,7 +1454,7 @@ private String getIdentifier(X509Certificate cert, KeyStore store)
 
                 Certificate[] certs = store.getCertificateChain(alias);
                 if (certs == null || certs.length == 0) {
-                    // no cert chain, so lets check if getCertificate gives us a  result.
+                    // no cert chain, so lets check if getCertificate gives us a result.
                     Certificate retrievedCert = store.getCertificate(alias);
                     if (retrievedCert != null) {
                         certs = new Certificate[]{retrievedCert};
@@ -1479,7 +1479,7 @@ private String getIdentifier(PublicKey publicKey, KeyStore store)
 
                 Certificate[] certs = store.getCertificateChain(alias);
                 if (certs == null || certs.length == 0) {
-                    // no cert chain, so lets check if getCertificate gives us a  result.
+                    // no cert chain, so lets check if getCertificate gives us a result.
                     Certificate retrievedCert = store.getCertificate(alias);
                     if (retrievedCert != null) {
                         certs = new Certificate[]{retrievedCert};

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/WSProviderConfig.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/WSProviderConfig.java
@@ -143,7 +143,7 @@ public static synchronized void cleanUp() {
      * Set the value of the internal addJceProviders flag.  This flag
      * turns on (or off) automatic registration of known JCE providers
      * that provide necessary cryptographic algorithms for use with WSS4J.
-     * By default, this flag is true.  You may wish (or need) to initialize
+     * By default, this flag is true. You may wish (or need) to initialize
      * the JCE manually, e.g., in some JVMs.
      */
     public static void setAddJceProviders(boolean value) {

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/OpenSAMLUtil.java b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/OpenSAMLUtil.java
@@ -166,7 +166,7 @@ public static XMLObject fromDom(Element root) throws WSSecurityException {
      * Convert a SAML Assertion from a XMLObject to a DOM Element
      *
      * @param xmlObject of type XMLObject
-     * @param doc  of type Document
+     * @param doc of type Document
      * @return Element
      * @throws WSSecurityException
      */
@@ -181,7 +181,7 @@ public static Element toDom(
      * Convert a SAML Assertion from a XMLObject to a DOM Element
      *
      * @param xmlObject of type XMLObject
-     * @param doc  of type Document
+     * @param doc of type Document
      * @param signObject whether to sign the XMLObject during marshalling
      * @return Element
      * @throws WSSecurityException

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SamlAssertionWrapper.java b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SamlAssertionWrapper.java
@@ -144,7 +144,7 @@ public SamlAssertionWrapper(Element element) throws WSSecurityException {
 
     /**
      * Constructor SamlAssertionWrapper creates a new SamlAssertionWrapper instance.
-     * This is the primary constructor.  All other constructor calls should
+     * This is the primary constructor. All other constructor calls should
      * be routed to this method to ensure that the wrapper is initialized
      * correctly.
      *

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/util/InetAddressUtils.java b/ws-security-common/src/main/java/org/apache/wss4j/common/util/InetAddressUtils.java
@@ -18,7 +18,7 @@
  */
 /*
  * This software consists of voluntary contributions made by many
- * individuals on behalf of the Apache Software Foundation.  For more
+ * individuals on behalf of the Apache Software Foundation. For more
  * information on the Apache Software Foundation, please see
  * <http://www.apache.org/>.
  *
@@ -56,7 +56,7 @@ public final class InetAddressUtils {
                 + "(([0-9A-Fa-f]{1,4}(:[0-9A-Fa-f]{1,4}){0,5})?)$"); // 0-6 hex fields
 
     /*
-     *  The above pattern is not totally rigorous as it allows for more than 7 hex fields in total
+     * The above pattern is not totally rigorous as it allows for more than 7 hex fields in total
      */
     private static final char COLON_CHAR = ':';
 

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/util/NSStack.java b/ws-security-common/src/main/java/org/apache/wss4j/common/util/NSStack.java
@@ -21,14 +21,14 @@
 
 /**
  * The abstraction this class provides is a push down stack of variable
- * length frames of prefix to namespace mappings.  Used for keeping track
+ * length frames of prefix to namespace mappings. Used for keeping track
  * of what namespaces are active at any given point as an XML document is
  * traversed or produced.
  * <p/>
  * From a performance point of view, this data will both be modified frequently
  * (at a minimum, there will be one push and pop per XML element processed),
  * and scanned frequently (many of the "good" mappings will be at the bottom
- * of the stack).  The one saving grace is that the expected maximum
+ * of the stack). The one saving grace is that the expected maximum
  * cardinalities of the number of frames and the number of total mappings
  * is only in the dozens, representing the nesting depth of an XML document
  * and the number of active namespaces at any point in the processing.
@@ -113,8 +113,8 @@ private void clearFrame() {
 
     /**
      * Reset the embedded iterator in this class to the top of the current
-     * (i.e., last) frame.  Note that this is not threadsafe, nor does it
-     * provide multiple iterators, so don't use this recursively.  Nor
+     * (i.e., last) frame. Note that this is not threadsafe, nor does it
+     * provide multiple iterators, so don't use this recursively. Nor
      * should you modify the stack while iterating over it.
      */
     public Mapping topOfFrame() {
@@ -139,7 +139,7 @@ public Mapping next() {
 
     /**
      * Add a mapping for a namespaceURI to the specified prefix to the top
-     * frame in the stack.  If the prefix is already mapped in that frame,
+     * frame in the stack. If the prefix is already mapped in that frame,
      * remap it to the (possibly different) namespaceURI.
      */
     public void add(String namespaceURI, String prefix) {
@@ -166,10 +166,10 @@ public void add(String namespaceURI, String prefix) {
     }
 
     /**
-     * Return an active prefix for the given namespaceURI.  NOTE : This
+     * Return an active prefix for the given namespaceURI. NOTE : This
      * may return null even if the namespaceURI was actually mapped further
      * up the stack IF the prefix which was used has been repeated further
-     * down the stack.  I.e.:
+     * down the stack. I.e.:
      * <p/>
      * <pre:outer xmlns:pre="namespace">
      * <pre:inner xmlns:pre="otherNamespace">

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/util/XMLUtils.java b/ws-security-common/src/main/java/org/apache/wss4j/common/util/XMLUtils.java
@@ -64,8 +64,8 @@ private XMLUtils() {
      * Gets a direct child with specified localname and namespace. <p/>
      *
      * @param parentNode the node where to start the search
-     * @param localName  local name of the child to get
-     * @param namespace  the namespace of the child to get
+     * @param localName local name of the child to get
+     * @param namespace the namespace of the child to get
      * @return the node or <code>null</code> if not such node found
      */
     public static Element getDirectChildElement(Node parentNode, String localName, String namespace) {