Merge pull request #15 from tls-attacker/unifiedSigValidation

Unified signature validation, fixed possible NPE
tls-attacker · Aug 4, 2022 · 4685db7 · 4685db7
2 parents 283e10f + 983c923
commit 4685db7
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 50 deletions.
diff --git a/TLS-Test-Framework/TestFramework/pom.xml b/TLS-Test-Framework/TestFramework/pom.xml
@@ -2,13 +2,13 @@
 <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/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
     <artifactId>TLS-Test-Framework</artifactId>
-    <version>1.0.1</version>
+    <version>1.0.2</version>
     <groupId>de.rub.nds.tlstest</groupId>
     <name>TLS-Test-Framework</name>
     <parent>
         <groupId>de.rub.nds.tlstest</groupId>
         <artifactId>TLS-Test-Framework-Aggregator</artifactId>
-        <version>1.0.1</version>
+        <version>1.0.2</version>
     </parent>
     <packaging>jar</packaging>
     <properties>

diff --git a/TLS-Test-Framework/pom.xml b/TLS-Test-Framework/pom.xml
@@ -7,12 +7,12 @@
     <groupId>de.rub.nds.tlstest</groupId>
     <artifactId>TLS-Test-Framework-Aggregator</artifactId>
     <packaging>pom</packaging>
-    <version>1.0.1</version>
+    <version>1.0.2</version>
     <name>TLS-Test-Framework-Aggregator</name>
     <parent>
         <groupId>de.rub.nds</groupId>
         <artifactId>TLS-Anvil</artifactId>
-        <version>1.0.1</version>
+        <version>1.0.2</version>
     </parent>
     <modules>
         <module>coffee4j</module>

diff --git a/TLS-Testsuite/pom.xml b/TLS-Testsuite/pom.xml
@@ -4,12 +4,12 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>de.rub.nds.tlstest</groupId>
     <artifactId>TLS-Testsuite</artifactId>
-    <version>1.0.1</version>
+    <version>1.0.2</version>
     <packaging>jar</packaging>
     <parent>
         <groupId>de.rub.nds</groupId>
         <artifactId>TLS-Anvil</artifactId>
-        <version>1.0.1</version>
+        <version>1.0.2</version>
     </parent>
     <dependencies>
         <dependency>
@@ -25,7 +25,7 @@
         <dependency>
             <groupId>de.rub.nds.tlstest</groupId>
             <artifactId>TLS-Test-Framework</artifactId>
-            <version>1.0.1</version>
+            <version>1.0.2</version>
         </dependency>
     </dependencies>
     <build>

diff --git a/.../src/main/java/de/rub/nds/tlstest/suite/tests/server/tls12/rfc5246/ServerKeyExchange.java b/.../src/main/java/de/rub/nds/tlstest/suite/tests/server/tls12/rfc5246/ServerKeyExchange.java
@@ -11,9 +11,12 @@
 import com.google.errorprone.annotations.CompatibleWith;
 import de.rub.nds.modifiablevariable.util.ArrayConverter;
 import de.rub.nds.tlsattacker.core.config.Config;
+import de.rub.nds.tlsattacker.core.constants.AlgorithmResolver;
+import de.rub.nds.tlsattacker.core.constants.CertificateKeyType;
 import de.rub.nds.tlsattacker.core.constants.CipherSuite;
 import de.rub.nds.tlsattacker.core.constants.HandshakeByteLength;
 import de.rub.nds.tlsattacker.core.constants.HandshakeMessageType;
+import de.rub.nds.tlsattacker.core.constants.SignatureAlgorithm;
 import de.rub.nds.tlsattacker.core.constants.SignatureAndHashAlgorithm;
 import de.rub.nds.tlsattacker.core.protocol.message.ClientHelloMessage;
 import de.rub.nds.tlsattacker.core.protocol.message.DHEServerKeyExchangeMessage;
@@ -25,6 +28,7 @@
 import de.rub.nds.tlsattacker.core.workflow.WorkflowTraceUtil;
 import de.rub.nds.tlsattacker.core.workflow.factory.WorkflowTraceType;
 import de.rub.nds.tlstest.framework.Validator;
+import de.rub.nds.tlstest.framework.annotations.DynamicValueConstraints;
 import de.rub.nds.tlstest.framework.annotations.KeyExchange;
 import de.rub.nds.tlstest.framework.annotations.RFC;
 import de.rub.nds.tlstest.framework.annotations.ServerTest;
@@ -36,17 +40,15 @@
 import de.rub.nds.tlstest.framework.constants.SeverityLevel;
 import de.rub.nds.tlstest.framework.execution.AnnotatedState;
 import de.rub.nds.tlstest.framework.execution.WorkflowRunner;
+import de.rub.nds.tlstest.framework.model.DerivationType;
 import de.rub.nds.tlstest.framework.testClasses.Tls12Test;
+import de.rub.nds.tlstest.suite.util.SignatureValidation;
 import java.io.IOException;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.InvalidKeyException;
-import java.security.KeyFactory;
 import java.security.NoSuchAlgorithmException;
-import java.security.PublicKey;
-import java.security.Signature;
 import java.security.SignatureException;
 import java.security.spec.InvalidKeySpecException;
-import java.security.spec.X509EncodedKeySpec;
 import static org.junit.Assert.assertTrue;
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.params.aggregator.ArgumentsAccessor;
@@ -61,6 +63,7 @@ public class ServerKeyExchange extends Tls12Test {
 
     @TlsTest(description = "Test if the Server sends Key Exchange Messages with valid signatures")
     @KeyExchange(supported = KeyExchangeType.ALL12, requiresServerKeyExchMsg = true)
+    @DynamicValueConstraints(affectedTypes = DerivationType.CIPHERSUITE, methods = "isSupportedCipherSuite")
     @CryptoCategory(SeverityLevel.HIGH)
     @HandshakeCategory(SeverityLevel.CRITICAL)
     @InteroperabilityCategory(SeverityLevel.CRITICAL)
@@ -75,36 +78,22 @@ public void signatureIsValid(ArgumentsAccessor argumentAccessor, WorkflowRunner
         });
     }
 
-    private boolean signatureValid(AnnotatedState annotatedState) {
+    private Boolean signatureValid(AnnotatedState annotatedState) {
         WorkflowTrace executedTrace = annotatedState.getWorkflowTrace();
         ClientHelloMessage clientHello = (ClientHelloMessage) WorkflowTraceUtil.getFirstSendMessage(HandshakeMessageType.CLIENT_HELLO, executedTrace);
         ServerHelloMessage serverHello = (ServerHelloMessage) WorkflowTraceUtil.getFirstReceivedMessage(HandshakeMessageType.SERVER_HELLO, executedTrace);
         ServerKeyExchangeMessage serverKeyExchange = (ServerKeyExchangeMessage) WorkflowTraceUtil.getFirstReceivedMessage(HandshakeMessageType.SERVER_KEY_EXCHANGE, executedTrace);
 
-
-        State sessionState = annotatedState.getState();
-
         byte[] signedKeyExchangeParameters = getSignedDataFromKeyExchangeMessage(serverKeyExchange);
         byte[] completeSignedData = ArrayConverter.concatenate(clientHello.getRandom().getValue(),
                 serverHello.getRandom().getValue(),
                 signedKeyExchangeParameters);
 
         byte[] givenSignature = serverKeyExchange.getSignature().getValue();
         SignatureAndHashAlgorithm selectedSignatureAndHashAlgo = SignatureAndHashAlgorithm.getSignatureAndHashAlgorithm(serverKeyExchange.getSignatureAndHashAlgorithm().getValue());
-
-        Signature signatureInstance;
 
         try {
-            signatureInstance = Signature.getInstance(selectedSignatureAndHashAlgo.getJavaName());
-            selectedSignatureAndHashAlgo.setupSignature(signatureInstance);
-
-            X509EncodedKeySpec rsaKeySpec = new X509EncodedKeySpec(sessionState.getTlsContext().getServerCertificate().getCertificateAt(0).getSubjectPublicKeyInfo().getEncoded());
-            KeyFactory keyFactory = KeyFactory.getInstance("RSA");
-            PublicKey rsaPublicKey = keyFactory.generatePublic(rsaKeySpec);
-
-            signatureInstance.initVerify(rsaPublicKey);
-            signatureInstance.update(completeSignedData);
-            return signatureInstance.verify(givenSignature);
+            return SignatureValidation.validationSuccessful(selectedSignatureAndHashAlgo, annotatedState, completeSignedData, givenSignature);
         } catch (SignatureException | InvalidKeyException | InvalidKeySpecException | IOException | InvalidAlgorithmParameterException | NoSuchAlgorithmException ex) {
             throw new AssertionError("Was unable to process signature for validation: " + ex);
         }
@@ -123,7 +112,7 @@ private byte[] getSignedDataFromKeyExchangeMessage(ServerKeyExchangeMessage serv
                     namedCurve,
                     publicKeyLength,
                     publicKey);
-        } else {
+        } else if(serverKeyExchange instanceof DHEServerKeyExchangeMessage) {
             DHEServerKeyExchangeMessage dheServerKeyExchange = (DHEServerKeyExchangeMessage) serverKeyExchange;
             return ArrayConverter.concatenate(
                     ArrayConverter.intToBytes(dheServerKeyExchange.getModulusLength().getValue(),
@@ -133,6 +122,16 @@ private byte[] getSignedDataFromKeyExchangeMessage(ServerKeyExchangeMessage serv
                     ArrayConverter.intToBytes(dheServerKeyExchange.getPublicKeyLength().getValue(), HandshakeByteLength.DH_PUBLICKEY_LENGTH),
                     dheServerKeyExchange.getPublicKey().getValue());
 
+        } else {
+            throw new AssertionError("Unsupported ServerKeyExchange type");
         }
     }
+
+    public boolean isSupportedCipherSuite(CipherSuite cipherSuiteCandidate) {
+        return cipherSuiteCandidate.isRealCipherSuite() 
+                && !cipherSuiteCandidate.isTLS13() 
+                && cipherSuiteCandidate.isEphemeral()
+                && (AlgorithmResolver.getCertificateKeyType(cipherSuiteCandidate) == CertificateKeyType.ECDSA || AlgorithmResolver.getCertificateKeyType(cipherSuiteCandidate) == CertificateKeyType.RSA
+                || AlgorithmResolver.getCertificateKeyType(cipherSuiteCandidate) == CertificateKeyType.DSS);
+    }
 }
diff --git a/.../src/main/java/de/rub/nds/tlstest/suite/tests/server/tls13/rfc8446/CertificateVerify.java b/.../src/main/java/de/rub/nds/tlstest/suite/tests/server/tls13/rfc8446/CertificateVerify.java
@@ -24,33 +24,24 @@
 import de.rub.nds.tlsattacker.core.workflow.action.TlsAction;
 import de.rub.nds.tlsattacker.core.workflow.factory.WorkflowTraceType;
 import de.rub.nds.tlstest.framework.Validator;
-import de.rub.nds.tlstest.framework.annotations.KeyExchange;
 import de.rub.nds.tlstest.framework.annotations.RFC;
 import de.rub.nds.tlstest.framework.annotations.ServerTest;
 import de.rub.nds.tlstest.framework.annotations.TlsTest;
-import de.rub.nds.tlstest.framework.annotations.categories.AlertCategory;
 import de.rub.nds.tlstest.framework.annotations.categories.ComplianceCategory;
 import de.rub.nds.tlstest.framework.annotations.categories.CryptoCategory;
-import de.rub.nds.tlstest.framework.annotations.categories.DeprecatedFeatureCategory;
 import de.rub.nds.tlstest.framework.annotations.categories.HandshakeCategory;
 import de.rub.nds.tlstest.framework.annotations.categories.InteroperabilityCategory;
-import de.rub.nds.tlstest.framework.annotations.categories.MessageStructureCategory;
-import de.rub.nds.tlstest.framework.annotations.categories.SecurityCategory;
-import de.rub.nds.tlstest.framework.constants.KeyExchangeType;
 import de.rub.nds.tlstest.framework.constants.SeverityLevel;
 import de.rub.nds.tlstest.framework.execution.AnnotatedState;
 import de.rub.nds.tlstest.framework.execution.WorkflowRunner;
 import de.rub.nds.tlstest.framework.testClasses.Tls13Test;
+import de.rub.nds.tlstest.suite.util.SignatureValidation;
 import java.io.IOException;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.InvalidKeyException;
-import java.security.KeyFactory;
 import java.security.NoSuchAlgorithmException;
-import java.security.PublicKey;
-import java.security.Signature;
 import java.security.SignatureException;
 import java.security.spec.InvalidKeySpecException;
-import java.security.spec.X509EncodedKeySpec;
 import static org.junit.Assert.assertTrue;
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.params.aggregator.ArgumentsAccessor;
@@ -84,19 +75,9 @@ private boolean signatureValid(AnnotatedState annotatedState) {
         SignatureAndHashAlgorithm selectedSignatureAndHashAlgo = SignatureAndHashAlgorithm.getSignatureAndHashAlgorithm(certificateVerify.getSignatureHashAlgorithm().getValue());
         byte[] givenSignature = certificateVerify.getSignature().getValue();
         byte[] signedData = getCompleteSignedData(annotatedState);
-        Signature signatureInstance;
 
         try {
-            signatureInstance = Signature.getInstance(selectedSignatureAndHashAlgo.getJavaName());
-            selectedSignatureAndHashAlgo.setupSignature(signatureInstance);
-
-            X509EncodedKeySpec rsaKeySpec = new X509EncodedKeySpec(annotatedState.getState().getTlsContext().getServerCertificate().getCertificateAt(0).getSubjectPublicKeyInfo().getEncoded());
-            KeyFactory keyFactory = KeyFactory.getInstance("RSA");
-            PublicKey rsaPublicKey = keyFactory.generatePublic(rsaKeySpec);
-
-            signatureInstance.initVerify(rsaPublicKey);
-            signatureInstance.update(signedData);
-            return signatureInstance.verify(givenSignature);
+            return SignatureValidation.validationSuccessful(selectedSignatureAndHashAlgo, annotatedState, signedData, givenSignature);
         } catch (SignatureException | InvalidKeyException | InvalidKeySpecException | IOException | InvalidAlgorithmParameterException | NoSuchAlgorithmException ex) {
             throw new AssertionError("Was unable to process signature for validation: " + ex);
         }

diff --git a/TLS-Testsuite/src/main/java/de/rub/nds/tlstest/suite/util/SignatureValidation.java b/TLS-Testsuite/src/main/java/de/rub/nds/tlstest/suite/util/SignatureValidation.java
@@ -0,0 +1,50 @@
+/**
+ * TLS-Testsuite - A testsuite for the TLS protocol
+ *
+ * Copyright 2022 Ruhr University Bochum
+ *
+ * Licensed under Apache License 2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
+ */
+package de.rub.nds.tlstest.suite.util;
+
+import de.rub.nds.tlsattacker.core.constants.SignatureAlgorithm;
+import de.rub.nds.tlsattacker.core.constants.SignatureAndHashAlgorithm;
+import de.rub.nds.tlsattacker.core.state.State;
+import de.rub.nds.tlstest.framework.execution.AnnotatedState;
+import java.io.IOException;
+import java.security.InvalidAlgorithmParameterException;
+import java.security.InvalidKeyException;
+import java.security.KeyFactory;
+import java.security.NoSuchAlgorithmException;
+import java.security.PublicKey;
+import java.security.Signature;
+import java.security.SignatureException;
+import java.security.spec.InvalidKeySpecException;
+import java.security.spec.X509EncodedKeySpec;
+
+public class SignatureValidation {
+    public static Boolean validationSuccessful(SignatureAndHashAlgorithm selectedSignatureAndHashAlgo, AnnotatedState annotatedState, byte[] completeSignedData, byte[] givenSignature) throws SignatureException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, InvalidKeyException, IOException, InvalidKeySpecException {
+        State sessionState = annotatedState.getState();
+        Signature signatureInstance = Signature.getInstance(selectedSignatureAndHashAlgo.getJavaName());
+        selectedSignatureAndHashAlgo.setupSignature(signatureInstance);
+        X509EncodedKeySpec keySpec = new X509EncodedKeySpec(sessionState.getTlsContext().getServerCertificate().getCertificateAt(0).getSubjectPublicKeyInfo().getEncoded());
+
+        KeyFactory keyFactory;
+        PublicKey publicKey;
+        if(selectedSignatureAndHashAlgo.getSignatureAlgorithm() == SignatureAlgorithm.ECDSA) {
+            keyFactory = KeyFactory.getInstance("EC");
+            publicKey = keyFactory.generatePublic(keySpec);
+        } else if(selectedSignatureAndHashAlgo.getSignatureAlgorithm() == SignatureAlgorithm.RSA) {
+            keyFactory = KeyFactory.getInstance("RSA");
+            publicKey = keyFactory.generatePublic(keySpec);
+        } else {
+            keyFactory = KeyFactory.getInstance("DSA");
+            publicKey = keyFactory.generatePublic(keySpec);
+        }
+
+        signatureInstance.initVerify(publicKey);
+        signatureInstance.update(completeSignedData);
+        return signatureInstance.verify(givenSignature);
+    }
+}
diff --git a/pom.xml b/pom.xml
@@ -5,7 +5,7 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>de.rub.nds</groupId>
     <artifactId>TLS-Anvil</artifactId>
-    <version>1.0.1</version>
+    <version>1.0.2</version>
     <name>TLS-Anvil</name>
     <packaging>pom</packaging>
     <modules>