Skip to content

Commit 4b4dbf3

Browse files
adrikaguptaadrikagupta
committed
add kms key rotation for secure keys
1 parent 60e58d4 commit 4b4dbf3

4 files changed

Lines changed: 212 additions & 4 deletions

File tree

‎cdap-securestore-ext-cloudkms/pom.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@
7777
<groupId>junit</groupId>
7878
<artifactId>junit</artifactId>
7979
</dependency>
80+
<dependency>
81+
<groupId>org.mockito</groupId>
82+
<artifactId>mockito-core</artifactId>
83+
</dependency>
8084
</dependencies>
8185

8286
<profiles>

‎cdap-securestore-ext-cloudkms/src/main/java/io/cdap/cdap/securestore/gcp/cloudkms/CloudKMSClient.java‎

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@
4040
import java.net.HttpURLConnection;
4141
import java.net.URL;
4242
import java.nio.charset.StandardCharsets;
43+
import java.time.Instant;
4344
import java.util.HashSet;
45+
import java.util.List;
4446
import java.util.Map;
4547
import java.util.Set;
4648
import org.slf4j.Logger;
@@ -49,7 +51,7 @@
4951
/**
5052
* Wrapper on {@link CloudKMS} client.
5153
*/
52-
class CloudKMSClient implements Closeable {
54+
public class CloudKMSClient implements Closeable {
5355

5456
private static final Logger LOG = LoggerFactory.getLogger(CloudKMSClient.class);
5557
// When created in the global location, Cloud KMS resources are available from zones spread around the world.
@@ -59,6 +61,7 @@ class CloudKMSClient implements Closeable {
5961
private static final String CLOUD_KMS = "cloudkms";
6062
private static final String PROJECT_ID = "project.id";
6163
private static final String SERVICE_ACCOUNT_FILE = "service.account.file";
64+
private static final String ENABLE_KEY_ROTATION = "key.rotation.enabled";
6265
private static final String METADATA_SERVER_API = "metadata.server.api";
6366
private static final String DEFAULT_METADATA_SERVER_API =
6467
"http://metadata.google.internal/computeMetadata"
@@ -73,12 +76,14 @@ class CloudKMSClient implements Closeable {
7376
// In-memory cache to hold created crypto keys, this is to avoid checking if a given crypto key exists.
7477
private final Set<String> knownCryptoKeys;
7578

79+
private final Boolean enableKeyRotation;
80+
7681
/**
7782
* Constructs Cloud KMS client.
7883
*
7984
* @throws IOException if cloud kms client can not be created
8085
*/
81-
CloudKMSClient(Map<String, String> properties) throws IOException {
86+
protected CloudKMSClient(Map<String, String> properties) throws IOException {
8287
String metadataServerApi = properties.getOrDefault(METADATA_SERVER_API,
8388
DEFAULT_METADATA_SERVER_API);
8489
this.projectId = properties.containsKey(PROJECT_ID) ? properties.get(PROJECT_ID) :
@@ -87,6 +92,9 @@ class CloudKMSClient implements Closeable {
8792
this.cloudKMS = createCloudKMS(serviceAccountFile);
8893
this.keyringId = properties.getOrDefault(KEYRING_ID, DEFAULT_KEYRING_ID);
8994
this.knownCryptoKeys = new HashSet<>();
95+
this.enableKeyRotation = Boolean.parseBoolean(
96+
properties.getOrDefault(ENABLE_KEY_ROTATION, "true"));
97+
9098
}
9199

92100
/**
@@ -115,7 +123,7 @@ private String getSystemProjectId(String metadataServerApi) throws IOException {
115123
* @return an authorized CloudKMS client
116124
* @throws IOException if credentials can not be created in current environment
117125
*/
118-
private CloudKMS createCloudKMS(String serviceAccountFile) throws IOException {
126+
protected CloudKMS createCloudKMS(String serviceAccountFile) throws IOException {
119127
HttpTransport transport = new NetHttpTransport();
120128
JsonFactory jsonFactory = new JacksonFactory();
121129
GoogleCredential credential;
@@ -163,13 +171,66 @@ void createKeyRingIfNotExists() throws IOException {
163171
}
164172
}
165173

174+
public void enableRotationForExistingKeys() throws IOException {
175+
if (!this.enableKeyRotation) {
176+
return;
177+
}
178+
179+
String parentKeyRing = String.format("projects/%s/locations/%s/keyRings/%s", projectId,
180+
LOCATION_ID, keyringId);
181+
try {
182+
List<CryptoKey> cryptoKeys = cloudKMS.projects().locations().keyRings().cryptoKeys()
183+
.list(parentKeyRing).execute().getCryptoKeys();
184+
if (cryptoKeys == null || cryptoKeys.isEmpty()) {
185+
LOG.info("No existing crypto keys found in keyring {}.", keyringId);
186+
return;
187+
}
188+
189+
for (CryptoKey cryptoKey : cryptoKeys) {
190+
String cryptoKeyName = cryptoKey.getName();
191+
String cryptoKeyId = cryptoKeyName.substring(
192+
cryptoKeyName.lastIndexOf('/') + 1);
193+
194+
if (cryptoKey.getRotationPeriod() == null || cryptoKey.getRotationPeriod().isEmpty()) {
195+
CryptoKey updateRequest = new CryptoKey();
196+
// Setting rotation period to 90 days for KMS keys.
197+
long rotationInSeconds = java.time.Duration.ofDays(90).getSeconds();
198+
updateRequest.setRotationPeriod(rotationInSeconds + "s");
199+
// Setting nextRotationTime to 1 day after the key is updated.
200+
Instant nextRotationTime = Instant.now().plus(java.time.Duration.ofDays(1));
201+
updateRequest.setNextRotationTime(nextRotationTime.toString());
202+
203+
String resourceName = String.format("projects/%s/locations/%s/keyRings/%s/cryptoKeys/%s",
204+
projectId, LOCATION_ID, keyringId, cryptoKeyId);
205+
206+
try {
207+
cloudKMS.projects().locations().keyRings().cryptoKeys()
208+
.patch(resourceName, updateRequest)
209+
.setUpdateMask(
210+
"rotation_period,next_rotation_time")
211+
.execute();
212+
LOG.info("Successfully updated rotation period for crypto key {}.", cryptoKeyId);
213+
} catch (GoogleJsonResponseException e) {
214+
throw new IOException(
215+
String.format("Failed to update rotation period for crypto key %s due to %s",
216+
cryptoKeyId, e.getMessage()), e);
217+
}
218+
}
219+
}
220+
} catch (GoogleJsonResponseException e) {
221+
throw new IOException(
222+
String.format("Exception occurred while listing crypto keys in keyring %s: %s",
223+
keyringId, e.getMessage()), e);
224+
}
225+
}
226+
166227
/**
167228
* Creates a new crypto key on google cloud kms with the given id.
168229
*
169230
* @param cryptoKeyId crypto key id
170231
* @throws IOException if there's an error creating crypto key
171232
*/
172-
void createCryptoKeyIfNotExists(String cryptoKeyId) throws IOException {
233+
public void createCryptoKeyIfNotExists(String cryptoKeyId) throws IOException {
173234
// If crypto key is already created, do not attempt to create it again.
174235
if (knownCryptoKeys.contains(cryptoKeyId)) {
175236
return;
@@ -182,6 +243,14 @@ void createCryptoKeyIfNotExists(String cryptoKeyId) throws IOException {
182243
// This will allow the API access to the key for symmetric encryption and decryption.
183244
cryptoKey.setPurpose(ENCRYPT_DECRYPT);
184245

246+
if (this.enableKeyRotation) {
247+
long rotationInSeconds = java.time.Duration.ofDays(90).getSeconds();
248+
cryptoKey.setRotationPeriod(rotationInSeconds + "s");
249+
250+
Instant nextRotationTime = Instant.now().plus(java.time.Duration.ofDays(1));
251+
cryptoKey.setNextRotationTime(nextRotationTime.toString());
252+
}
253+
185254
try {
186255
cloudKMS.projects().locations().keyRings().cryptoKeys()
187256
.create(parent, cryptoKey)

‎cdap-securestore-ext-cloudkms/src/main/java/io/cdap/cdap/securestore/gcp/cloudkms/CloudSecretManager.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public void initialize(SecretManagerContext context) throws IOException {
5555
this.store = context.getSecretStore();
5656
this.client = new CloudKMSClient(context.getProperties());
5757
this.client.createKeyRingIfNotExists();
58+
this.client.enableRotationForExistingKeys();
5859
}
5960

6061
@Override

‎cdap-securestore-ext-cloudkms/src/test/java/io/cdap/cdap/securestore/spi/CloudKMSClientTest.java‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package io.cdap.cdap.securestore.spi;
2+
3+
import com.google.api.services.cloudkms.v1.CloudKMS;
4+
import com.google.api.services.cloudkms.v1.model.CryptoKey;
5+
import com.google.api.services.cloudkms.v1.model.ListCryptoKeysResponse;
6+
import io.cdap.cdap.securestore.gcp.cloudkms.CloudKMSClient;
7+
import java.io.IOException;
8+
import java.time.Duration;
9+
import java.time.Instant;
10+
import java.util.Arrays;
11+
import java.util.HashMap;
12+
import java.util.Map;
13+
import org.junit.Before;
14+
import org.junit.Test;
15+
import org.junit.runner.RunWith;
16+
import org.mockito.ArgumentCaptor;
17+
import org.mockito.Captor;
18+
import org.mockito.Mock;
19+
import org.mockito.junit.MockitoJUnitRunner;
20+
21+
import static org.junit.Assert.assertEquals;
22+
import static org.junit.Assert.assertNotNull;
23+
import static org.junit.Assert.assertTrue;
24+
import static org.mockito.ArgumentMatchers.any;
25+
import static org.mockito.ArgumentMatchers.anyString;
26+
import static org.mockito.Mockito.times;
27+
import static org.mockito.Mockito.verify;
28+
import static org.mockito.Mockito.when;
29+
30+
@RunWith(MockitoJUnitRunner.class)
31+
public class CloudKMSClientTest {
32+
33+
private static final String TEST_PROJECT_ID = "test-project";
34+
private static final String TEST_KEYRING_ID = "test-instance";
35+
private static final String LOCATION_ID = "global";
36+
37+
@Mock
38+
private CloudKMS mockKms;
39+
@Mock
40+
private CloudKMS.Projects mockProjects;
41+
@Mock
42+
private CloudKMS.Projects.Locations mockLocations;
43+
@Mock
44+
private CloudKMS.Projects.Locations.KeyRings mockKeyRings;
45+
@Mock
46+
private CloudKMS.Projects.Locations.KeyRings.CryptoKeys mockCryptoKeys;
47+
@Mock
48+
private CloudKMS.Projects.Locations.KeyRings.CryptoKeys.Create mockCreate;
49+
@Mock
50+
private CloudKMS.Projects.Locations.KeyRings.CryptoKeys.Patch mockPatch;
51+
@Mock
52+
private CloudKMS.Projects.Locations.KeyRings.CryptoKeys.List mockList;
53+
54+
@Captor
55+
private ArgumentCaptor<CryptoKey> cryptoKeyCaptor;
56+
@Captor
57+
private ArgumentCaptor<String> resourceNameCaptor;
58+
@Captor
59+
private ArgumentCaptor<String> updateMaskCaptor;
60+
61+
private CloudKMSClient cloudKMSClient;
62+
63+
@Before
64+
public void setUp() throws Exception {
65+
when(mockKms.projects()).thenReturn(mockProjects);
66+
when(mockProjects.locations()).thenReturn(mockLocations);
67+
when(mockLocations.keyRings()).thenReturn(mockKeyRings);
68+
when(mockKeyRings.cryptoKeys()).thenReturn(mockCryptoKeys);
69+
when(mockCryptoKeys.create(anyString(), any(CryptoKey.class))).thenReturn(mockCreate);
70+
when(mockCreate.setCryptoKeyId(anyString())).thenReturn(mockCreate);
71+
when(mockCryptoKeys.patch(anyString(), any(CryptoKey.class))).thenReturn(mockPatch);
72+
when(mockPatch.setUpdateMask(anyString())).thenReturn(mockPatch);
73+
when(mockCryptoKeys.list(anyString())).thenReturn(mockList);
74+
75+
Map<String, String> properties = new HashMap<>();
76+
properties.put("project.id", TEST_PROJECT_ID);
77+
properties.put("keyring.id", TEST_KEYRING_ID);
78+
cloudKMSClient = new CloudKMSClient(properties) {
79+
@Override
80+
protected CloudKMS createCloudKMS(String serviceAccountFile) {
81+
return mockKms;
82+
}
83+
};
84+
}
85+
86+
@Test
87+
public void testCreateCryptoKeyIfNotExists_setsRotationPeriodDetails() throws IOException {
88+
String cryptoKeyId = "new-test-key";
89+
Instant testStartTime = Instant.now();
90+
cloudKMSClient.createCryptoKeyIfNotExists(cryptoKeyId);
91+
92+
verify(mockCryptoKeys).create(anyString(), cryptoKeyCaptor.capture());
93+
CryptoKey capturedKey = cryptoKeyCaptor.getValue();
94+
95+
long expectedSeconds = Duration.ofDays(90).getSeconds();
96+
assertEquals(expectedSeconds + "s", capturedKey.getRotationPeriod());
97+
98+
assertNotNull(capturedKey.getNextRotationTime());
99+
Instant nextRotation = Instant.parse(capturedKey.getNextRotationTime());
100+
assertTrue(nextRotation.isAfter(testStartTime));
101+
}
102+
103+
@Test
104+
public void testEnableRotationForExistingKeys_updatesKeyWithoutRotation() throws IOException {
105+
String keyToUpdateId = "key-without-rotation";
106+
String keyToUpdateFullName = String.format("projects/%s/locations/%s/keyRings/%s/cryptoKeys/%s",
107+
TEST_PROJECT_ID, LOCATION_ID, TEST_KEYRING_ID, keyToUpdateId);
108+
109+
String keyToSkipId = "key-with-rotation";
110+
String keyToSkipFullName = String.format("projects/%s/locations/%s/keyRings/%s/cryptoKeys/%s",
111+
TEST_PROJECT_ID, LOCATION_ID, TEST_KEYRING_ID, keyToSkipId);
112+
113+
CryptoKey keyToUpdate = new CryptoKey().setName(keyToUpdateFullName);
114+
CryptoKey keyToSkip = new CryptoKey().setName(keyToSkipFullName).setRotationPeriod("7776000s");
115+
116+
ListCryptoKeysResponse response = new ListCryptoKeysResponse().setCryptoKeys(
117+
Arrays.asList(keyToUpdate, keyToSkip));
118+
when(mockList.execute()).thenReturn(response);
119+
120+
cloudKMSClient.enableRotationForExistingKeys();
121+
122+
verify(mockPatch, times(1)).execute();
123+
verify(mockCryptoKeys).patch(resourceNameCaptor.capture(), cryptoKeyCaptor.capture());
124+
verify(mockPatch).setUpdateMask(updateMaskCaptor.capture());
125+
126+
assertEquals(keyToUpdateFullName, resourceNameCaptor.getValue());
127+
128+
long expectedSeconds = Duration.ofDays(90).getSeconds();
129+
assertEquals(expectedSeconds + "s", cryptoKeyCaptor.getValue().getRotationPeriod());
130+
assertEquals("rotation_period,next_rotation_time", updateMaskCaptor.getValue());
131+
}
132+
133+
}
134+

0 commit comments

Comments
 (0)