Add tests

square · Jul 31, 2023 · 1e94a68 · 1e94a68
1 parent 694daf0
commit 1e94a68
Show file tree

Hide file tree

Showing 9 changed files with 271 additions and 22 deletions.
diff --git a/cli/src/main/java/keywhiz/cli/Utilities.java b/cli/src/main/java/keywhiz/cli/Utilities.java
@@ -17,7 +17,7 @@
 
 public class Utilities {
 
-  public static final String VALID_NAME_PATTERN = "^[a-zA-Z_0-9\\-.:]+$";
+  public static final String VALID_NAME_PATTERN = "^[a-zA-Z_0-9\\-.:/]+$";
 
   public static boolean validName(String name) {
     // "." is allowed at any position but the first.

diff --git a/cli/src/test/java/keywhiz/cli/commands/AddActionTest.java b/cli/src/test/java/keywhiz/cli/commands/AddActionTest.java
@@ -66,6 +66,10 @@ public class AddActionTest {
   Secret secret =
       new Secret(15, "newSecret", null, null, () -> "c2VjcmV0MQ==", "checksum", NOW, null, NOW, null,
           null, null, ImmutableMap.of(), 0, 1L, NOW, null);
+
+  Secret secretWithSpecialName =
+      new Secret(16, "sp:ns/owner/secret-name", null, null, () -> "c2VjcmV0MQ==", "checksum", NOW, null, NOW, null,
+          null, null, ImmutableMap.of(), 0, 1L, NOW, null);
   SanitizedSecret sanitizedSecret = SanitizedSecret.fromSecret(secret);
   SecretDetailResponse secretDetailResponse = SecretDetailResponse.fromSecret(secret, null, null);
 
@@ -285,4 +289,33 @@ public void addValidatesSecretName() throws Exception {
 
     addAction.run();
   }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void addValidatesSecretNameNoDollarSign() throws Exception {
+    addActionConfig.addType = Arrays.asList("secret");
+    addActionConfig.name = "Invalid$Name";
+
+    addAction.run();
+  }
+
+  @Test
+  public void addValidatesSecretNameSpecialName() throws Exception {
+    addActionConfig.addType = Arrays.asList("secret");
+    addActionConfig.name = secretWithSpecialName.getDisplayName();
+    addActionConfig.expiry = "2006-01-02T15:04:05Z";
+
+    byte[] content = base64Decoder.decode(secretWithSpecialName.getSecret());
+    addAction.stream = new ByteArrayInputStream(content);
+    when(keywhizClient.getSanitizedSecretByName(secretWithSpecialName.getName()))
+        .thenThrow(new NotFoundException()); // Call checks for existence.
+
+    when(
+        keywhizClient.createSecret(secretWithSpecialName.getName(), null, "", content,
+            secretWithSpecialName.getMetadata(), 1136214245))
+        .thenReturn(secretDetailResponse);
+
+    addAction.run();
+    verify(keywhizClient, times(1)).createSecret(secretWithSpecialName.getName(), null, "", content,
+        secretWithSpecialName.getMetadata(), 1136214245);
+  }
 }
diff --git a/client/src/main/java/keywhiz/client/KeywhizClient.java b/client/src/main/java/keywhiz/client/KeywhizClient.java
@@ -210,7 +210,11 @@ public SecretDetailResponse createSecret(
             .metadata(metadata)
             .expiry(expiry)
             .build();
-    String response = httpPost(baseUrl.resolve("/admin/secrets"), request);
+    HttpUrl url = baseUrl.newBuilder()
+        .addPathSegment("admin")
+        .addPathSegment("secrets")
+        .build();
+    String response = httpPost(url, request);
     return mapper.readValue(response, SecretDetailResponse.class);
   }
 

diff --git a/server/src/main/java/keywhiz/KeywhizConfig.java b/server/src/main/java/keywhiz/KeywhizConfig.java
@@ -76,7 +76,7 @@ public class KeywhizConfig extends Configuration {
 
   @Nullable
   @JsonProperty
-  private Map<String, String> ownerReservedPrefixes;
+  private Map<String, String> reservedPrefixes;
 
   @NotNull
   @JsonProperty
@@ -222,23 +222,20 @@ public String getDerivationProviderClass() {
     return backupExportKeys.get(name);
   }
 
-  @Nullable public String getOwnerRestriction(String secretName) {
-    if (ownerReservedPrefixes == null) {
-      return null;
+  public boolean canCreateSecretWithName(String secretName, String ownerName) {
+    if (reservedPrefixes == null) {
+      return true;
     }
-    String expectedOwner = null;
-    for (Map.Entry<String, String> entry : ownerReservedPrefixes.entrySet()) {
+    for (Map.Entry<String, String> entry : reservedPrefixes.entrySet()) {
       String owner = entry.getKey();
       String prefix = entry.getValue();
       if (secretName.startsWith(prefix)) {
-        if (expectedOwner != null) {
-          throw new RuntimeException("Multiple reserved owners for prefix");
-        } else {
-          expectedOwner = owner;
+        if (ownerName == null || !ownerName.equals(owner)) {
+          return false;
         }
       }
     }
-    return expectedOwner;
+    return true;
   }
 
   public RowHmacCheck getRowHmacCheck() {

diff --git a/server/src/main/java/keywhiz/service/daos/SecretDAO.java b/server/src/main/java/keywhiz/service/daos/SecretDAO.java
@@ -128,13 +128,9 @@ public long createSecret(
             + "names must be %d characters or less", name, SECRET_NAME_MAX_LENGTH));
       }
 
-      String ownerRestriction = config.getOwnerRestriction(name);
-      if (ownerRestriction != null) {
-        if (!ownerName.equals(ownerRestriction)) {
-          throw new BadRequestException(format("secret cannot be created with name `%s` for owner "
-                  + "`%s` - this name format is reserved by owner `%s`", name, ownerName,
-              ownerRestriction));
-        }
+      if (!config.canCreateSecretWithName(name, ownerName)) {
+        throw new BadRequestException(format("secret cannot be created with name `%s` for owner "
+            + "`%s` - this name format is reserved", name, ownerName));
       }
 
       long now = OffsetDateTime.now().toEpochSecond();

diff --git a/server/src/test/java/keywhiz/KeywhizConfigTest.java b/server/src/test/java/keywhiz/KeywhizConfigTest.java
@@ -66,12 +66,42 @@ public void parseNewSecretOwnershipStrategyNone() {
   }
 
   @Test
-  public void parsNewSecretOwnershipStrategyInfer() {
+  public void parseNewSecretOwnershipStrategyInfer() {
     KeywhizConfig config = loadConfig("new-secret-ownership-strategy-infer.yaml");
     assertThat(config.getNewSecretOwnershipStrategy()).isEqualTo(
         KeywhizConfig.NewSecretOwnershipStrategy.INFER_FROM_CLIENT);
   }
 
+  @Test
+  public void parseReservedPrefixes() {
+    KeywhizConfig config = loadConfig("with-reserved-prefixes.yaml");
+    assertThat(config.canCreateSecretWithName("any-secret-name", "any-owner-name")).isTrue();
+
+    assertThat(config.canCreateSecretWithName("reserved-prefix", "any-owner-name")).isTrue();
+    assertThat(config.canCreateSecretWithName("reserved-prefix", "reservedOwner")).isTrue();
+
+    assertThat(config.canCreateSecretWithName("reserved-prefix:", "any-owner-name")).isFalse();
+    assertThat(config.canCreateSecretWithName("reserved-prefix:", "reservedOwner")).isTrue();
+
+    assertThat(config.canCreateSecretWithName("reserved-prefix:secretName", "any-owner-name")).isFalse();
+    assertThat(config.canCreateSecretWithName("reserved-prefix:secretName", "reservedOwner")).isTrue();
+
+    assertThat(config.canCreateSecretWithName("extra-prefix-reserved-prefix:secretName", "any-owner-name")).isTrue();
+    assertThat(config.canCreateSecretWithName("extra-prefix-reserved-prefix:secretName", "reservedOwner")).isTrue();
+
+    assertThat(config.canCreateSecretWithName("reserved-prefix:extra:secretName", "reservedOwner")).isFalse();
+    assertThat(config.canCreateSecretWithName("reserved-prefix:extra:secretName", "anotherOwner")).isFalse();
+
+    assertThat(config.canCreateSecretWithName("ab", "reservedOwner")).isTrue();
+    assertThat(config.canCreateSecretWithName("ab", "noColonInPrefix")).isTrue();
+
+    assertThat(config.canCreateSecretWithName("abc", "reservedOwner")).isFalse();
+    assertThat(config.canCreateSecretWithName("abc", "noColonInPrefix")).isTrue();
+
+    assertThat(config.canCreateSecretWithName("abcdef", "reservedOwner")).isFalse();
+    assertThat(config.canCreateSecretWithName("abcdef", "noColonInPrefix")).isTrue();
+  }
+
   private static KeywhizConfig loadConfig(String resource) {
     Validator validator = Validation.buildDefaultValidatorFactory().getValidator();
     ObjectMapper objectMapper = createObjectMapper();

diff --git a/server/src/test/java/keywhiz/service/daos/SecretDAOTest.java b/server/src/test/java/keywhiz/service/daos/SecretDAOTest.java
@@ -279,6 +279,30 @@ public void createOrUpdateExistingSecretUpdatesOwner() {
     assertThat(secretDAO.getSecrets(null, null, null,null, null)).containsOnly(secret1, secret2b, newSecret);
   }
 
+  @Test public void createSecretWithReservedPrefix() {
+    groupDAO.createGroup("specialOwner", "creator", "description", NO_METADATA);
+    String name = "sp:namespace/owner/secretName";
+    String content = "c2VjcmV0MQ==";
+    String hmac = cryptographer.computeHmac(content.getBytes(UTF_8), "hmackey");
+    String encryptedContent = cryptographer.encryptionKeyDerivedFrom(name).encrypt(content);
+    long newId = secretDAO.createSecret(name, "specialOwner", encryptedContent, hmac, "creator",
+        ImmutableMap.of(), 0, "", null, ImmutableMap.of());
+    SecretSeriesAndContent newSecret = secretDAO.getSecretById(newId).get();
+    assertThat(newSecret).isNotNull();
+  }
+
+  @Test(expected = BadRequestException.class)
+  public void createSecretFailsIfPrefixReservedByDifferentOwner() {
+    groupDAO.createGroup("specialOwner", "creator", "description", NO_METADATA);
+    groupDAO.createGroup("regularOwner", "creator", "description", NO_METADATA);
+    String name = "sp:namespace/owner/secretName";
+    String content = "c2VjcmV0MQ==";
+    String hmac = cryptographer.computeHmac(content.getBytes(UTF_8), "hmackey");
+    String encryptedContent = cryptographer.encryptionKeyDerivedFrom(name).encrypt(content);
+    secretDAO.createSecret(name, "regularOwner", encryptedContent, hmac, "creator",
+        ImmutableMap.of(), 0, "", null, ImmutableMap.of());
+  }
+
   @Test(expected = DataAccessException.class)
   public void createSecretFailsIfSecretExists() {
     String name = "newSecret";

diff --git a/server/src/test/resources/configs/with-reserved-prefixes.yaml b/server/src/test/resources/configs/with-reserved-prefixes.yaml
@@ -0,0 +1,162 @@
+server:
+  applicationConnectors:
+    - type: resources-https
+      port: 4445
+      keyStorePath: dev_and_test_keystore.p12
+      keyStorePassword: ponies
+      keyStoreType: PKCS12
+      trustStorePath: dev_and_test_truststore.p12
+      trustStorePassword: ponies
+      trustStoreType: PKCS12
+      wantClientAuth: true
+      validateCerts: true
+      validatePeers: true
+      crlPath: dev_and_test.crl
+      supportedProtocols: [TLSv1.2]
+      supportedCipherSuites:
+        - TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
+        - TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
+        - TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
+        - TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
+        - TLS_RSA_WITH_AES_128_CBC_SHA256
+        - TLS_RSA_WITH_AES_128_GCM_SHA256
+        - TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA256
+        - TLS_ECDH_RSA_WITH_AES_128_CBC_SHA256
+        - TLS_DHE_RSA_WITH_AES_128_CBC_SHA256
+        - TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
+        - TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
+        - TLS_RSA_WITH_AES_128_CBC_SHA
+        - TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA
+        - TLS_ECDH_RSA_WITH_AES_128_CBC_SHA
+        - TLS_DHE_RSA_WITH_AES_128_CBC_SHA
+  adminConnectors:
+    - type: http
+      bindHost: localhost
+      port: 8081
+
+logging:
+  appenders:
+    - type: console
+
+environment: testing
+
+database:
+  driverClass: com.mysql.cj.jdbc.Driver
+  url: jdbc:mysql://address=(host=localhost)(%custom_mysql_port%)(useUnicode=true)(characterEncoding=utf8)/keywhizdb_test
+  user: root
+  properties:
+    charSet: UTF-8
+  initialSize: 2
+  minSize: 2
+  maxSize: 2
+  # There is explicitly no password. Do not uncomment.
+  # password:
+
+readonlyDatabase:
+  driverClass: com.mysql.cj.jdbc.Driver
+  url: jdbc:mysql://address=(host=localhost)(%custom_mysql_port%)(useUnicode=true)(characterEncoding=utf8)/keywhizdb_test
+  user: root
+  properties:
+    charSet: UTF-8
+  readOnlyByDefault: true
+  initialSize: 1
+  minSize: 1
+  maxSize: 1
+  # There is explicitly no password. Do not uncomment.
+  # password:
+
+migrationsDir:
+  db/mysql/migration
+
+statusCacheExpiry: PT3S
+
+backupExportKeys:
+  test: |
+    -----BEGIN PGP PUBLIC KEY BLOCK-----
+
+    xsBNBFltUFoBCAC4aUBq1b6YYK65spHuVx+6FiQ9TiFMoiC4SpiyKH0oKsaa6uRz
+    EKzpBp0GoCIBhavBpmnzpNzdhuBrkAzK4543bxXEGGmjsbSV69ysgLBhTyrngOuS
+    diPVgaXIf47FpA/YoIlbyG1uQZFZ6bzJQL8gr8dbO5plFCaIUAFQhx88gNBmGgRk
+    rW5iU6nzlNzVRlkCAnK18YNv0h08nNRtXKvmLAnM6RSaVWsqDeisA/717dp1o4Hz
+    CofZGPdUkEoZkx2UekH9E7kzH90D2QmR+PdWtOz+5gtOMXgrpsJoh3fhwXVPo8dz
+    MT/5iLbReoM8TZVOLPsLyVJdd/oeV/5e6HvzABEBAAHNHlRlc3QgRXhwb3J0IDx0
+    ZXN0QGV4YW1wbGUuY29tPsLAbQQTAQoAFwUCWW1QWgIbLwMLCQcDFQoIAh4BAheA
+    AAoJEEssmAxO8mrteUkH/jnisJDq64UKImwVJa7FF6Wjb82QbY1Sv+sXbw6o0Eog
+    tH5YBlow+TnaG6Vd1mk4qfWjOAViG8EbXhaEQSuRMZaX/PGh/2DnZqkKHIWkzVmm
+    R5FdfKBO1KukyTe1HLdC7lJLsuXFw9SwZ/CeP/zQpXJzI5j36Nzj82pKLZA58MSU
+    3suJ4dTtUwvHA81yy/tgAIFHWayyJ4lV08QzR4c7Ey58rPqrtxzSZ5jgqTK3bmje
+    kxWnZ00vPLR3dyDSiYTnMS5f0LEb1nAevXaSyVT9c/0rYOmN41j5rCR6V6SsKZUq
+    uPJi4ROhQcRApKcC/wxRQitxqAa/RnkwkO0lMHGxdEPOwE0EWW1QWgEIAMRL6Ke+
+    sdmWLBnt2/4O7WRQ45aeJjgO/X6kelHEulGlk4bqn+bumgZLnt/NLXJSh5ZSa1XM
+    mkZVml5zmLLAq1N56583WMlEofXadkBuxUkaP3NWsdbT/JCCzHDhgDK6pePK03ac
+    KTwahGMia8SZ41VqYMa9aAqPP3ywQKIBxyCvewoUM4YJTGLLY8LMxcZnyM1si9Ip
+    3MnyvzXFMmZx9EzQiXePzc7zzdWp7HX63UkT2t567CDGe7NqHIef4GL/ENpSO2XT
+    PsJlzNZTlVxM6sggIsKDSrOCU5LzpM21Ql12L4hDE6gv0qxquI54cVZyJ8L9HfTf
+    pnZoSfVUYLqcQ2MAEQEAAcLBhAQYAQoADwUCWW1QWgUJDwmcAAIbLgEpCRBLLJgM
+    TvJq7cBdIAQZAQoABgUCWW1QWgAKCRCdJ0JDr9ocExyVCACOo/EZAyTEV1IM1gx3
+    LuXp6Nmhk9OIJAg4EKVrUa4l5J5mFLeTLiIvHrChuuZUT3HvFa4aHesCiUY+kag7
+    Z8SdLot4zRSB2JZQ0dVr/9N82+TEHkVAApq2UXd97O/EbYO3jmGAdfdHNjbmyABa
+    eEsx9YcIAKeIDrHv9idbM1gkvmQ9iwGznHzmQf/ubHE3f0OFczjYV/8lQmVePEbT
+    pXSEuJBvRbpVdSWnIjz1XqAxpkIG/4joBbVkoKRSCgX0Mxwrz+BXZM5LuEzeC9H7
+    z3L+hfFOYjPZ8BYl8f/Eb5SSsLDoZdKZTN7Gg2YMRTB6xS9+r3Nnu/hFubV0MkGy
+    7J9oymsIAJu/dfQlj5ZoyTn4h545J1eMDn0bVlyvGRxfgac2IsGe2ssPoXr0kaE7
+    I2tLWZzDixnVs7a/7ES63GX4ONoul5T0pwVTtnebYaum7wbSx0vUEvNftzR9MnrJ
+    inl8ZKLlWdpg1R5iGCGsKIojo4uEqbM/DBXX5/F+xrX9MCbMmfPLHjKA0f/gB2IW
+    HpIWh0ibQnZy1y7ApaJnofIUZo5WIgB+wjVgkkE3zvxWNnoK+oNLQEOKyqYobZvo
+    7KUyclo9qWVy3EEZTrhO8/XCjVKL3/Tl7Wy3kf+6058tCeQbH6DgpzsaP+XwJ8tR
+    waxiKBbHE6eyC11mrir2QI4RXT2P7ETOwE0EWW1QWgEIAN0vGNm0aNRD1OrB0DUp
+    5ltGRgTzuTA05Rd3RXZ4OnZsxLQjARSzFREQsIgehHgBS6anHMzitL3eLRjZM+8x
+    1vCRNT3vhNB2ljf4LdvcsQmAzP9Upe5BzVT+ywrIE6JjKCcg3RVWNJYBYRSq6VwS
+    MRN8GfrUvUGSCNr97t5W0mt1vQH7Y2BcaER/YYMQ86gqbB3j5M8WQseSHKdYbTh8
+    59QiPQuNR6GGrAvp6HaZWRPH7XZt4IiDYiBet66fHIm6YLjO0Ds+Yad4aYZwLitE
+    gHHokJUjLm3fqL05FJDSiE3KaBT07tXANH2flqIe8wbGTUCmj7DuwfNuGiXzTnO4
+    xZ8AEQEAAcLBhAQYAQoADwUCWW1QWgUJDwmcAAIbLgEpCRBLLJgMTvJq7cBdIAQZ
+    AQoABgUCWW1QWgAKCRCv6D9VTBZojbe/B/4tp6Fd69/j5wksKfSAxAbIhRNBNj3e
+    Zyhis1CZc0Xgw4ACqJbq/e+nqEbumV11WJOT5DrqTOZkw6u7ey8FvU7tngCMK1eP
+    0vvdiFDR14mhoWo2kdOkYWe5WpDnGfQXRjjfRMg/Pf/dp0sUfAvmutSS1yHsGpE8
+    SDLuIRKO9GLTJs4UCopEk53iMHmTebOqC6wokQjSVedDkVhaX5QTvhHIYwU4dTik
+    OxjIhqn/k1m3rexJIT2SPQeL2eD0fhQxrxND835vIE7NCEvVpni4Mi5sFq0288Rb
+    +UaUERS9Lssp2NuJSoaLASB0WH+AMhGvneSAQKr5LLnE0jjyv0vZ6Q3FlMkH/Awl
+    HoP7mUOr3uw/nJOq47l7V/paM1dQgmD2+PvTyxzB5PnclHraJBS91bdUGIUxqula
+    C9ugQY+/SxCc5EwaleyOZNC2onXqLV4kPYWCYFAS5atMMTQA96TkkXVP4gaAO1hF
+    NvrndG70YWkoB8ArCKfNEx/CR4msZBBvUKntjKQNiOXI5Y4wzJIZzoBCQ2XDnIid
+    M+L/BZ5d0PhHRL3WCdSKd/l3/SPRrkaSKp5Ii3G7AH4CVH51rTWi4MFOBYy/LopA
+    pjVkvUV7XB8oxsBAkjW4DRPFs8EpxvjRNCWFtGXqRYQ5LqGMgi2vnwG8py3uRvsd
+    EuOSrLrAPaUbk42SHyg=
+    =bG4m
+    -----END PGP PUBLIC KEY BLOCK-----
+
+userAuth:
+  type: bcrypt
+
+# Base64 of "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+cookieKey: QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUE=
+
+sessionCookie:
+  name: session
+  path: /admin
+
+contentKeyStore:
+  path: derivation.jceks
+  type: JCEKS
+  password: CHANGE
+  alias: basekey
+
+rowHmacCheck: enforced
+
+flywaySchemaTable: schema_version
+
+clientAuthConfig:
+  xfcc:
+    - port: 4446
+      allowedClientNames: []
+      allowedSpiffeIds: []
+  type:
+    useCommonName: true
+    useSpiffeId: true
+  createMissingClients: true
+
+reservedPrefixes:
+  reservedOwner: "reserved-prefix:"
+  anotherOwner: "reserved-prefix:extra:"
+  noColonInPrefix: "abc"
diff --git a/server/src/test/resources/keywhiz-test.yaml b/server/src/test/resources/keywhiz-test.yaml
@@ -160,4 +160,7 @@ clientAuthConfig:
   type:
     useCommonName: true
     useSpiffeId: true
-  createMissingClients: true
+  createMissingClients: true
+
+reservedPrefixes:
+  specialOwner: "sp:"