Skip to content
This repository has been archived by the owner on Nov 22, 2023. It is now read-only.

Commit

Permalink
Add support for reserved prefixes (#1227)
Browse files Browse the repository at this point in the history
This allows operators of a keywhiz service to reserve secret names that begin with a specified prefix for a specified owner. For example, we can enforce that a secret beginning with `sp:` can only be created if the ownerName is `specialOwner`.
  • Loading branch information
chloe-loo authored Jul 31, 2023
1 parent 74060a0 commit 685d197
Show file tree
Hide file tree
Showing 6 changed files with 261 additions and 7 deletions.
20 changes: 20 additions & 0 deletions server/src/main/java/keywhiz/KeywhizConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ public class KeywhizConfig extends Configuration {
@JsonProperty
private Map<String, String> backupExportKeys;

@Nullable
@JsonProperty
private Map<String, String> reservedPrefixes;

@NotNull
@JsonProperty
private KeyStoreConfig contentKeyStore;
Expand Down Expand Up @@ -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<String, String> 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;
}
Expand Down
25 changes: 20 additions & 5 deletions server/src/main/java/keywhiz/service/daos/SecretDAO.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -752,6 +761,7 @@ public static class SecretDAOFactory implements DAOFactory<SecretDAO> {
private final GroupDAO.GroupDAOFactory groupDAOFactory;
private final ContentCryptographer cryptographer;
private final PermissionCheck permissionCheck;
private final KeywhizConfig config;

@Inject public SecretDAOFactory(
DSLContext jooq,
Expand All @@ -760,14 +770,16 @@ public static class SecretDAOFactory implements DAOFactory<SecretDAO> {
SecretSeriesDAOFactory secretSeriesDAOFactory,
GroupDAO.GroupDAOFactory groupDAOFactory,
ContentCryptographer cryptographer,
PermissionCheck permissionCheck) {
PermissionCheck permissionCheck,
KeywhizConfig config) {
this.jooq = jooq;
this.readonlyJooq = readonlyJooq;
this.secretContentDAOFactory = secretContentDAOFactory;
this.secretSeriesDAOFactory = secretSeriesDAOFactory;
this.groupDAOFactory = groupDAOFactory;
this.cryptographer = cryptographer;
this.permissionCheck = permissionCheck;
this.config = config;
}

@Override public SecretDAO readwrite() {
Expand All @@ -777,7 +789,8 @@ public static class SecretDAOFactory implements DAOFactory<SecretDAO> {
secretSeriesDAOFactory,
groupDAOFactory,
cryptographer,
permissionCheck);
permissionCheck,
config);
}

@Override public SecretDAO readonly() {
Expand All @@ -787,7 +800,8 @@ public static class SecretDAOFactory implements DAOFactory<SecretDAO> {
secretSeriesDAOFactory,
groupDAOFactory,
cryptographer,
permissionCheck);
permissionCheck,
config);
}

@Override public SecretDAO using(Configuration configuration) {
Expand All @@ -798,7 +812,8 @@ public static class SecretDAOFactory implements DAOFactory<SecretDAO> {
secretSeriesDAOFactory,
groupDAOFactory,
cryptographer,
permissionCheck);
permissionCheck,
config);
}
}
}
32 changes: 31 additions & 1 deletion server/src/test/java/keywhiz/KeywhizConfigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
24 changes: 24 additions & 0 deletions server/src/test/java/keywhiz/service/daos/SecretDAOTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
162 changes: 162 additions & 0 deletions server/src/test/resources/configs/with-reserved-prefixes.yaml
Original file line number Diff line number Diff line change
@@ -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"
5 changes: 4 additions & 1 deletion server/src/test/resources/keywhiz-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -160,4 +160,7 @@ clientAuthConfig:
type:
useCommonName: true
useSpiffeId: true
createMissingClients: true
createMissingClients: true

reservedPrefixes:
specialOwner: "sp:"

0 comments on commit 685d197

Please sign in to comment.