diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCryptoComparison.qhelp b/java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCryptoComparison.qhelp index 6ae9274ee37..520e861105f 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCryptoComparison.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCryptoComparison.qhelp @@ -4,7 +4,7 @@

When comparing results of cryptographic operations, such as MAC or digital signature, -a constant time algorithm should be used. In other words, the comparison time should not depend on +a constant-time algorithm should be used. In other words, the comparison time should not depend on the content of the input. Otherwise, attackers may be able to implement a timing attack if they can control input. A successful timing attack may result in leaking secrets or authentication bypass.

@@ -21,12 +21,12 @@ and does not depend on the contents of the arrays.

The following example uses Arrays.equals() method for comparing MAC. -This method implements a non-constant time algorithm: +This method implements a non-constant-time algorithm:

-The next example uses a safe constant time algorithm for comparing MAC: +The next example uses a safe constant-time algorithm for comparing MAC:

diff --git a/java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCryptoComparison.ql b/java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCryptoComparison.ql index e0d82f61e2a..59f9a9713d6 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCryptoComparison.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCryptoComparison.ql @@ -1,6 +1,6 @@ /** - * @name Using a non-constant time algorithm for comparing results of a cryptographic operation - * @description When comparing results of a cryptographic operation, a constant time algorithm should be used. + * @name Using a non-constant-time algorithm for comparing results of a cryptographic operation + * @description When comparing results of a cryptographic operation, a constant-time algorithm should be used. * Otherwise, attackers may be able to implement a timing attack if they can control input. * A successful attack may result in leaking secrets or authentication bypass. * @kind path-problem @@ -12,6 +12,7 @@ */ import java +import semmle.code.java.controlflow.Guards import semmle.code.java.dataflow.TaintTracking import semmle.code.java.dataflow.TaintTracking2 import semmle.code.java.dataflow.FlowSources @@ -146,7 +147,7 @@ private class NonConstantTimeComparisonCall extends StaticMethodAccess { /** * A config that tracks data flow from remote user input to methods - * that compare inputs using a non-constant time algorithm. + * that compare inputs using a non-constant-time algorithm. */ private class UserInputInComparisonConfig extends TaintTracking2::Configuration { UserInputInComparisonConfig() { this = "UserInputInComparisonConfig" } @@ -169,25 +170,69 @@ private predicate looksLikeConstant(Expr expr) { expr.(VarAccess).getVariable().isFinal() and expr.getType() instanceof TypeString } -/** A sink that compares input using a non-constant time algorithm. */ +/** + * Holds if `firstObject` and `secondObject` are compared using a method + * that does not use a constant-time algorithm, for example, `String.equals()`. + */ +private predicate isNonConstantEqualsCall(Expr firstObject, Expr secondObject) { + exists(NonConstantTimeEqualsCall call | + firstObject = call.getQualifier() and + secondObject = call.getAnArgument() + or + firstObject = call.getAnArgument() and + secondObject = call.getQualifier() + ) +} + +/** + * Holds if `firstInput` and `secondInput` are compared using a static method + * that does not use a constant-time algorithm, for example, `Arrays.equals()`. + */ +private predicate isNonConstantTimeComparisonCall(Expr firstInput, Expr secondInput) { + exists(NonConstantTimeComparisonCall call | + firstInput = call.getArgument(0) and secondInput = call.getArgument(1) + or + firstInput = call.getArgument(1) and secondInput = call.getArgument(0) + ) +} + +/** + * Holds if there is a fast-fail check while comparing `firstArray` and `secondArray`. + */ +private predicate existsFailFastCheck(Expr firstArray, Expr secondArray) { + exists( + Guard guard, EqualityTest eqTest, boolean branch, Stmt fastFailingStmt, + ArrayAccess firstArrayAccess, ArrayAccess secondArrayAccess + | + guard = eqTest and + // For `==` false branch is fail fast; for `!=` true branch is fail fast + branch = eqTest.polarity().booleanNot() and + ( + fastFailingStmt instanceof ReturnStmt or + fastFailingStmt instanceof BreakStmt or + fastFailingStmt instanceof ThrowStmt + ) and + guard.controls(fastFailingStmt.getBasicBlock(), branch) and + DataFlow::localExprFlow(firstArrayAccess, eqTest.getLeftOperand()) and + DataFlow::localExprFlow(secondArrayAccess, eqTest.getRightOperand()) + | + firstArrayAccess.getArray() = firstArray and secondArray = secondArrayAccess + or + secondArrayAccess.getArray() = firstArray and secondArray = firstArrayAccess + ) +} + +/** A sink that compares input using a non-constant-time algorithm. */ private class NonConstantTimeComparisonSink extends DataFlow::Node { Expr anotherParameter; NonConstantTimeComparisonSink() { ( - exists(NonConstantTimeEqualsCall call | - this.asExpr() = call.getQualifier() and - anotherParameter = call.getAnArgument() - or - this.asExpr() = call.getAnArgument() and - anotherParameter = call.getQualifier() - ) + isNonConstantEqualsCall(this.asExpr(), anotherParameter) or - exists(NonConstantTimeComparisonCall call | call.getAnArgument() = this.asExpr() | - this.asExpr() = call.getArgument(0) and anotherParameter = call.getArgument(1) - or - this.asExpr() = call.getArgument(1) and anotherParameter = call.getArgument(0) - ) + isNonConstantTimeComparisonCall(this.asExpr(), anotherParameter) + or + existsFailFastCheck(this.asExpr(), anotherParameter) ) and not looksLikeConstant(anotherParameter) } @@ -202,7 +247,7 @@ private class NonConstantTimeComparisonSink extends DataFlow::Node { /** * A configuration that tracks data flow from cryptographic operations - * to methods that compare data using a non-constant time algorithm. + * to methods that compare data using a non-constant-time algorithm. */ private class NonConstantTimeCryptoComparisonConfig extends TaintTracking::Configuration { NonConstantTimeCryptoComparisonConfig() { this = "NonConstantTimeCryptoComparisonConfig" } @@ -220,4 +265,4 @@ where sink.getNode().(NonConstantTimeComparisonSink).includesUserInput() ) select sink.getNode(), source, sink, - "Using a non-constant time algorithm for comparing results of a cryptographic operation." + "Using a non-constant-time algorithm for comparing results of a cryptographic operation." diff --git a/java/ql/test/experimental/query-tests/security/CWE-208/NonConstantTimeCryptoComparison.expected b/java/ql/test/experimental/query-tests/security/CWE-208/NonConstantTimeCryptoComparison.expected index c41e22c21d5..ee8a0a1bd61 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-208/NonConstantTimeCryptoComparison.expected +++ b/java/ql/test/experimental/query-tests/security/CWE-208/NonConstantTimeCryptoComparison.expected @@ -1,46 +1,50 @@ edges -| NonConstantTimeCryptoComparison.java:19:28:19:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:20:43:20:51 | actualMac | -| NonConstantTimeCryptoComparison.java:28:28:28:40 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:29:84:29:92 | actualMac : byte[] | -| NonConstantTimeCryptoComparison.java:29:84:29:92 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:29:66:29:93 | castToObjectArray(...) | -| NonConstantTimeCryptoComparison.java:36:21:36:29 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:38:43:38:51 | actualMac | -| NonConstantTimeCryptoComparison.java:56:28:56:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:57:40:57:48 | signature | -| NonConstantTimeCryptoComparison.java:67:21:67:29 | signature : byte[] | NonConstantTimeCryptoComparison.java:68:40:68:48 | signature | -| NonConstantTimeCryptoComparison.java:85:22:85:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:87:45:87:47 | tag | -| NonConstantTimeCryptoComparison.java:97:24:97:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:98:40:98:42 | tag | -| NonConstantTimeCryptoComparison.java:107:52:107:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:109:40:109:42 | tag : ByteBuffer | -| NonConstantTimeCryptoComparison.java:109:40:109:42 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:109:40:109:50 | array(...) | -| NonConstantTimeCryptoComparison.java:119:52:119:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:120:49:120:51 | tag | -| NonConstantTimeCryptoComparison.java:136:22:136:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:137:40:137:42 | tag | +| NonConstantTimeCryptoComparison.java:20:28:20:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:21:43:21:51 | actualMac | +| NonConstantTimeCryptoComparison.java:29:28:29:40 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:30:84:30:92 | actualMac : byte[] | +| NonConstantTimeCryptoComparison.java:30:84:30:92 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:30:66:30:93 | castToObjectArray(...) | +| NonConstantTimeCryptoComparison.java:37:21:37:29 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:39:43:39:51 | actualMac | +| NonConstantTimeCryptoComparison.java:57:28:57:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:58:40:58:48 | signature | +| NonConstantTimeCryptoComparison.java:68:21:68:29 | signature : byte[] | NonConstantTimeCryptoComparison.java:69:40:69:48 | signature | +| NonConstantTimeCryptoComparison.java:86:22:86:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:88:45:88:47 | tag | +| NonConstantTimeCryptoComparison.java:98:24:98:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:99:40:99:42 | tag | +| NonConstantTimeCryptoComparison.java:108:52:108:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:110:40:110:42 | tag : ByteBuffer | +| NonConstantTimeCryptoComparison.java:110:40:110:42 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:110:40:110:50 | array(...) | +| NonConstantTimeCryptoComparison.java:120:52:120:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:121:49:121:51 | tag | +| NonConstantTimeCryptoComparison.java:137:22:137:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:138:40:138:42 | tag | +| NonConstantTimeCryptoComparison.java:168:34:168:50 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:171:26:171:36 | computedTag | nodes -| NonConstantTimeCryptoComparison.java:19:28:19:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] | -| NonConstantTimeCryptoComparison.java:20:43:20:51 | actualMac | semmle.label | actualMac | -| NonConstantTimeCryptoComparison.java:28:28:28:40 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] | -| NonConstantTimeCryptoComparison.java:29:66:29:93 | castToObjectArray(...) | semmle.label | castToObjectArray(...) | -| NonConstantTimeCryptoComparison.java:29:84:29:92 | actualMac : byte[] | semmle.label | actualMac : byte[] | -| NonConstantTimeCryptoComparison.java:36:21:36:29 | actualMac : byte[] | semmle.label | actualMac : byte[] | -| NonConstantTimeCryptoComparison.java:38:43:38:51 | actualMac | semmle.label | actualMac | -| NonConstantTimeCryptoComparison.java:56:28:56:40 | sign(...) : byte[] | semmle.label | sign(...) : byte[] | -| NonConstantTimeCryptoComparison.java:57:40:57:48 | signature | semmle.label | signature | -| NonConstantTimeCryptoComparison.java:67:21:67:29 | signature : byte[] | semmle.label | signature : byte[] | -| NonConstantTimeCryptoComparison.java:68:40:68:48 | signature | semmle.label | signature | -| NonConstantTimeCryptoComparison.java:85:22:85:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] | -| NonConstantTimeCryptoComparison.java:87:45:87:47 | tag | semmle.label | tag | -| NonConstantTimeCryptoComparison.java:97:24:97:26 | tag : byte[] | semmle.label | tag : byte[] | -| NonConstantTimeCryptoComparison.java:98:40:98:42 | tag | semmle.label | tag | -| NonConstantTimeCryptoComparison.java:107:52:107:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer | -| NonConstantTimeCryptoComparison.java:109:40:109:42 | tag : ByteBuffer | semmle.label | tag : ByteBuffer | -| NonConstantTimeCryptoComparison.java:109:40:109:50 | array(...) | semmle.label | array(...) | -| NonConstantTimeCryptoComparison.java:119:52:119:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer | -| NonConstantTimeCryptoComparison.java:120:49:120:51 | tag | semmle.label | tag | -| NonConstantTimeCryptoComparison.java:136:22:136:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] | -| NonConstantTimeCryptoComparison.java:137:40:137:42 | tag | semmle.label | tag | +| NonConstantTimeCryptoComparison.java:20:28:20:44 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] | +| NonConstantTimeCryptoComparison.java:21:43:21:51 | actualMac | semmle.label | actualMac | +| NonConstantTimeCryptoComparison.java:29:28:29:40 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] | +| NonConstantTimeCryptoComparison.java:30:66:30:93 | castToObjectArray(...) | semmle.label | castToObjectArray(...) | +| NonConstantTimeCryptoComparison.java:30:84:30:92 | actualMac : byte[] | semmle.label | actualMac : byte[] | +| NonConstantTimeCryptoComparison.java:37:21:37:29 | actualMac : byte[] | semmle.label | actualMac : byte[] | +| NonConstantTimeCryptoComparison.java:39:43:39:51 | actualMac | semmle.label | actualMac | +| NonConstantTimeCryptoComparison.java:57:28:57:40 | sign(...) : byte[] | semmle.label | sign(...) : byte[] | +| NonConstantTimeCryptoComparison.java:58:40:58:48 | signature | semmle.label | signature | +| NonConstantTimeCryptoComparison.java:68:21:68:29 | signature : byte[] | semmle.label | signature : byte[] | +| NonConstantTimeCryptoComparison.java:69:40:69:48 | signature | semmle.label | signature | +| NonConstantTimeCryptoComparison.java:86:22:86:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] | +| NonConstantTimeCryptoComparison.java:88:45:88:47 | tag | semmle.label | tag | +| NonConstantTimeCryptoComparison.java:98:24:98:26 | tag : byte[] | semmle.label | tag : byte[] | +| NonConstantTimeCryptoComparison.java:99:40:99:42 | tag | semmle.label | tag | +| NonConstantTimeCryptoComparison.java:108:52:108:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer | +| NonConstantTimeCryptoComparison.java:110:40:110:42 | tag : ByteBuffer | semmle.label | tag : ByteBuffer | +| NonConstantTimeCryptoComparison.java:110:40:110:50 | array(...) | semmle.label | array(...) | +| NonConstantTimeCryptoComparison.java:120:52:120:54 | tag : ByteBuffer | semmle.label | tag : ByteBuffer | +| NonConstantTimeCryptoComparison.java:121:49:121:51 | tag | semmle.label | tag | +| NonConstantTimeCryptoComparison.java:137:22:137:46 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] | +| NonConstantTimeCryptoComparison.java:138:40:138:42 | tag | semmle.label | tag | +| NonConstantTimeCryptoComparison.java:168:34:168:50 | doFinal(...) : byte[] | semmle.label | doFinal(...) : byte[] | +| NonConstantTimeCryptoComparison.java:171:26:171:36 | computedTag | semmle.label | computedTag | #select -| NonConstantTimeCryptoComparison.java:20:43:20:51 | actualMac | NonConstantTimeCryptoComparison.java:19:28:19:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:20:43:20:51 | actualMac | Using a non-constant time algorithm for comparing results of a cryptographic operation. | -| NonConstantTimeCryptoComparison.java:29:66:29:93 | castToObjectArray(...) | NonConstantTimeCryptoComparison.java:28:28:28:40 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:29:66:29:93 | castToObjectArray(...) | Using a non-constant time algorithm for comparing results of a cryptographic operation. | -| NonConstantTimeCryptoComparison.java:38:43:38:51 | actualMac | NonConstantTimeCryptoComparison.java:36:21:36:29 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:38:43:38:51 | actualMac | Using a non-constant time algorithm for comparing results of a cryptographic operation. | -| NonConstantTimeCryptoComparison.java:57:40:57:48 | signature | NonConstantTimeCryptoComparison.java:56:28:56:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:57:40:57:48 | signature | Using a non-constant time algorithm for comparing results of a cryptographic operation. | -| NonConstantTimeCryptoComparison.java:68:40:68:48 | signature | NonConstantTimeCryptoComparison.java:67:21:67:29 | signature : byte[] | NonConstantTimeCryptoComparison.java:68:40:68:48 | signature | Using a non-constant time algorithm for comparing results of a cryptographic operation. | -| NonConstantTimeCryptoComparison.java:87:45:87:47 | tag | NonConstantTimeCryptoComparison.java:85:22:85:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:87:45:87:47 | tag | Using a non-constant time algorithm for comparing results of a cryptographic operation. | -| NonConstantTimeCryptoComparison.java:98:40:98:42 | tag | NonConstantTimeCryptoComparison.java:97:24:97:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:98:40:98:42 | tag | Using a non-constant time algorithm for comparing results of a cryptographic operation. | -| NonConstantTimeCryptoComparison.java:109:40:109:50 | array(...) | NonConstantTimeCryptoComparison.java:107:52:107:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:109:40:109:50 | array(...) | Using a non-constant time algorithm for comparing results of a cryptographic operation. | -| NonConstantTimeCryptoComparison.java:120:49:120:51 | tag | NonConstantTimeCryptoComparison.java:119:52:119:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:120:49:120:51 | tag | Using a non-constant time algorithm for comparing results of a cryptographic operation. | +| NonConstantTimeCryptoComparison.java:21:43:21:51 | actualMac | NonConstantTimeCryptoComparison.java:20:28:20:44 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:21:43:21:51 | actualMac | Using a non-constant-time algorithm for comparing results of a cryptographic operation. | +| NonConstantTimeCryptoComparison.java:30:66:30:93 | castToObjectArray(...) | NonConstantTimeCryptoComparison.java:29:28:29:40 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:30:66:30:93 | castToObjectArray(...) | Using a non-constant-time algorithm for comparing results of a cryptographic operation. | +| NonConstantTimeCryptoComparison.java:39:43:39:51 | actualMac | NonConstantTimeCryptoComparison.java:37:21:37:29 | actualMac : byte[] | NonConstantTimeCryptoComparison.java:39:43:39:51 | actualMac | Using a non-constant-time algorithm for comparing results of a cryptographic operation. | +| NonConstantTimeCryptoComparison.java:58:40:58:48 | signature | NonConstantTimeCryptoComparison.java:57:28:57:40 | sign(...) : byte[] | NonConstantTimeCryptoComparison.java:58:40:58:48 | signature | Using a non-constant-time algorithm for comparing results of a cryptographic operation. | +| NonConstantTimeCryptoComparison.java:69:40:69:48 | signature | NonConstantTimeCryptoComparison.java:68:21:68:29 | signature : byte[] | NonConstantTimeCryptoComparison.java:69:40:69:48 | signature | Using a non-constant-time algorithm for comparing results of a cryptographic operation. | +| NonConstantTimeCryptoComparison.java:88:45:88:47 | tag | NonConstantTimeCryptoComparison.java:86:22:86:46 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:88:45:88:47 | tag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. | +| NonConstantTimeCryptoComparison.java:99:40:99:42 | tag | NonConstantTimeCryptoComparison.java:98:24:98:26 | tag : byte[] | NonConstantTimeCryptoComparison.java:99:40:99:42 | tag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. | +| NonConstantTimeCryptoComparison.java:110:40:110:50 | array(...) | NonConstantTimeCryptoComparison.java:108:52:108:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:110:40:110:50 | array(...) | Using a non-constant-time algorithm for comparing results of a cryptographic operation. | +| NonConstantTimeCryptoComparison.java:121:49:121:51 | tag | NonConstantTimeCryptoComparison.java:120:52:120:54 | tag : ByteBuffer | NonConstantTimeCryptoComparison.java:121:49:121:51 | tag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. | +| NonConstantTimeCryptoComparison.java:171:26:171:36 | computedTag | NonConstantTimeCryptoComparison.java:168:34:168:50 | doFinal(...) : byte[] | NonConstantTimeCryptoComparison.java:171:26:171:36 | computedTag | Using a non-constant-time algorithm for comparing results of a cryptographic operation. | diff --git a/java/ql/test/experimental/query-tests/security/CWE-208/NonConstantTimeCryptoComparison.java b/java/ql/test/experimental/query-tests/security/CWE-208/NonConstantTimeCryptoComparison.java index c611c64e7ed..cfeaec124a8 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-208/NonConstantTimeCryptoComparison.java +++ b/java/ql/test/experimental/query-tests/security/CWE-208/NonConstantTimeCryptoComparison.java @@ -1,3 +1,4 @@ +import java.io.InputStream; import java.net.Socket; import java.nio.ByteBuffer; import java.security.Key; @@ -11,7 +12,7 @@ import javax.crypto.Mac; public class NonConstantTimeCryptoComparison { - // BAD: compare MACs using a non-constant time method + // BAD: compare MACs using a non-constant-time method public boolean unsafeMacCheckWithArrayEquals(byte[] expectedMac, Socket socket) throws Exception { Mac mac = Mac.getInstance("HmacSHA256"); byte[] data = new byte[1024]; @@ -20,7 +21,7 @@ public class NonConstantTimeCryptoComparison { return Arrays.equals(expectedMac, actualMac); } - // BAD: compare MACs using a non-constant time method + // BAD: compare MACs using a non-constant-time method public boolean unsafeMacCheckWithArraysDeepEquals(byte[] expectedMac, Socket socket) throws Exception { Mac mac = Mac.getInstance("HmacSHA256"); byte[] data = socket.getInputStream().readAllBytes(); @@ -29,7 +30,7 @@ public class NonConstantTimeCryptoComparison { return Arrays.deepEquals(castToObjectArray(expectedMac), castToObjectArray(actualMac)); } - // BAD: compare MACs using a non-constant time method + // BAD: compare MACs using a non-constant-time method public boolean unsafeMacCheckWithDoFinalWithOutputArray(byte[] data, Socket socket) throws Exception { Mac mac = Mac.getInstance("HmacSHA256"); byte[] actualMac = new byte[256]; @@ -38,7 +39,7 @@ public class NonConstantTimeCryptoComparison { return Arrays.equals(expectedMac, actualMac); } - // GOOD: compare MACs using a constant time method + // GOOD: compare MACs using a constant-time method public boolean saferMacCheck(byte[] expectedMac, Socket socket) throws Exception { Mac mac = Mac.getInstance("HmacSHA256"); byte[] data = new byte[1024]; @@ -47,7 +48,7 @@ public class NonConstantTimeCryptoComparison { return MessageDigest.isEqual(expectedMac, actualMac); } - // BAD: compare signatures using a non-constant time method + // BAD: compare signatures using a non-constant-time method public boolean unsafeCheckSignatures(byte[] expected, Socket socket, PrivateKey key) throws Exception { Signature engine = Signature.getInstance("SHA256withRSA"); engine.initSign(key); @@ -57,7 +58,7 @@ public class NonConstantTimeCryptoComparison { return Arrays.equals(expected, signature); } - // BAD: compare signatures using a non-constant time method + // BAD: compare signatures using a non-constant-time method public boolean unsafeCheckSignaturesWithOutputArray(byte[] expected, Socket socket, PrivateKey key) throws Exception { Signature engine = Signature.getInstance("SHA256withRSA"); engine.initSign(key); @@ -68,7 +69,7 @@ public class NonConstantTimeCryptoComparison { return Arrays.equals(expected, signature); } - // GOOD: compare signatures using a constant time method + // GOOD: compare signatures using a constant-time method public boolean saferCheckSignatures(byte[] expected, Socket socket, PrivateKey key) throws Exception { Signature engine = Signature.getInstance("SHA256withRSA"); engine.initSign(key); @@ -78,7 +79,7 @@ public class NonConstantTimeCryptoComparison { return MessageDigest.isEqual(expected, signature); } - // BAD: compare ciphertexts using a non-constant time method + // BAD: compare ciphertexts using a non-constant-time method public boolean unsafeCheckCiphertext(Socket socket, byte[] plaintext, Key key) throws Exception { Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding"); cipher.init(Cipher.ENCRYPT_MODE, key); @@ -87,7 +88,7 @@ public class NonConstantTimeCryptoComparison { return Objects.deepEquals(expected, tag); } - // BAD: compare ciphertexts using a non-constant time method + // BAD: compare ciphertexts using a non-constant-time method public boolean unsafeCheckCiphertextWithOutputArray(byte[] expected, Socket socket, Key key) throws Exception { Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding"); cipher.init(Cipher.ENCRYPT_MODE, key); @@ -98,7 +99,7 @@ public class NonConstantTimeCryptoComparison { return Arrays.equals(expected, tag); } - // BAD: compare ciphertexts using a non-constant time method + // BAD: compare ciphertexts using a non-constant-time method public boolean unsafeCheckCiphertextWithByteBuffer(Socket socket, byte[] plaintext, Key key) throws Exception { Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding"); cipher.init(Cipher.ENCRYPT_MODE, key); @@ -109,7 +110,7 @@ public class NonConstantTimeCryptoComparison { return Arrays.equals(expected, tag.array()); } - // BAD: compare ciphertexts using a non-constant time method + // BAD: compare ciphertexts using a non-constant-time method public boolean unsafeCheckCiphertextWithByteBufferEquals(byte[] expected, Socket socket, Key key) throws Exception { Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding"); cipher.init(Cipher.ENCRYPT_MODE, key); @@ -120,7 +121,7 @@ public class NonConstantTimeCryptoComparison { return ByteBuffer.wrap(expected).equals(tag); } - // GOOD: compare ciphertexts using a constant time method + // GOOD: compare ciphertexts using a constant-time method public boolean saferCheckCiphertext(Socket socket, byte[] plaintext, Key key) throws Exception { Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding"); cipher.init(Cipher.ENCRYPT_MODE, key); @@ -129,7 +130,7 @@ public class NonConstantTimeCryptoComparison { return MessageDigest.isEqual(expected, tag); } - // GOOD: compare ciphertexts using a constant time method, but no user input + // GOOD: compare ciphertexts using a constant-time method, but no user input public boolean noUserInputWhenCheckingCiphertext(byte[] expected, byte[] plaintext, Key key) throws Exception { Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5Padding"); cipher.init(Cipher.ENCRYPT_MODE, key); @@ -137,7 +138,7 @@ public class NonConstantTimeCryptoComparison { return Arrays.equals(expected, tag); } - // GOOD: compare MAC with constant using a constant time method + // GOOD: compare MAC with constant using a constant-time method public boolean compareMacWithConstant(Socket socket) throws Exception { Mac mac = Mac.getInstance("HmacSHA256"); byte[] data = new byte[1024]; @@ -153,5 +154,50 @@ public class NonConstantTimeCryptoComparison { } return result; } + + // BAD: compare MAC using a non-constant-time loop + public boolean unsafeMacCheckWithLoop(Socket socket) throws Exception { + try (InputStream is = socket.getInputStream()) { + byte[] data = new byte[256]; + byte[] tag = new byte[32]; + + is.read(data); + is.read(tag); + + Mac mac = Mac.getInstance("Hmac256"); + byte[] computedTag = mac.doFinal(data); + + for (int i = 0; i < computedTag.length; i++) { + byte a = computedTag[i]; + byte b = tag[i]; + if (a != b) { + return false; + } + } + + return true; + } + } + + // GOOD: compare MAC using a constant-time loop + public boolean safeMacCheckWithLoop(Socket socket) throws Exception { + try (InputStream is = socket.getInputStream()) { + byte[] data = new byte[256]; + byte[] tag = new byte[32]; + + is.read(data); + is.read(tag); + + Mac mac = Mac.getInstance("Hmac256"); + byte[] computedTag = mac.doFinal(data); + + int result = 0; + for (int i = 0; i < computedTag.length; i++) { + result |= computedTag[i] ^ tag[i]; + } + + return result == 0; + } + } } \ No newline at end of file