Add priority to CredentialProvider in GenericWeakCredentialDetector.

PiperOrigin-RevId: 617758218 Change-Id: Ibf7e6a1700e840e6797d254d0c819921755b1d28
google · Mar 21, 2024 · db00d42 · db00d42
1 parent ab5e8d2
commit db00d42
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 10 deletions.
diff --git a/...ns/detectors/credentials/genericweakcredentialdetector/GenericWeakCredentialDetector.java b/...ns/detectors/credentials/genericweakcredentialdetector/GenericWeakCredentialDetector.java
@@ -57,6 +57,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 import javax.inject.Inject;
@@ -156,7 +157,7 @@ private ImmutableList<TestCredential> runTesterAndAddFinding(
 
     // Multiple providers could give the same credentials, so create
     // a set to dedupe them before testing.
-    HashSet<TestCredential> credentials = new HashSet<>();
+    HashSet<TestCredential> credentials = new LinkedHashSet<>();
 
     String serviceName = NetworkServiceUtils.getServiceName(networkService);
 
@@ -170,12 +171,14 @@ private ImmutableList<TestCredential> runTesterAndAddFinding(
               .collect(toImmutableSet());
     }
 
-    for (CredentialProvider provider : effectiveProvider) {
+    // Sort all providers according to their priorities
+    ImmutableList<CredentialProvider> prioritizedCredProviders =
+        ImmutableList.sortedCopyOf(CredentialProvider.comparator(), effectiveProvider);
+    for (CredentialProvider provider : prioritizedCredProviders) {
       provider.generateTestCredentials(networkService).forEachRemaining(credentials::add);
     }
 
-    return new WeakCredentialComposer(
-            ImmutableList.sortedCopyOf(TestCredential.comparator(), credentials), tester)
+    return new WeakCredentialComposer(ImmutableList.copyOf(credentials), tester)
         .run(networkService);
   }
 

diff --git a/...edentials/genericweakcredentialdetector/GenericWeakCredentialDetectorBootstrapModule.java b/...edentials/genericweakcredentialdetector/GenericWeakCredentialDetectorBootstrapModule.java
@@ -38,11 +38,11 @@
 import com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.tester.CredentialTester;
 import com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.testers.grafana.GrafanaCredentialTester;
 import com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.testers.hydra.HydraCredentialTester;
-import com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.testers.rabbitmq.RabbitMQCredentialTester;
 import com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.testers.jenkins.JenkinsCredentialTester;
 import com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.testers.mysql.MysqlCredentialTester;
 import com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.testers.ncrack.NcrackCredentialTester;
 import com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.testers.postgres.PostgresCredentialTester;
+import com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.testers.rabbitmq.RabbitMQCredentialTester;
 import com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.testers.wordpress.WordpressCredentialTester;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -74,8 +74,8 @@ protected void configurePlugin() {
 
     Multibinder<CredentialProvider> credentialProviderBinder =
         Multibinder.newSetBinder(binder(), CredentialProvider.class);
-    credentialProviderBinder.addBinding().to(Top100Passwords.class);
     credentialProviderBinder.addBinding().to(DefaultCredentials.class);
+    credentialProviderBinder.addBinding().to(Top100Passwords.class);
 
     registerPlugin(GenericWeakCredentialDetector.class);
   }

diff --git a/...gins/detectors/credentials/genericweakcredentialdetector/provider/CredentialProvider.java b/...gins/detectors/credentials/genericweakcredentialdetector/provider/CredentialProvider.java
@@ -15,8 +15,11 @@
  */
 package com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.provider;
 
+import static java.util.Comparator.comparing;
+
 import com.google.tsunami.plugins.detectors.credentials.genericweakcredentialdetector.proto.CredentialType;
 import com.google.tsunami.proto.NetworkService;
+import java.util.Comparator;
 import java.util.Iterator;
 
 /**
@@ -35,4 +38,15 @@ public abstract class CredentialProvider {
   public abstract String description();
 
   public abstract Iterator<TestCredential> generateTestCredentials(NetworkService networkService);
+
+  // Credential pairs from the high priority CredentialProviders are tested first,
+  // a CredentialProvider with priority 1 is tested before the one at priority 2.
+  public abstract int priority();
+
+  public static Comparator<CredentialProvider> comparator() {
+    return COMPARATOR;
+  }
+
+  private static final Comparator<CredentialProvider> COMPARATOR =
+      comparing(CredentialProvider::priority);
 }
diff --git a/...gins/detectors/credentials/genericweakcredentialdetector/provider/DefaultCredentials.java b/...gins/detectors/credentials/genericweakcredentialdetector/provider/DefaultCredentials.java
@@ -80,4 +80,9 @@ public Iterator<TestCredential> generateTestCredentials(NetworkService networkSe
                     usernamePassworkPair.get(0), Optional.of(usernamePassworkPair.get(1))))
         .iterator();
   }
+
+  @Override
+  public int priority() {
+    return 1;
+  }
 }
diff --git a/...plugins/detectors/credentials/genericweakcredentialdetector/provider/Top100Passwords.java b/...plugins/detectors/credentials/genericweakcredentialdetector/provider/Top100Passwords.java
@@ -200,4 +200,10 @@ public String description() {
   public Iterator<TestCredential> generateTestCredentials(NetworkService unused) {
     return credentials.iterator();
   }
+
+  @Override
+  // Top 100 passwords are tested after default credentials.
+  public int priority() {
+    return 2;
+  }
 }
diff --git a/...etectors/credentials/genericweakcredentialdetector/GenericWeakCredentialDetectorTest.java b/...etectors/credentials/genericweakcredentialdetector/GenericWeakCredentialDetectorTest.java
@@ -75,6 +75,8 @@
 public final class GenericWeakCredentialDetectorTest {
   @Rule public final MockitoRule mockito = MockitoJUnit.rule();
 
+  private static final TestCredential HIGH_PRIORITY_CRED =
+      TestCredential.create("admin", Optional.of("admin"));
   private static final Instant FAKE_NOW = Instant.parse("2020-01-01T00:00:00.00Z");
   private static final ImmutableList<TestCredential> TEST_CREDENTIALS1 =
       ImmutableList.of(
@@ -86,8 +88,11 @@ public final class GenericWeakCredentialDetectorTest {
           TestCredential.create("username2", Optional.of("password2")),
           TestCredential.create("username3", Optional.of("password3")));
 
+  private static final ImmutableList<TestCredential> TEST_CREDENTIALS_HIGH_PRIORITY =
+      ImmutableList.of(HIGH_PRIORITY_CRED);
   @Mock private CredentialProvider provider1;
   @Mock private CredentialProvider provider2;
+  @Mock private CredentialProvider provider3;
   @Mock private CredentialTester tester1;
   @Mock private CredentialTester tester2;
   @Mock private CredentialTester tester3;
@@ -111,6 +116,11 @@ public void setupPlugin() throws IOException {
         .thenAnswer(invocation -> TEST_CREDENTIALS1.iterator());
     when(provider2.generateTestCredentials(any()))
         .thenAnswer(invocation -> TEST_CREDENTIALS2.iterator());
+    when(provider3.generateTestCredentials(any()))
+        .thenAnswer(invocation -> TEST_CREDENTIALS_HIGH_PRIORITY.iterator());
+    when(provider1.priority()).thenReturn(2);
+    when(provider2.priority()).thenReturn(2);
+    when(provider3.priority()).thenReturn(1);
     when(tester1.canAccept(any())).thenReturn(true);
     when(tester2.canAccept(any())).thenReturn(true);
     when(tester3.canAccept(any())).thenReturn(true);
@@ -163,7 +173,7 @@ public MockResponse dispatch(RecordedRequest request) throws InterruptedExceptio
 
     plugin =
         new GenericWeakCredentialDetector(
-            ImmutableSet.of(provider1, provider2),
+            ImmutableSet.of(provider1, provider2, provider3),
             ImmutableSet.of(tester1, tester2, tester3),
             fakeUtcClock,
             httpClient);
@@ -250,11 +260,14 @@ public void detect_onlyTestsUniqueCredentials() {
     runDetectOnMockWebServer();
 
     verify(tester1).testValidCredentials(any(), listCaptor1.capture());
-    assertThat(listCaptor1.getValue()).hasSize(3);
+    assertThat(listCaptor1.getValue()).hasSize(4);
+    assertThat(listCaptor1.getValue().get(0)).isEqualTo(HIGH_PRIORITY_CRED);
     verify(tester2).testValidCredentials(any(), listCaptor2.capture());
-    assertThat(listCaptor2.getValue()).hasSize(3);
+    assertThat(listCaptor2.getValue()).hasSize(4);
+    assertThat(listCaptor2.getValue().get(0)).isEqualTo(HIGH_PRIORITY_CRED);
     verify(tester3).testValidCredentials(any(), listCaptor3.capture());
-    assertThat(listCaptor3.getValue()).hasSize(3);
+    assertThat(listCaptor3.getValue()).hasSize(4);
+    assertThat(listCaptor3.getValue().get(0)).isEqualTo(HIGH_PRIORITY_CRED);
   }
 
   @Test