diff --git a/server/src/main/java/keywhiz/KeywhizConfig.java b/server/src/main/java/keywhiz/KeywhizConfig.java index c450c9d92..111361765 100644 --- a/server/src/main/java/keywhiz/KeywhizConfig.java +++ b/server/src/main/java/keywhiz/KeywhizConfig.java @@ -74,6 +74,10 @@ public class KeywhizConfig extends Configuration { @JsonProperty private Map backupExportKeys; + @Nullable + @JsonProperty + private Map reservedPrefixes; + @NotNull @JsonProperty private KeyStoreConfig contentKeyStore; @@ -218,6 +222,22 @@ public String getDerivationProviderClass() { return backupExportKeys.get(name); } + public boolean canCreateSecretWithName(String secretName, String ownerName) { + if (reservedPrefixes == null) { + return true; + } + for (Map.Entry entry : reservedPrefixes.entrySet()) { + String owner = entry.getKey(); + String prefix = entry.getValue(); + if (secretName.startsWith(prefix)) { + if (ownerName == null || !ownerName.equals(owner)) { + return false; + } + } + } + return true; + } + public RowHmacCheck getRowHmacCheck() { return rowHmacCheck; } diff --git a/server/src/main/java/keywhiz/service/daos/SecretDAO.java b/server/src/main/java/keywhiz/service/daos/SecretDAO.java index b2f6b451e..88bf3a63c 100644 --- a/server/src/main/java/keywhiz/service/daos/SecretDAO.java +++ b/server/src/main/java/keywhiz/service/daos/SecretDAO.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; +import keywhiz.KeywhizConfig; import keywhiz.api.automation.v2.PartialUpdateSecretRequestV2; import keywhiz.api.model.Group; import keywhiz.api.model.SanitizedSecret; @@ -73,6 +74,7 @@ public class SecretDAO { private final GroupDAO.GroupDAOFactory groupDAOFactory; private final ContentCryptographer cryptographer; private final PermissionCheck permissionCheck; + private final KeywhizConfig config; // this is the maximum length of a secret name, so that it will still fit in the 255 char limit // of the database field if it is deleted and auto-renamed @@ -88,13 +90,15 @@ public SecretDAO( SecretSeriesDAOFactory secretSeriesDAOFactory, GroupDAO.GroupDAOFactory groupDAOFactory, ContentCryptographer cryptographer, - PermissionCheck permissionCheck) { + PermissionCheck permissionCheck, + KeywhizConfig config) { this.dslContext = dslContext; this.secretContentDAOFactory = secretContentDAOFactory; this.secretSeriesDAOFactory = secretSeriesDAOFactory; this.groupDAOFactory = groupDAOFactory; this.cryptographer = cryptographer; this.permissionCheck = permissionCheck; + this.config = config; } @VisibleForTesting @@ -124,6 +128,11 @@ public long createSecret( + "names must be %d characters or less", name, SECRET_NAME_MAX_LENGTH)); } + 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(); SecretContentDAO secretContentDAO = secretContentDAOFactory.using(configuration); @@ -752,6 +761,7 @@ public static class SecretDAOFactory implements DAOFactory { private final GroupDAO.GroupDAOFactory groupDAOFactory; private final ContentCryptographer cryptographer; private final PermissionCheck permissionCheck; + private final KeywhizConfig config; @Inject public SecretDAOFactory( DSLContext jooq, @@ -760,7 +770,8 @@ public static class SecretDAOFactory implements DAOFactory { SecretSeriesDAOFactory secretSeriesDAOFactory, GroupDAO.GroupDAOFactory groupDAOFactory, ContentCryptographer cryptographer, - PermissionCheck permissionCheck) { + PermissionCheck permissionCheck, + KeywhizConfig config) { this.jooq = jooq; this.readonlyJooq = readonlyJooq; this.secretContentDAOFactory = secretContentDAOFactory; @@ -768,6 +779,7 @@ public static class SecretDAOFactory implements DAOFactory { this.groupDAOFactory = groupDAOFactory; this.cryptographer = cryptographer; this.permissionCheck = permissionCheck; + this.config = config; } @Override public SecretDAO readwrite() { @@ -777,7 +789,8 @@ public static class SecretDAOFactory implements DAOFactory { secretSeriesDAOFactory, groupDAOFactory, cryptographer, - permissionCheck); + permissionCheck, + config); } @Override public SecretDAO readonly() { @@ -787,7 +800,8 @@ public static class SecretDAOFactory implements DAOFactory { secretSeriesDAOFactory, groupDAOFactory, cryptographer, - permissionCheck); + permissionCheck, + config); } @Override public SecretDAO using(Configuration configuration) { @@ -798,7 +812,8 @@ public static class SecretDAOFactory implements DAOFactory { secretSeriesDAOFactory, groupDAOFactory, cryptographer, - permissionCheck); + permissionCheck, + config); } } } diff --git a/server/src/test/java/keywhiz/KeywhizConfigTest.java b/server/src/test/java/keywhiz/KeywhizConfigTest.java index e06f387f5..7d8a357a6 100644 --- 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 handleReservedPrefixes() { + 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 index d2a159800..37da810d6 100644 --- 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:key_name"; + 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:key_name"; + 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 new file mode 100644 index 000000000..726c72f30 --- /dev/null +++ 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" \ No newline at end of file diff --git a/server/src/test/resources/keywhiz-test.yaml b/server/src/test/resources/keywhiz-test.yaml index 743e6bd12..a551ab546 100644 --- 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 \ No newline at end of file + createMissingClients: true + +reservedPrefixes: + specialOwner: "sp:" \ No newline at end of file