doc: Merge two descriptions of insecure-compare

doc/rules/insecureCompare.md

@@ -2,41 +2,38 @@
 
 Identifies cases in which secrets are being compared directly using `string.equals`.
 
-## Rule logic
-
-Every event that satisfies the following conditions is checked:
-
-- labeled `string.equals`
-- takes one argument
+Ordinary string comparison of user-provided data with a piece of secret data can leak information
+about the secret through [timing attacks](https://en.wikipedia.org/wiki/Timing_attack), because of
+short-circuting (returning false on first mismatch). This can allow the attacker to significantly
+speed up brute forcing, turning an intractable exponential problem into a linear one.
 
-The rule examines both the `receiver` (aka `this / self` object) and the single `argument`. If both
-values are un-bcrypted secrets, then a finding is reported.
+## Rule logic
 
-A value is considered a secret if:
+The rule looks for events labeled `secret` and `string.equals`.
 
-- it is generated by an event labeled `secret`
-- it's a match to the list of known secret regular expressions
+On encountering an event labeled `secret` it remembers the return value.
 
-A value is considered bcrypted if it matches the bcrypt regular expression.
+On encountering `string.equals` it looks at the arguments and the receiver. If any of the arguments
+is a previously seen secret, or matches a list of known secret-like regular expressions, the
+comparison is flagged as insecure, unless any of the arguments is a BCrypt digest.
 
 ## Notes
 
-Direct comparison of secrets using `string.equals` is insecure, because it's vulnerable to a
-[timing attck](https://en.wikipedia.org/wiki/Timing_attack).
+When generating appmaps, ensure that string comparison functions (such as `String#==` in Ruby and
+`String.equals` in Java) are traced and correctly labeled with `string.equals`. Any functions you
+know to return a secret (eg. a model accessor for an API key) should be labeled `secret`.
 
-This rule can introduce a signifacnt performance penalty to the application code, because it records
-every string comparison.
+Since using this rule generally requires labeling all string comparisons its use is currently
+limited due to performance and space overhead of tracing them: string comparisons are commonly used
+throughout the code base and libraries.
 
-## Resolution
+It's advisable to only trace string equality on a limited subset of tests (perhaps the ones known to
+touch secret related functionality); alas, this is not something easily attainable with current
+AppMap recorders and requires swapping or modifying configuration files.
 
-Replace direct string comparison of secrets with a secure compare function from a standard library.
-
-## Options
-
-None
-
-## Examples
+## Resolution
 
-```yaml
-- rule: insecureCompare
-```
+To fix this issue, either hash values before comparing or use a constant-time comparison in such
+operations. Constant-time comparison functions always compare every character of the string,
+removing the timing side channel. You can usually find functions like this in cryptographic and
+utility libraries, eg. `ActiveSupport::SecurityUtils.secure_compare`.