Fix #73 Credentials should not be hard-coded(jproperties:S2068) - Add…

… the ability to ignore encrypted values
racodond · Dec 11, 2016 · 1acbaf0 · 1acbaf0
1 parent 4533291
commit 1acbaf0
Show file tree

Hide file tree

Showing 21 changed files with 132 additions and 57 deletions.
diff --git a/...ts/custom/hardCodedCredentials.properties → ...edentials/hardCodedCredentials.properties b/...ts/custom/hardCodedCredentials.properties → ...edentials/hardCodedCredentials.properties
@@ -1,18 +1,18 @@
 # Noncompliant {{Remove this hard-coded username.}}
-myloginzzz=blabla
+hcc.myloginzzz=hcc1
 
 # Noncompliant {{Remove this hard-coded username.}}
-username=blabla
+hcc.username=hcc2
 
 # Noncompliant {{Remove this hard-coded password.}}
-my_password=blabla
+hcc.password=hcc13
 
 # Noncompliant {{Remove this hard-coded password.}}
-mypasswd=blabla
+hcc.mypasswd=hcc4
 
 # Noncompliant {{Remove this hard-coded password.}}
-pwd=blabla
+hcc.pwd=hcc5
 
 # Compliant
-abc=blabla
-pass=blabla
+hcc.abc=hcc6
+hcc.pass=hcc7
diff --git a/...ling/projects/custom/hard-coded-credentials/hardCodedCredentialsValuesToIgnore.properties b/...ling/projects/custom/hard-coded-credentials/hardCodedCredentialsValuesToIgnore.properties
@@ -0,0 +1,5 @@
+hccvi.myloginzzz=ENC(hccvi1)
+hccvi.username=OBF:hccvi2
+hccvi.password=ENC(hccvi3)
+hccvi.mypasswd=OBF:hccvi4
+hccvi.pwd=OBF:hccvi5
diff --git a/its/ruling/tests/src/test/expected/jproperties-S2068.json b/its/ruling/tests/src/test/expected/jproperties-S2068.json
@@ -1,5 +1,5 @@
 {
-'project:custom/hardCodedCredentials.properties':[
+'project:custom/hard-coded-credentials/hardCodedCredentials.properties':[
 2,
 5,
 8,

diff --git a/its/ruling/tests/src/test/expected/jproperties-duplicated-values.json b/its/ruling/tests/src/test/expected/jproperties-duplicated-values.json
@@ -20,9 +20,6 @@
 'project:custom/emptyElement.properties':[
 1,
 ],
-'project:custom/hardCodedCredentials.properties':[
-2,
-],
 'project:custom/indentation/indentation.properties':[
 1,
 ],

diff --git a/its/ruling/tests/src/test/expected/jproperties-key-naming-convention.json b/its/ruling/tests/src/test/expected/jproperties-key-naming-convention.json
@@ -15,9 +15,6 @@
 27,
 28,
 ],
-'project:custom/hardCodedCredentials.properties':[
-8,
-],
 'project:custom/keyNamingConvention.properties':[
 6,
 10,

diff --git a/its/ruling/tests/src/test/java/org/sonar/jproperties/its/ProfileGenerator.java b/its/ruling/tests/src/test/java/org/sonar/jproperties/its/ProfileGenerator.java
@@ -40,6 +40,7 @@ public class ProfileGenerator {
 
   private static Multimap<String, Parameter> parameters = ImmutableListMultimap.<String, Parameter>builder()
     .put("maximum-number-keys", new Parameter("numberKeys", "40"))
+    .put("S2068", new Parameter("encryptedCredentialsToIgnore", "^(ENC\\(|OBF:).+$"))
     .build();
 
   public static void generateProfile(Orchestrator orchestrator) {

diff --git a/jproperties-checks/src/main/java/org/sonar/jproperties/checks/CheckUtils.java b/jproperties-checks/src/main/java/org/sonar/jproperties/checks/CheckUtils.java
@@ -30,7 +30,7 @@ public class CheckUtils {
   private CheckUtils() {
   }
 
-  public static String paramsErrorMessage(Class<? extends JavaPropertiesCheck> clazz, String message) {
+  public static String paramErrorMessage(Class<? extends JavaPropertiesCheck> clazz, String message) {
     return "Check jproperties:" + clazz.getAnnotation(Rule.class).key()
       + " (" + clazz.getAnnotation(Rule.class).name() + "): "
       + message;

diff --git a/...ies-checks/src/main/java/org/sonar/jproperties/checks/generic/CommentConventionCheck.java b/...ies-checks/src/main/java/org/sonar/jproperties/checks/generic/CommentConventionCheck.java
@@ -74,7 +74,7 @@ public void visitComment(SyntaxTrivia trivia) {
   @Override
   public void validateParameters() {
     if (!Arrays.asList("#", "!").contains(startingCommentToken)) {
-      throw new IllegalStateException(paramsErrorMessage());
+      throw new IllegalStateException(paramErrorMessage());
     }
   }
 
@@ -83,8 +83,8 @@ void setStartingCommentToken(String startingCommentToken) {
     this.startingCommentToken = startingCommentToken;
   }
 
-  private String paramsErrorMessage() {
-    return CheckUtils.paramsErrorMessage(
+  private String paramErrorMessage() {
+    return CheckUtils.paramErrorMessage(
       this.getClass(),
       "startingCommentToken parameter is not valid.\nActual: '" + startingCommentToken + "'\n" + "Expected: '#' or '!'");
   }

diff --git a/...cks/src/main/java/org/sonar/jproperties/checks/generic/CommentRegularExpressionCheck.java b/...cks/src/main/java/org/sonar/jproperties/checks/generic/CommentRegularExpressionCheck.java
@@ -65,12 +65,12 @@ public void validateParameters() {
     try {
       Pattern.compile(regularExpression);
     } catch (PatternSyntaxException exception) {
-      throw new IllegalStateException(paramsErrorMessage(), exception);
+      throw new IllegalStateException(paramErrorMessage(), exception);
     }
   }
 
-  private String paramsErrorMessage() {
-    return CheckUtils.paramsErrorMessage(
+  private String paramErrorMessage() {
+    return CheckUtils.paramErrorMessage(
       this.getClass(),
       "regularExpression parameter \"" + regularExpression + "\" is not a valid regular expression.");
   }

diff --git a/...ties-checks/src/main/java/org/sonar/jproperties/checks/generic/DuplicatedValuesCheck.java b/...ties-checks/src/main/java/org/sonar/jproperties/checks/generic/DuplicatedValuesCheck.java
@@ -94,7 +94,7 @@ public void validateParameters() {
     try {
       Pattern.compile(valuesToIgnore);
     } catch (PatternSyntaxException exception) {
-      throw new IllegalStateException(paramsErrorMessage(), exception);
+      throw new IllegalStateException(paramErrorMessage(), exception);
     }
   }
 
@@ -127,8 +127,8 @@ private static String valueWithoutLineBreak(String value) {
     return value.replaceAll("\\\\\\n\\s*", "");
   }
 
-  private String paramsErrorMessage() {
-    return CheckUtils.paramsErrorMessage(
+  private String paramErrorMessage() {
+    return CheckUtils.paramErrorMessage(
       this.getClass(),
       "valuesToIgnore parameter \"" + valuesToIgnore + "\" is not a valid regular expression.");
   }

diff --git a/...ies-checks/src/main/java/org/sonar/jproperties/checks/generic/EndLineCharactersCheck.java b/...ies-checks/src/main/java/org/sonar/jproperties/checks/generic/EndLineCharactersCheck.java
@@ -65,7 +65,7 @@ public void visitProperties(PropertiesTree tree) {
   @Override
   public void validateParameters() {
     if (!Arrays.asList("CRLF", "CR", "LF").contains(endLineCharacters)) {
-      throw new IllegalStateException(paramsErrorMessage());
+      throw new IllegalStateException(paramErrorMessage());
     }
   }
 
@@ -90,8 +90,8 @@ private boolean fileContainsIllegalEndLineCharacters() {
     }
   }
 
-  private String paramsErrorMessage() {
-    return CheckUtils.paramsErrorMessage(
+  private String paramErrorMessage() {
+    return CheckUtils.paramErrorMessage(
         this.getClass(),
         "endLineCharacters parameter is not valid.\nActual: '" + endLineCharacters + "'\nExpected: 'CR' or 'CRLF' or 'LF'");
   }

diff --git a/jproperties-checks/src/main/java/org/sonar/jproperties/checks/generic/FileNameCheck.java b/jproperties-checks/src/main/java/org/sonar/jproperties/checks/generic/FileNameCheck.java
@@ -65,7 +65,7 @@ public void validateParameters() {
       Pattern.compile(format);
     } catch (PatternSyntaxException exception) {
       throw new IllegalStateException(
-        CheckUtils.paramsErrorMessage(this.getClass(), "format parameter \"" + format + "\" is not a valid regular expression."),
+        CheckUtils.paramErrorMessage(this.getClass(), "format parameter \"" + format + "\" is not a valid regular expression."),
         exception);
     }
   }

diff --git a/...-checks/src/main/java/org/sonar/jproperties/checks/generic/HardCodedCredentialsCheck.java b/...-checks/src/main/java/org/sonar/jproperties/checks/generic/HardCodedCredentialsCheck.java
@@ -19,16 +19,20 @@
  */
 package org.sonar.jproperties.checks.generic;
 
-import java.util.regex.Pattern;
-
+import com.google.common.annotations.VisibleForTesting;
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
+import org.sonar.check.RuleProperty;
+import org.sonar.jproperties.checks.CheckUtils;
 import org.sonar.jproperties.checks.Tags;
-import org.sonar.plugins.jproperties.api.tree.KeyTree;
+import org.sonar.plugins.jproperties.api.tree.PropertyTree;
 import org.sonar.plugins.jproperties.api.visitors.DoubleDispatchVisitorCheck;
 import org.sonar.squidbridge.annotations.ActivatedByDefault;
 import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
 
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+
 @Rule(
   key = "S2068",
   name = "Credentials should not be hard-coded",
@@ -38,18 +42,56 @@
 @ActivatedByDefault
 public class HardCodedCredentialsCheck extends DoubleDispatchVisitorCheck {
 
+  private static final String DEFAULT_ENCRYPTED_CREDENTIALS_TO_IGNORE = "";
+
   private static final Pattern HARD_CODED_USERNAME = Pattern.compile(".*(login|username).*", Pattern.CASE_INSENSITIVE);
   private static final Pattern HARD_CODED_PASSWORD = Pattern.compile(".*(password|passwd|pwd).*", Pattern.CASE_INSENSITIVE);
 
+  @RuleProperty(
+    key = "encryptedCredentialsToIgnore",
+    description = "Regular expression of encrypted credentials to ignore. "
+      + "See " + CheckUtils.LINK_TO_JAVA_REGEX_PATTERN_DOC + " for detailed regular expression syntax. "
+      + "For example, to ignore encrypted credentials starting with 'ENC(' and 'OBF:', set the parameter to '^(ENC\\(|OBF:).+$'. "
+      + "Leave empty if encrypted credentials should not be ignored.",
+    defaultValue = DEFAULT_ENCRYPTED_CREDENTIALS_TO_IGNORE)
+  private String encryptedCredentialsToIgnore = DEFAULT_ENCRYPTED_CREDENTIALS_TO_IGNORE;
+
   @Override
-  public void visitKey(KeyTree tree) {
-    if (HARD_CODED_USERNAME.matcher(tree.text()).matches()) {
-      addPreciseIssue(tree, "Remove this hard-coded username.");
+  public void visitProperty(PropertyTree tree) {
+    if (tree.value() == null) {
+      return;
     }
-    if (HARD_CODED_PASSWORD.matcher(tree.text()).matches()) {
+
+    if (HARD_CODED_USERNAME.matcher(tree.key().text()).matches()
+      && ("".equals(encryptedCredentialsToIgnore) || (!"".equals(encryptedCredentialsToIgnore) && !tree.value().text().matches(encryptedCredentialsToIgnore)))) {
+      addPreciseIssue(tree, "Remove this hard-coded username.");
+
+    } else if (HARD_CODED_PASSWORD.matcher(tree.key().text()).matches()
+      && ("".equals(encryptedCredentialsToIgnore) || (!"".equals(encryptedCredentialsToIgnore) && !tree.value().text().matches(encryptedCredentialsToIgnore)))) {
       addPreciseIssue(tree, "Remove this hard-coded password.");
     }
-    super.visitKey(tree);
+    super.visitProperty(tree);
+
+  }
+
+  @Override
+  public void validateParameters() {
+    try {
+      Pattern.compile(encryptedCredentialsToIgnore);
+    } catch (PatternSyntaxException exception) {
+      throw new IllegalStateException(paramErrorMessage(), exception);
+    }
+  }
+
+  @VisibleForTesting
+  void setEncryptedCredentialsToIgnore(String encryptedCredentialsToIgnore) {
+    this.encryptedCredentialsToIgnore = encryptedCredentialsToIgnore;
+  }
+
+  private String paramErrorMessage() {
+    return CheckUtils.paramErrorMessage(
+      this.getClass(),
+      "encryptedCredentialsToIgnore parameter \"" + encryptedCredentialsToIgnore + "\" is not a valid regular expression.");
   }
 
 }
diff --git a/...s-checks/src/main/java/org/sonar/jproperties/checks/generic/KeyNamingConventionCheck.java b/...s-checks/src/main/java/org/sonar/jproperties/checks/generic/KeyNamingConventionCheck.java
@@ -64,7 +64,7 @@ public void validateParameters() {
     try {
       Pattern.compile(format);
     } catch (PatternSyntaxException exception) {
-      throw new IllegalStateException(paramsErrorMessage(), exception);
+      throw new IllegalStateException(paramErrorMessage(), exception);
     }
   }
 
@@ -73,8 +73,8 @@ void setFormat(String format) {
     this.format = format;
   }
 
-  private String paramsErrorMessage() {
-    return CheckUtils.paramsErrorMessage(
+  private String paramErrorMessage() {
+    return CheckUtils.paramErrorMessage(
       this.getClass(),
       "format parameter \"" + format + "\" is not a valid regular expression.");
   }

diff --git a/...-checks/src/main/java/org/sonar/jproperties/checks/generic/KeyRegularExpressionCheck.java b/...-checks/src/main/java/org/sonar/jproperties/checks/generic/KeyRegularExpressionCheck.java
@@ -65,12 +65,12 @@ public void validateParameters() {
     try {
       Pattern.compile(regularExpression);
     } catch (PatternSyntaxException exception) {
-      throw new IllegalStateException(paramsErrorMessage(), exception);
+      throw new IllegalStateException(paramErrorMessage(), exception);
     }
   }
 
-  private String paramsErrorMessage() {
-    return CheckUtils.paramsErrorMessage(
+  private String paramErrorMessage() {
+    return CheckUtils.paramErrorMessage(
       this.getClass(),
       "regularExpression parameter \"" + regularExpression + "\" is not a valid regular expression.");
   }

diff --git a/...s-checks/src/main/java/org/sonar/jproperties/checks/generic/SeparatorConventionCheck.java b/...s-checks/src/main/java/org/sonar/jproperties/checks/generic/SeparatorConventionCheck.java
@@ -89,7 +89,7 @@ public void visitProperty(PropertyTree tree) {
   @Override
   public void validateParameters() {
     if (!Arrays.asList("=", ":").contains(separator)) {
-      throw new IllegalStateException(paramsErrorMessage());
+      throw new IllegalStateException(paramErrorMessage());
     }
   }
 
@@ -98,8 +98,8 @@ void setSeparator(String separator) {
     this.separator = separator;
   }
 
-  private String paramsErrorMessage() {
-    return CheckUtils.paramsErrorMessage(
+  private String paramErrorMessage() {
+    return CheckUtils.paramErrorMessage(
       this.getClass(),
       "separator parameter is not valid.\nActual: \"" + separator + "\"\nExpected: '=' or ':'");
   }

diff --git a/...hecks/src/main/java/org/sonar/jproperties/checks/generic/ValueRegularExpressionCheck.java b/...hecks/src/main/java/org/sonar/jproperties/checks/generic/ValueRegularExpressionCheck.java
@@ -65,12 +65,12 @@ public void validateParameters() {
     try {
       Pattern.compile(regularExpression);
     } catch (PatternSyntaxException exception) {
-      throw new IllegalStateException(paramsErrorMessage(), exception);
+      throw new IllegalStateException(paramErrorMessage(), exception);
     }
   }
 
-  private String paramsErrorMessage() {
-    return CheckUtils.paramsErrorMessage(
+  private String paramErrorMessage() {
+    return CheckUtils.paramErrorMessage(
       this.getClass(),
       "regularExpression parameter \"" + regularExpression + "\" is not a valid regular expression.");
   }

diff --git a/...perties-checks/src/main/resources/org/sonar/l10n/jproperties/rules/jproperties/S2068.html b/...perties-checks/src/main/resources/org/sonar/l10n/jproperties/rules/jproperties/S2068.html
@@ -8,6 +8,11 @@
     This rule flags instances of hard-coded credentials. It checks for keys containing <code>login</code>,
     <code>username</code>, <code>password</code>, <code>passwd</code> or <code>pwd</code>.
 </p>
+<p>
+    Even if it is not very secure, encrypted credentials can be set in Java .properties files. By default, issues are
+    raised on those encrypted credentials. But, if you don't want issues to be raised on encrypted credentials, you can
+    set the <code>encryptedCredentialsToIgnore</code> parameter.
+</p>
 
 <h2>Noncompliant Code Example</h2>
 <pre>

diff --git a/...cks/src/test/java/org/sonar/jproperties/checks/generic/HardCodedCredentialsCheckTest.java b/...cks/src/test/java/org/sonar/jproperties/checks/generic/HardCodedCredentialsCheckTest.java
@@ -21,14 +21,37 @@
 
 import org.junit.Test;
 import org.sonar.jproperties.checks.CheckTestUtils;
-import org.sonar.jproperties.checks.generic.HardCodedCredentialsCheck;
 import org.sonar.jproperties.checks.verifier.JavaPropertiesCheckVerifier;
 
+import static org.fest.assertions.Assertions.assertThat;
+
 public class HardCodedCredentialsCheckTest {
 
   @Test
   public void should_find_some_hard_coded_credentials_and_raise_some_issues() {
-    JavaPropertiesCheckVerifier.verify(new HardCodedCredentialsCheck(), CheckTestUtils.getTestFile("hardCodedCredentials.properties"));
+    JavaPropertiesCheckVerifier.verify(new HardCodedCredentialsCheck(), CheckTestUtils.getTestFile("hard-coded-credentials/hardCodedCredentials.properties"));
+  }
+
+  @Test
+  public void should_not_find_some_hard_coded_credentials_because_values_to_ignore_are_set() {
+    HardCodedCredentialsCheck check = new HardCodedCredentialsCheck();
+    check.setEncryptedCredentialsToIgnore("^(ENC\\(|OBF:).+$");
+    JavaPropertiesCheckVerifier.verify(check, CheckTestUtils.getTestFile("hard-coded-credentials/hardCodedCredentialsValuesToIgnore.properties"));
+  }
+
+  @Test
+  public void should_throw_an_illegal_state_exception_as_the_encryptedCredentialsToIgnore_parameter_is_not_a_valid_regular_expression() {
+    try {
+      HardCodedCredentialsCheck check = new HardCodedCredentialsCheck();
+      check.setEncryptedCredentialsToIgnore("(");
+
+      JavaPropertiesCheckVerifier.issues(check, CheckTestUtils.getTestFile("hard-coded-credentials/hardCodedCredentials.properties")).noMore();
+
+    } catch (IllegalStateException e) {
+      assertThat(e.getMessage()).isEqualTo("Check jproperties:S2068 (Credentials should not be hard-coded): "
+        + "encryptedCredentialsToIgnore parameter \"(\" is not a valid regular expression.");
+    }
   }
 
+
 }
diff --git a/...es/checks/hardCodedCredentials.properties → ...edentials/hardCodedCredentials.properties b/...es/checks/hardCodedCredentials.properties → ...edentials/hardCodedCredentials.properties
@@ -1,18 +1,18 @@
 # Noncompliant {{Remove this hard-coded username.}}
-myloginzzz=blabla
+hcc.myloginzzz=hcc1
 
 # Noncompliant {{Remove this hard-coded username.}}
-username=blabla
+hcc.username=hcc2
 
 # Noncompliant {{Remove this hard-coded password.}}
-my_password=blabla
+hcc.password=hcc13
 
 # Noncompliant {{Remove this hard-coded password.}}
-mypasswd=blabla
+hcc.mypasswd=hcc4
 
 # Noncompliant {{Remove this hard-coded password.}}
-pwd=blabla
+hcc.pwd=hcc5
 
 # Compliant
-abc=blabla
-pass=blabla
+hcc.abc=hcc6
+hcc.pass=hcc7