Renamed queries and merged qhelp files

Author: artem-smotrakov

java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCheckOnSignature.qhelp

@@ -0,0 +1,4 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+<include src="TimingAttackAgainstSignature.qhelp" />
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCheckRecommendation.inc.qhelp

@@ -1,13 +1,13 @@
 /**
- * @name Using a non-constant-time algorithm for checking a signature
+ * @name Possible timing attack against signature validation
  * @description When checking a signature over a message, a constant-time algorithm should be used.
  *              Otherwise, there is a risk of a timing attack that allows an attacker
  *              to forge a valid signature for an arbitrary message. For a successful attack,
  *              the attacker has to be able to send to the validation procedure both the message and the signature.
  * @kind path-problem
  * @problem.severity warning
  * @precision medium
- * @id java/non-constant-time-in-signature-check
+ * @id java/possible-timing-attack-against-signature
  * @tags security
  *       external/cwe/cwe-208
  */
@@ -18,5 +18,5 @@ import DataFlow::PathGraph
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, NonConstantTimeCryptoComparisonConfig conf
 where conf.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "Using a non-constant-time method for checking a $@.", source,
+select sink.getNode(), source, sink, "Possible timing attack against $@ validation.", source,
   source.getNode().(CryptoOperationSource).getCall().getResultType()

java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCheckReferences.inc.qhelp

@@ -1,4 +1,7 @@
-public boolean check(byte[] signature, byte[] message, SecretKey key) throws Exception {
+public boolean validate(HttpRequest request, SecretKey key) throws Exception {
+    byte[] message = getMessageFrom(request);
+    byte[] signature = getSignatureFrom(request);
+
     Mac mac = Mac.getInstance("HmacSHA256");
     mac.init(new SecretKeySpec(key.getEncoded(), "HmacSHA256"));
     byte[] actual = mac.doFinal(message);

java/ql/src/experimental/Security/CWE/CWE-208/PossibleTimingAttackAgainstSignature.qhelp

@@ -11,21 +11,45 @@ both the message and the signature. A successful attack can result in authentica
 </p>
 </overview>
 
-<include src="NonConstantTimeCheckRecommendation.inc.qhelp" />
+<recommendation>
+<p>
+Use <code>MessageDigest.isEqual()</code> method to check MACs and signatures.
+If this method is used, then the calculation time depends only on the length of input byte arrays,
+and does not depend on the contents of the arrays.
+</p>
+</recommendation>
 
 <example>
 <p>
 The following example uses <code>Arrays.equals()</code> method for validating a MAC over a message.
 This method implements a non-constant-time algorithm.
 Both the message and the signature come from an untrusted HTTP request:
 </p>
-<sample src="UnsafeMacComparisonWithRemoteInputs.java" />
+<sample src="UnsafeMacComparison.java" />
 
 <p>
 The next example uses a safe constant-time algorithm for validating a MAC:
 </p>
-<sample src="SafeMacComparisonWithRemoteInputs.java" />
+<sample src="SafeMacComparison.java" />
 </example>
 
-<include src="NonConstantTimeCheckReferences.inc.qhelp" />
+<references>
+<li>
+  Wikipedia:
+  <a href="https://en.wikipedia.org/wiki/Timing_attack">Timing attack</a>.
+</li>
+<li>
+  Coursera:
+  <a href="https://www.coursera.org/lecture/crypto/timing-attacks-on-mac-verification-FHGW1">Timing attacks on MAC verification</a>
+</li>
+<li>
+  NCC Group:
+  <a href="https://www.nccgroup.trust/globalassets/our-research/us/whitepapers/TimeTrial.pdf">Time Trial: Racing Towards Practical Remote Timing Attacks</a>
+</li>
+<li>
+  Java API Specification:
+  <a href="https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/security/MessageDigest.html#isEqual(byte[],byte[])">MessageDigest.isEqual() method</a>
+</li>
+</references>
+
 </qhelp>

java/ql/src/experimental/Security/CWE/CWE-208/NonConstantTimeCheckOnSignature.ql renamed to java/ql/src/experimental/Security/CWE/CWE-208/PossibleTimingAttackAgainstSignature.ql

@@ -1,4 +1,7 @@
-public boolean check(byte[] signature, byte[] message, SecretKey key) throws Exception {
+public boolean validate(HttpRequest request, SecretKey key) throws Exception {
+    byte[] message = getMessageFrom(request);
+    byte[] signature = getSignatureFrom(request);
+
     Mac mac = Mac.getInstance("HmacSHA256");
     mac.init(new SecretKeySpec(key.getEncoded(), "HmacSHA256"));
     byte[] actual = mac.doFinal(message);