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

Add support for reserved prefixes #1227

Merged
merged 4 commits into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:"
Loading