Obfuscate passwords properly (#171)

* Include newlines in password obfuscation pattern
* Obfuscate failed operations with passwords

CHANGES.md

@@ -1,5 +1,8 @@
 # Changes
 
+## Version 2.8.0
+* Obfuscate passwords properly - #170
+
 ## Version 2.7.0
 * Build with Cassandra 3.11.10 (only flavor ecaudit_c3.11)
 * Build with Cassandra 3.0.24 (only flavor ecaudit_c3.0)

ecaudit/src/main/java/com/ericsson/bss/cassandra/ecaudit/entry/AuditEntry.java

@@ -46,6 +46,7 @@ public class AuditEntry implements AuditRecord
     private final Status status;
     private final Long timestamp;
     private final String subject;
+    private final boolean hasKnownOperation;
 
     /**
      * @see #newBuilder()
@@ -62,6 +63,7 @@ private AuditEntry(Builder builder)
         this.status = builder.status;
         this.timestamp = builder.timestamp;
         this.subject = builder.subject;
+        this.hasKnownOperation = builder.hasKnownOperation;
     }
 
     @Override
@@ -145,6 +147,14 @@ public Optional<String> getSubject()
         return Optional.ofNullable(subject);
     }
 
+    /**
+     * @return True if the statement was parsed successfully.
+     */
+    public boolean hasKnownOperation()
+    {
+        return hasKnownOperation;
+    }
+
     /**
      * Create a new {@link Builder} instance.
      *
@@ -170,6 +180,7 @@ public static class Builder
         private Status status;
         private Long timestamp;
         private String subject;
+        private boolean hasKnownOperation = true;
 
         public Builder client(InetSocketAddress address)
         {
@@ -255,6 +266,12 @@ public Builder timestamp(Long timestamp)
             return this;
         }
 
+        public Builder knownOperation(boolean hasKnownOperation)
+        {
+            this.hasKnownOperation = hasKnownOperation;
+            return this;
+        }
+
         /**
          * Configure this builder from an existing {@link AuditEntry} instance.
          *
@@ -273,6 +290,7 @@ public Builder basedOn(AuditEntry entry)
             this.status = entry.getStatus();
             this.timestamp = entry.getTimestamp();
             this.subject = entry.getSubject().orElse(null);
+            this.hasKnownOperation = entry.hasKnownOperation();
             return this;
         }
 

ecaudit/src/main/java/com/ericsson/bss/cassandra/ecaudit/entry/factory/AuditEntryBuilderFactory.java

@@ -112,7 +112,7 @@ public Builder createEntryBuilder(String operation, ClientState state)
         catch (RuntimeException e)
         {
             LOG.debug("Failed to parse or prepare statement - assuming default permissions and resources", e);
-            return createDefaultEntryBuilder();
+            return createDefaultEntryBuilder().knownOperation(false);
         }
     }
 
@@ -186,7 +186,7 @@ private Builder createEntryBuilder(ParsedStatement parsedStatement)
         }
 
         LOG.warn("Detected unrecognized CQLStatement in audit mapping");
-        return createDefaultEntryBuilder();
+        return createDefaultEntryBuilder().knownOperation(false);
     }
 
     @SuppressWarnings("PMD")
@@ -226,7 +226,7 @@ public Builder createEntryBuilder(CQLStatement statement)
         }
 
         LOG.warn("Detected unrecognized CQLStatement in audit mapping");
-        return createDefaultEntryBuilder();
+        return createDefaultEntryBuilder().knownOperation(false);
     }
 
     public Builder createBatchEntryBuilder()

ecaudit/src/main/java/com/ericsson/bss/cassandra/ecaudit/obfuscator/PasswordObfuscator.java

@@ -36,18 +36,18 @@ public class PasswordObfuscator implements AuditObfuscator
 {
     private final static String PASSWORD_OBFUSCATED = "*****";
 
+    private static final int PATTERN_FLAGS = Pattern.CASE_INSENSITIVE | Pattern.DOTALL;
+
     private final static String REGEX_PASSWORD_GROUP = "password";
-    private final static Pattern QUERY_CREATE_ALTER_USER_PATTERN =
-            Pattern.compile(".*password\\s+'(?<password>[^\\s]+)'.*", Pattern.CASE_INSENSITIVE);
-    private final static Pattern QUERY_CREATE_ALTER_ROLE_PATTERN =
-            Pattern.compile(".*password\\s*=\\s*'(?<password>[^\\s]+)'.*", Pattern.CASE_INSENSITIVE);
+    private final static Pattern PASSWORD_PATTERN =
+            Pattern.compile(".*password\\s*=?\\s*'(?<password>[^\\s]+)'.*", PATTERN_FLAGS);
 
     private final static Set<Permission> PASSWORD_PERMISSIONS = ImmutableSet.of(Permission.CREATE, Permission.ALTER);
 
     @Override
-    public AuditEntry obfuscate(final AuditEntry entry)
+    public AuditEntry obfuscate(AuditEntry entry)
     {
-        if (isRoleResource(entry.getResource()) && isPasswordPermission(entry.getPermissions()))
+        if (shouldObfuscate(entry))
         {
             AuditEntry obfuscatedEntry = entry;
 
@@ -65,6 +65,12 @@ public AuditEntry obfuscate(final AuditEntry entry)
         return entry;
     }
 
+    private boolean shouldObfuscate(AuditEntry entry)
+    {
+        return !entry.hasKnownOperation()
+               || isRoleResource(entry.getResource()) && isPasswordPermission(entry.getPermissions());
+    }
+
     private boolean isRoleResource(IResource resource)
     {
         return resource instanceof RoleResource;
@@ -82,18 +88,10 @@ private boolean isPasswordPermission(Set<Permission> permissions)
      */
     private String obfuscateOperation(String operation)
     {
-        // CREATE ROLE / ALTER ROLE query
-        Matcher createAlterRoleMatcher = QUERY_CREATE_ALTER_ROLE_PATTERN.matcher(operation);
-        if (createAlterRoleMatcher.matches())
-        {
-            return obfuscateOperation(createAlterRoleMatcher, operation);
-        }
-
-        // CREATE USER / ALTER USER query
-        Matcher createAlterUserMatcher = QUERY_CREATE_ALTER_USER_PATTERN.matcher(operation);
-        if (createAlterUserMatcher.matches())
+        Matcher passwordMatcher = PASSWORD_PATTERN.matcher(operation);
+        if (passwordMatcher.matches())
         {
-            return obfuscateOperation(createAlterUserMatcher, operation);
+            return obfuscateOperation(passwordMatcher, operation);
         }
 
         return operation;

ecaudit/src/test/java/com/ericsson/bss/cassandra/ecaudit/obfuscator/TestPasswordObfuscator.java

@@ -30,6 +30,7 @@
 import com.ericsson.bss.cassandra.ecaudit.entry.AuditEntry;
 import com.ericsson.bss.cassandra.ecaudit.common.record.AuditOperation;
 import com.ericsson.bss.cassandra.ecaudit.common.record.SimpleAuditOperation;
+import org.apache.cassandra.auth.DataResource;
 import org.apache.cassandra.auth.Permission;
 import org.apache.cassandra.auth.RoleResource;
 
@@ -86,7 +87,8 @@ public void testCreateRolePasswordObfuscation()
         "CREATE ROLE coach WITH PASSWORD ='%s' AND LOGIN = true;",
         "CREATE ROLE coach WITH PASSWORD='%s' AND LOGIN = true;",
         "CREATE ROLE coach WITH PASSWORD= '%s' AND LOGIN = true;",
-        "CREATE ROLE coach WITH PASSWORD  =   '%s' AND LOGIN = true;");
+        "CREATE ROLE coach WITH PASSWORD  =   '%s' AND LOGIN = true;",
+        "CREATE ROLE coach WITH PASSWORD\n = '%s'\n AND LOGIN = true");
 
         validateQueries(createRoleQueries, "coach", Permission.CREATE);
     }
@@ -97,7 +99,8 @@ public void testCreateUserPasswordObfuscation()
         Map<String, String> createUserQueries = createPasswordQueries(
         "CREATE USER akers WITH PASSWORD '%s' SUPERUSER;",
         "CREATE USER akers WITH PASSWORD  '%s'  SUPERUSER;",
-        "CREATE USER akers WITH PASSWORD  '%s'  SUPERUSER;");
+        "CREATE USER akers WITH PASSWORD  '%s'  SUPERUSER;",
+        "CREATE USER akers WITH PASSWORD\n '%s'\n SUPERUSER");
 
         validateQueries(createUserQueries, "akers", Permission.CREATE);
     }
@@ -110,7 +113,8 @@ public void testAlterRolePasswordObfuscation()
         "ALTER ROLE coach WITH PASSWORD = '%s'",
         "ALTER ROLE coach WITH PASSWORD ='%s'",
         "ALTER ROLE coach WITH PASSWORD='%s'",
-        "ALTER ROLE coach WITH PASSWORD=  '%s'");
+        "ALTER ROLE coach WITH PASSWORD=  '%s'",
+        "ALTER ROLE coach WITH PASSWORD\n = '%s'");
 
         validateQueries(alterRoleQueries, "coach", Permission.ALTER);
     }
@@ -122,11 +126,24 @@ public void testAlterUserPasswordObfuscation()
         "ALTER USER moss WITH PASSWORD '%s';",
         "ALTER USER moss WITH PASSWORD  '%s';",
         "ALTER USER moss WITH PASSWORD '%s' ;",
-        "ALTER USER moss WITH PASSWORD  '%s'    ;");
+        "ALTER USER moss WITH PASSWORD  '%s'    ;",
+        "ALTER USER moss WITH PASSWORD\n '%s';");
 
         validateQueries(alterUserQueries, "moss", Permission.ALTER);
     }
 
+    @Test
+    public void testUnparsedStatementsObfuscation()
+    {
+        Map<String, String> alterUserQueries = createPasswordQueries(
+        "ALTER USER coach WITH PASSWORD \n'%s' extra_characters;",
+        "ALTER ROLE coach \nWITH PASSWORD = '%s' extra_characters;",
+        "CREATE USER coach WITH PASSWORD '%s' SUPERUSER extra_characters;",
+        "CREATE ROLE coach WITH PASSWORD\n = '%s'\n AND LOGIN = true extra_characters");
+
+        validateUnknownQueries(alterUserQueries);
+    }
+
     private void validateUnmodifiedQueries(List<String> queries, String username, Permission permission)
     {
         for (String query : queries)
@@ -157,6 +174,22 @@ private void validateQueries(Map<String, String> queries, String username, Permi
         }
     }
 
+    private void validateUnknownQueries(Map<String, String> queries)
+    {
+        for (String query : queries.keySet())
+        {
+            AuditEntry entry = AuditEntry.newBuilder()
+                                         .operation(new SimpleAuditOperation(query))
+                                         .permissions(Permission.ALL)
+                                         .resource(DataResource.root())
+                                         .knownOperation(false)
+                                         .build();
+
+            AuditEntry obfuscated = myObfuscator.obfuscate(entry);
+            assertThat(obfuscated.getOperation().getOperationString()).isEqualTo(queries.get(query));
+        }
+    }
+
     private static Map<String, String> createPasswordQueries(String... variants)
     {
         Map<String, String> passwordQueries = new HashMap<>();

integration-test-standard/src/test/java/com/ericsson/bss/cassandra/ecaudit/integration/standard/ITVerifyAudit.java

@@ -575,7 +575,8 @@ public void testFailedStatementsAreLogged()
                 "SELECT * FROM ecks.invalidtbl",
                 "DELETE FROM invalidks.invalidtbl WHERE partk = 2",
                 "DROP KEYSPACE invalidks",
-                "DROP ROLE invaliduser");
+                "DROP ROLE invaliduser",
+                "CREATE ROLE invaliduser \nWITH PASSWORD = 'secret' _unknown_");
 
         for (String statement : statements)
         {