Minor Improvements and removed Padding Check

Added descriptions
Added SessionTicketZeroKey details in SiteReportPrinter
Refactored checkForMasterSecret
Removed Padding Check

Author: DaJeZi

src/main/java/de/rub/nds/tlsscanner/probe/SessionTicketZeroKeyProbe.java

@@ -10,6 +10,7 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
 
@@ -18,6 +19,8 @@
 import javax.crypto.spec.IvParameterSpec;
 import javax.crypto.spec.SecretKeySpec;
 
+import org.apache.commons.lang3.ArrayUtils;
+
 import de.rub.nds.modifiablevariable.util.ArrayConverter;
 import de.rub.nds.tlsattacker.core.config.Config;
 import de.rub.nds.tlsattacker.core.constants.CipherSuite;
@@ -38,14 +41,55 @@
 import de.rub.nds.tlsscanner.report.result.ProbeResult;
 import de.rub.nds.tlsscanner.report.result.SessionTicketZeroKeyResult;
 
+/**
+ * 
+ * The Probe checks for CVE-2020-13777.
+ * 
+ * Quote: "GnuTLS 3.6.x before 3.6.14 uses incorrect cryptography for encrypting
+ * a session ticket (a loss of confidentiality in TLS 1.2, and an authentication
+ * bypass in TLS 1.3). The earliest affected version is 3.6.4 (2018-09-24)
+ * because of an error in a 2018-09-18 commit. Until the first key rotation, the
+ * TLS server always uses wrong data in place of an encryption key derived from
+ * an application."[1]
+ * 
+ * Reference [1]: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-13777
+ * Reference [2]: https://www.gnutls.org/security-new.html
+ * 
+ */
 public class SessionTicketZeroKeyProbe extends TlsProbe {
 
+    /**
+     * Magic Bytes the plaintext state in GnuTls starts with
+     */
     public static final byte[] GNU_TLS_MAGIC_BYTES = ArrayConverter.hexStringToByteArray("FAE1C0EA");
+
+    /**
+     * Offset of the IV according to the ticket struct in rfc5077
+     */
     public static final int IV_OFFSET = 16;
+
+    /**
+     * Length of the IV according to the ticket struct in rfc5077
+     */
     public static final int IV_LEN = 16;
+
+    /**
+     * Offset of the length field for the in the encrypted state according to
+     * the ticket struct in rfc5077
+     */
     public static final int SESSION_STATE_LENFIELD_OFFSET = 32;
+
+    /**
+     * Length of the length field for the in the encrypted state according to
+     * the ticket struct in rfc5077
+     */
     public static final int SESSION_STATE_LENFIELD_LEN = 2;
+
+    /**
+     * Offset of the encrypted state according to the ticket struct in rfc5077
+     */
     public static final int SESSION_STATE_OFFSET = 34;
+
     private List<CipherSuite> supportedSuites;
 
     public SessionTicketZeroKeyProbe(ScannerConfig scannerConfig, ParallelExecutor parallelExecutor) {
@@ -79,13 +123,11 @@ public ProbeResult executeTest() {
             executeState(state);
         } catch (Exception E) {
             LOGGER.error("Could not scan for " + getProbeName(), E);
-            return new SessionTicketZeroKeyResult(TestResult.ERROR_DURING_TEST, TestResult.ERROR_DURING_TEST,
-                    TestResult.ERROR_DURING_TEST);
+            return new SessionTicketZeroKeyResult(TestResult.ERROR_DURING_TEST, TestResult.ERROR_DURING_TEST);
         }
 
         if (!WorkflowTraceUtil.didReceiveMessage(HandshakeMessageType.NEW_SESSION_TICKET, state.getWorkflowTrace())) {
-            return new SessionTicketZeroKeyResult(TestResult.UNSUPPORTED, TestResult.UNSUPPORTED,
-                    TestResult.UNSUPPORTED);
+            return new SessionTicketZeroKeyResult(TestResult.UNSUPPORTED, TestResult.UNSUPPORTED);
         }
 
         byte[] ticket = null;
@@ -96,8 +138,7 @@ public ProbeResult executeTest() {
             }
         }
 
-        byte[] key = ArrayConverter
-                .hexStringToByteArray("0000000000000000000000000000000000000000000000000000000000000000");
+        byte[] key = new byte[32];
         byte[] iv, encryptedSessionState;
         byte[] decryptedSessionState = null;
 
@@ -114,9 +155,8 @@ public ProbeResult executeTest() {
             decryptedSessionState = cipher.doFinal(encryptedSessionState);
             LOGGER.debug("decryptedSsessionState" + ArrayConverter.bytesToHexString(decryptedSessionState));
         } catch (Exception e) {
-            return new SessionTicketZeroKeyResult(TestResult.FALSE, TestResult.FALSE, TestResult.FALSE);
+            return new SessionTicketZeroKeyResult(TestResult.FALSE, TestResult.FALSE);
         }
-        TestResult hasCorrectPadding = TestResult.TRUE;
         TestResult hasDecryptableMasterSecret;
         TestResult hasGnuTlsMagicBytes;
 
@@ -125,14 +165,15 @@ public ProbeResult executeTest() {
         } else {
             hasDecryptableMasterSecret = TestResult.FALSE;
         }
+
         if (checkForGnuTlsMagicBytes(decryptedSessionState)) {
             hasGnuTlsMagicBytes = TestResult.TRUE;
 
         } else {
             hasGnuTlsMagicBytes = TestResult.FALSE;
         }
 
-        return new SessionTicketZeroKeyResult(hasCorrectPadding, hasDecryptableMasterSecret, hasGnuTlsMagicBytes);
+        return new SessionTicketZeroKeyResult(hasDecryptableMasterSecret, hasGnuTlsMagicBytes);
     }
 
     @Override
@@ -141,20 +182,12 @@ public boolean canBeExecuted(SiteReport report) {
     }
 
     private boolean checkForMasterSecret(byte[] decState, TlsContext context) {
-        boolean found = false;
-        byte[] ms = context.getMasterSecret();
-        for (int i = 0; i < decState.length - ms.length; i++) {
-            found = true;
-            for (int j = 0; j < ms.length; j++) {
-                if (decState[i + j] != ms[j]) {
-                    found = false;
-                    break;
-                }
-            }
-            if (found)
-                return true;
+        List<Byte> target = Arrays.asList(ArrayUtils.toObject(context.getMasterSecret()));
+        List<Byte> source = Arrays.asList(ArrayUtils.toObject(decState));
+        if (Collections.indexOfSubList(source, target) == -1) {
+            return false;
         }
-        return false;
+        return true;
     }
 
     private boolean checkForGnuTlsMagicBytes(byte[] decState) {
@@ -170,13 +203,12 @@ private boolean checkForGnuTlsMagicBytes(byte[] decState) {
 
     @Override
     public ProbeResult getCouldNotExecuteResult() {
-        return new SessionTicketZeroKeyResult(TestResult.COULD_NOT_TEST, TestResult.COULD_NOT_TEST,
-                TestResult.COULD_NOT_TEST);
+        return new SessionTicketZeroKeyResult(TestResult.COULD_NOT_TEST, TestResult.COULD_NOT_TEST);
     }
 
     @Override
     public void adjustConfig(SiteReport report) {
-            supportedSuites = new ArrayList<>(report.getCipherSuites());
+        supportedSuites = new ArrayList<>(report.getCipherSuites());
     }
 
-}
+}

src/main/java/de/rub/nds/tlsscanner/report/AnalyzedProperty.java

@@ -187,8 +187,6 @@ public enum AnalyzedProperty {
     REUSES_DH_PUBLICKEY(AnalyzedPropertyCategory.FRESHNESS),
     REUSES_GCM_NONCES(AnalyzedPropertyCategory.FRESHNESS),
     REQUIRES_SNI(AnalyzedPropertyCategory.SNI),
-	
-    HAS_CORRECT_TICKET_PADDING(AnalyzedPropertyCategory.SESSION_TICKET),
     HAS_GNU_TLS_MAGIC_BYTES(AnalyzedPropertyCategory.SESSION_TICKET);	
 
     private AnalyzedPropertyCategory category;

src/main/java/de/rub/nds/tlsscanner/report/SiteReportPrinter.java

@@ -117,6 +117,7 @@ public String getFullReport() {
         appendAttackVulnerabilities(builder);
         appendBleichenbacherResults(builder);
         appendPaddingOracleResults(builder);
+        sessionTicketZeroKeyDetails(builder);
         //appendGcm(builder);
         appendRfc(builder);
         appendCertificate(builder);
@@ -576,7 +577,7 @@ private StringBuilder appendAttackVulnerabilities(StringBuilder builder) {
         prettyAppend(builder, "DROWN", AnalyzedProperty.VULNERABLE_TO_DROWN);
         prettyAppend(builder, "Heartbleed", AnalyzedProperty.VULNERABLE_TO_HEARTBLEED);
         prettyAppend(builder, "EarlyCcs", AnalyzedProperty.VULNERABLE_TO_EARLY_CCS);
-        prettyAppend(builder, "SessionTicketZeroKey", AnalyzedProperty.VULNERABLE_TO_SESSION_TICKET_ZERO_KEY);
+        prettyAppend(builder, "CVE-2020-13777 (Zero key)", AnalyzedProperty.VULNERABLE_TO_SESSION_TICKET_ZERO_KEY);
         return builder;
     }
 
@@ -1417,5 +1418,14 @@ private void appendPerformanceData(StringBuilder builder) {
             LOGGER.debug("Not printing performance data.");
         }
     }
+    
+    private StringBuilder sessionTicketZeroKeyDetails(StringBuilder builder) {
+        if (report.getResult(AnalyzedProperty.VULNERABLE_TO_SESSION_TICKET_ZERO_KEY) == TestResult.TRUE) {
+            prettyAppendHeading(builder, "Session Ticket Zero Key Attack Details");
+            prettyAppend(builder, "Has GnuTls magic bytes:", AnalyzedProperty.HAS_GNU_TLS_MAGIC_BYTES);
+        }
+        return builder;
+    }    
+    
 }
 

src/main/java/de/rub/nds/tlsscanner/report/result/SessionTicketZeroKeyResult.java

@@ -15,24 +15,20 @@
 
 public class SessionTicketZeroKeyResult extends ProbeResult {
 
-    private TestResult hasCorrectTicketPadding;
     private TestResult hasDecryptableMasterSecret;
     private TestResult hasGnuTlsMagicBytes;
 
-    public SessionTicketZeroKeyResult(TestResult hasCorrectTicketPadding, TestResult hasDecryptableMasterSecret,
-            TestResult hasGnuTlsMagicBytes) {
+    public SessionTicketZeroKeyResult(TestResult hasDecryptableMasterSecret, TestResult hasGnuTlsMagicBytes) {
         super(ProbeType.SESSION_TICKET_ZERO_KEY);
-        this.hasCorrectTicketPadding = hasCorrectTicketPadding;
         this.hasDecryptableMasterSecret = hasDecryptableMasterSecret;
         this.hasGnuTlsMagicBytes = hasGnuTlsMagicBytes;
 
     }
 
     @Override
     protected void mergeData(SiteReport report) {
-        report.putResult(AnalyzedProperty.HAS_CORRECT_TICKET_PADDING, this.hasCorrectTicketPadding);
         report.putResult(AnalyzedProperty.VULNERABLE_TO_SESSION_TICKET_ZERO_KEY, this.hasDecryptableMasterSecret);
         report.putResult(AnalyzedProperty.HAS_GNU_TLS_MAGIC_BYTES, this.hasGnuTlsMagicBytes);
     }
 
-}
+}