Skip to content

Commit 8ff0041

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

5 files changed

Lines changed: 232 additions & 4 deletions

File tree

‎cdap-common/src/main/resources/cdap-default.xml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5020,6 +5020,16 @@
50205020
</description>
50215021
</property>
50225022

5023+
<property>
5024+
<name>security.store.system.properties.gcp-cloudkms.enable.key.auto.rotation</name>
5025+
<value>true</value>
5026+
<description>
5027+
Checks whether to enable auto-rotation when creating KMS keys for
5028+
encrypting/decrypting secure keys. Also checks whether to enable
5029+
auto-rotation for existing keys during upgrade.
5030+
</description>
5031+
</property>
5032+
50235033
<property>
50245034
<name>security.token.digest.algorithm</name>
50255035
<value>HmacSHA256</value>

‎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: 84 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_AUTO_ROTATION = "enable.key.auto.rotation";
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 enableKeyAutoRotation;
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,8 @@ 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.enableKeyAutoRotation = Boolean.getBoolean(properties.getOrDefault(ENABLE_KEY_AUTO_ROTATION, "true"));
96+
9097
}
9198

9299
/**
@@ -115,7 +122,7 @@ private String getSystemProjectId(String metadataServerApi) throws IOException {
115122
* @return an authorized CloudKMS client
116123
* @throws IOException if credentials can not be created in current environment
117124
*/
118-
private CloudKMS createCloudKMS(String serviceAccountFile) throws IOException {
125+
protected CloudKMS createCloudKMS(String serviceAccountFile) throws IOException {
119126
HttpTransport transport = new NetHttpTransport();
120127
JsonFactory jsonFactory = new JacksonFactory();
121128
GoogleCredential credential;
@@ -163,13 +170,78 @@ void createKeyRingIfNotExists() throws IOException {
163170
}
164171
}
165172

173+
public void enableAutoRotationForExistingKeys() throws IOException {
174+
if(!this.enableKeyAutoRotation){
175+
return;
176+
}
177+
String parentKeyRing = String.format("projects/%s/locations/%s/keyRings/%s", projectId,
178+
LOCATION_ID, keyringId);
179+
LOG.debug("Listing crypto keys in keyring {}.", keyringId);
180+
try {
181+
List<CryptoKey> cryptoKeys = cloudKMS.projects().locations().keyRings().cryptoKeys()
182+
.list(parentKeyRing).execute().getCryptoKeys();
183+
if (cryptoKeys == null || cryptoKeys.isEmpty()) {
184+
LOG.info("No existing crypto keys found in keyring {}.", keyringId);
185+
return;
186+
}
187+
LOG.info("Found {} existing crypto keys in keyring {}. Checking rotation periods.",
188+
cryptoKeys.size(), keyringId);
189+
190+
for (CryptoKey cryptoKey : cryptoKeys) {
191+
String cryptoKeyName = cryptoKey.getName(); // Full resource name of the crypto key
192+
LOG.debug("Full crypto key name: {}", cryptoKeyName);
193+
String cryptoKeyId = cryptoKeyName.substring(cryptoKeyName.lastIndexOf('/') + 1);
194+
LOG.debug("crypto key id: {}", cryptoKeyId);
195+
if (cryptoKey.getRotationPeriod() == null || cryptoKey.getRotationPeriod().isEmpty()) {
196+
LOG.info("Crypto key {} does not have a rotation period set. Updating it.", cryptoKeyId);
197+
198+
// Create an update request
199+
CryptoKey updateRequest = new CryptoKey();
200+
long rotationInSeconds = java.time.Duration.ofDays(1).getSeconds();
201+
updateRequest.setRotationPeriod(rotationInSeconds + "s");
202+
203+
// Calculate next rotation time (1 minute from now), FOR PRODUCTION, MAKE THIS A REASONABLE FUTURE DATE.
204+
Instant nextRotationTime = Instant.now().plus(java.time.Duration.ofMinutes(1));
205+
updateRequest.setNextRotationTime(nextRotationTime.toString());
206+
207+
// The resource name for the update operation is the full name of the cryptokey
208+
String resourceName = String.format("projects/%s/locations/%s/keyRings/%s/cryptoKeys/%s",
209+
projectId, LOCATION_ID, keyringId, cryptoKeyId);
210+
211+
LOG.debug("Key resource: {}, updates rotation period: {}, updates rotation time next: {}", resourceName, rotationInSeconds+"s",nextRotationTime.toString());
212+
213+
try {
214+
// Patch the crypto key to update its rotation period and next rotation time
215+
cloudKMS.projects().locations().keyRings().cryptoKeys()
216+
.patch(resourceName, updateRequest)
217+
.setUpdateMask(
218+
"rotation_period,next_rotation_time") // IMPORTANT: specify fields to update
219+
.execute();
220+
LOG.info("Successfully updated rotation period for crypto key {}.", cryptoKeyId);
221+
} catch (GoogleJsonResponseException e) {
222+
LOG.error("Failed to update rotation period for crypto key {}: {}", cryptoKeyId,
223+
e.getMessage(), e);
224+
// Decide if you want to throw an exception here or just log and continue
225+
}
226+
} else {
227+
LOG.debug("Crypto key {} already has a rotation period set ({}). Skipping.",
228+
cryptoKeyId, cryptoKey.getRotationPeriod());
229+
}
230+
}
231+
232+
} catch (GoogleJsonResponseException e) {
233+
throw new IOException(
234+
String.format("Exception occurred while listing crypto keys in keyring %s: %s", keyringId, e.getMessage()), e);
235+
}
236+
}
237+
166238
/**
167239
* Creates a new crypto key on google cloud kms with the given id.
168240
*
169241
* @param cryptoKeyId crypto key id
170242
* @throws IOException if there's an error creating crypto key
171243
*/
172-
void createCryptoKeyIfNotExists(String cryptoKeyId) throws IOException {
244+
public void createCryptoKeyIfNotExists(String cryptoKeyId) throws IOException {
173245
// If crypto key is already created, do not attempt to create it again.
174246
if (knownCryptoKeys.contains(cryptoKeyId)) {
175247
return;
@@ -181,6 +253,14 @@ void createCryptoKeyIfNotExists(String cryptoKeyId) throws IOException {
181253
CryptoKey cryptoKey = new CryptoKey();
182254
// This will allow the API access to the key for symmetric encryption and decryption.
183255
cryptoKey.setPurpose(ENCRYPT_DECRYPT);
256+
if (this.enableKeyAutoRotation) {
257+
// long rotationInSeconds = java.time.Duration.ofDays(90).getSeconds();
258+
long rotationInSeconds = java.time.Duration.ofDays(1).getSeconds();
259+
cryptoKey.setRotationPeriod(rotationInSeconds + "s");
260+
// Calculate next rotation time (1 minute from now), FOR TESTING ONLY.
261+
Instant nextRotationTime = Instant.now().plus(java.time.Duration.ofMinutes(1));
262+
cryptoKey.setNextRotationTime(nextRotationTime.toString());
263+
}
184264

185265
try {
186266
cloudKMS.projects().locations().keyRings().cryptoKeys()

‎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.enableAutoRotationForExistingKeys();
5859
}
5960

6061
@Override

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

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
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_setsRotationPeriod() 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(1).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 testEnableAutoRotationForExistingKeys_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(Arrays.asList(keyToUpdate, keyToSkip));
117+
when(mockList.execute()).thenReturn(response);
118+
119+
cloudKMSClient.enableAutoRotationForExistingKeys();
120+
121+
verify(mockPatch, times(1)).execute();
122+
verify(mockCryptoKeys).patch(resourceNameCaptor.capture(), cryptoKeyCaptor.capture());
123+
verify(mockPatch).setUpdateMask(updateMaskCaptor.capture());
124+
125+
assertEquals(keyToUpdateFullName, resourceNameCaptor.getValue());
126+
127+
long expectedSeconds = Duration.ofDays(1).getSeconds();
128+
assertEquals(expectedSeconds + "s", cryptoKeyCaptor.getValue().getRotationPeriod());
129+
assertEquals("rotation_period,next_rotation_time", updateMaskCaptor.getValue());
130+
}
131+
132+
}
133+

0 commit comments

Comments
 (0)