Lazy decryption of secrets.

square · Jun 28, 2016 · cf067c8 · cf067c8
1 parent c7aa3a9
commit cf067c8
Show file tree

Hide file tree

Showing 18 changed files with 62 additions and 58 deletions.
diff --git a/api/src/main/java/keywhiz/api/model/Secret.java b/api/src/main/java/keywhiz/api/model/Secret.java
@@ -18,7 +18,6 @@
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableMap;
-import java.text.ParseException;
 import java.util.Map;
 import java.util.Optional;
 import javax.annotation.Nullable;
@@ -27,7 +26,6 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Strings.nullToEmpty;
-import static java.util.regex.Pattern.quote;
 import static org.apache.commons.lang3.StringUtils.chomp;
 import static org.apache.commons.lang3.StringUtils.removeEnd;
 
@@ -47,7 +45,8 @@ public class Secret {
   private final String description;
 
   /** Base64-encoded content of this version of the secret. */
-  private final String secret;
+  private String secret;
+  private final LazyString encryptedSecret;
 
   private final ApiDate createdAt;
   private final String createdBy;
@@ -61,22 +60,22 @@ public class Secret {
   private final ImmutableMap<String, String> generationOptions;
 
   public Secret(long id,
-      String name,
-      @Nullable String description,
-      String secret,
-      ApiDate createdAt,
-      @Nullable String createdBy,
-      ApiDate updatedAt,
-      @Nullable String updatedBy,
-      @Nullable Map<String, String> metadata,
-      @Nullable String type,
-      @Nullable Map<String, String> generationOptions) {
+                String name,
+                @Nullable String description,
+                LazyString encryptedSecret,
+                ApiDate createdAt,
+                @Nullable String createdBy,
+                ApiDate updatedAt,
+                @Nullable String updatedBy,
+                @Nullable Map<String, String> metadata,
+                @Nullable String type,
+                @Nullable Map<String, String> generationOptions) {
 
     checkArgument(!name.isEmpty());
     this.id = id;
     this.name = name;
     this.description = nullToEmpty(description);
-    this.secret = checkNotNull(secret); /* Expected empty when sanitized. */
+    this.encryptedSecret = checkNotNull(encryptedSecret);
     this.createdAt = checkNotNull(createdAt);
     this.createdBy = nullToEmpty(createdBy);
     this.updatedAt = checkNotNull(updatedAt);
@@ -106,6 +105,9 @@ public String getDescription() {
   }
 
   public String getSecret() {
+    if (secret == null) {
+      secret = checkNotNull(encryptedSecret.decrypt());
+    }
     return secret;
   }
 
@@ -152,7 +154,7 @@ public boolean equals(Object o) {
       if (Objects.equal(this.id, that.id) &&
           Objects.equal(this.name, that.name) &&
           Objects.equal(this.description, that.description) &&
-          Objects.equal(this.secret, that.secret) &&
+          Objects.equal(this.getSecret(), that.getSecret()) &&
           Objects.equal(this.createdAt, that.createdAt) &&
           Objects.equal(this.createdBy, that.createdBy) &&
           Objects.equal(this.updatedAt, that.updatedAt) &&
@@ -167,7 +169,7 @@ public boolean equals(Object o) {
   }
 
   @Override public int hashCode() {
-    return Objects.hashCode(id, name, description, secret, createdAt, createdBy, updatedAt,
+    return Objects.hashCode(id, name, description, getSecret(), createdAt, createdBy, updatedAt,
         updatedBy, metadata, type, generationOptions);
   }
 
@@ -187,4 +189,8 @@ public String toString() {
         .add("generationOptions", generationOptions)
         .toString();
   }
+
+  public interface LazyString {
+    String decrypt();
+  }
 }
diff --git a/api/src/test/java/keywhiz/api/AutomationSecretResponseTest.java b/api/src/test/java/keywhiz/api/AutomationSecretResponseTest.java
@@ -33,8 +33,8 @@ public class AutomationSecretResponseTest {
   private static final ImmutableMap<String, String> metadata =
       ImmutableMap.of("key1", "value1", "key2", "value2");
   private static final ApiDate NOW = ApiDate.now();
-  private static final Secret secret = new Secret(0, "name", null,
-      "YWJj", NOW, null, NOW, null, metadata, "upload", null);
+  private static final Secret secret = new Secret(0, "name", null, () -> "YWJj", NOW, null, NOW, null, metadata,
+      "upload", null);
 
   @Test
   public void setsLength() {

diff --git a/api/src/test/java/keywhiz/api/SecretDeliveryResponseTest.java b/api/src/test/java/keywhiz/api/SecretDeliveryResponseTest.java
@@ -30,8 +30,8 @@ public class SecretDeliveryResponseTest {
   private static final ImmutableMap<String, String> metadata =
       ImmutableMap.of("key1", "value1", "key2", "value2");
   private static final ApiDate NOW = ApiDate.now();
-  private static final Secret secret = new Secret(0, "name", null,
-      "YWJj", NOW, null, NOW, null, metadata, "upload", null);
+  private static final Secret secret = new Secret(0, "name", null, () -> "YWJj", NOW, null, NOW, null, metadata,
+      "upload", null);
 
   @Test
   public void setsLength() {

diff --git a/api/src/test/java/keywhiz/api/model/SecretTest.java b/api/src/test/java/keywhiz/api/model/SecretTest.java
@@ -15,15 +15,18 @@
  */
 package keywhiz.api.model;
 
-import java.text.ParseException;
 import java.util.Base64;
+
+import keywhiz.api.ApiDate;
 import org.junit.Test;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static keywhiz.api.model.Secret.decodedLength;
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class SecretTest {
+  int called = 0;
+
   @Test public void decodedLengthCalculates() {
     Base64.Encoder encoder = Base64.getEncoder();
     String base64 = encoder.encodeToString("31337".getBytes(UTF_8));
@@ -40,4 +43,10 @@ public class SecretTest {
 
     assertThat(decodedLength("")).isZero();
   }
+
+  @Test public void callsDecryptOnlyOnce() {
+    Secret s = new Secret(42, "toto", null, () -> String.valueOf(++called), ApiDate.now(), "", ApiDate.now(), "", null, null, null);
+    assertThat(s.getSecret()).isEqualTo("1");
+    assertThat(s.getSecret()).isEqualTo("1");
+  }
 }
diff --git a/cli/src/test/java/keywhiz/cli/commands/AddActionTest.java b/cli/src/test/java/keywhiz/cli/commands/AddActionTest.java
@@ -55,8 +55,8 @@ public class AddActionTest {
 
   Client client = new Client(4, "newClient", null, null, null, null, null, true, false);
   Group group = new Group(4, "newGroup", null, null, null, null, null);
-  Secret secret = new Secret(15, "newSecret", null, "c2VjcmV0MQ==",
-      NOW, null, NOW, null, null, null, ImmutableMap.of());
+  Secret secret = new Secret(15, "newSecret", null, () -> "c2VjcmV0MQ==", NOW, null, NOW, null, null, null,
+      ImmutableMap.of());
   SanitizedSecret sanitizedSecret = SanitizedSecret.fromSecret(secret);
   SecretDetailResponse secretDetailResponse = SecretDetailResponse.fromSecret(secret, null, null);
 

diff --git a/cli/src/test/java/keywhiz/cli/commands/AssignActionTest.java b/cli/src/test/java/keywhiz/cli/commands/AssignActionTest.java
@@ -53,8 +53,8 @@ public class AssignActionTest {
   Group group = new Group(5, "group", null, null, null, null, null);
   GroupDetailResponse groupDetailResponse = GroupDetailResponse.fromGroup(group,
       ImmutableList.<SanitizedSecret>of(), ImmutableList.<Client>of());
-  Secret secret = new Secret(16, "secret", null, "c2VjcmV0MQ==", NOW,
-      null, NOW, null, null, null, ImmutableMap.of());
+  Secret secret = new Secret(16, "secret", null, () -> "c2VjcmV0MQ==", NOW, null, NOW, null, null, null,
+      ImmutableMap.of());
   SanitizedSecret sanitizedSecret = SanitizedSecret.fromSecret(secret);
 
   @Before

diff --git a/cli/src/test/java/keywhiz/cli/commands/DeleteActionTest.java b/cli/src/test/java/keywhiz/cli/commands/DeleteActionTest.java
@@ -50,8 +50,8 @@ public class DeleteActionTest {
   DeleteActionConfig deleteActionConfig;
   DeleteAction deleteAction;
 
-  Secret secret = new Secret(0, "secret", null, "c2VjcmV0MQ==", NOW,
-      null, NOW, null, null, null, ImmutableMap.of());
+  Secret secret = new Secret(0, "secret", null, () -> "c2VjcmV0MQ==", NOW, null, NOW, null, null, null,
+      ImmutableMap.of());
   SanitizedSecret sanitizedSecret = SanitizedSecret.fromSecret(secret);
 
   ByteArrayInputStream yes;

diff --git a/cli/src/test/java/keywhiz/cli/commands/DescribeActionTest.java b/cli/src/test/java/keywhiz/cli/commands/DescribeActionTest.java
@@ -48,8 +48,8 @@ public class DescribeActionTest {
 
   DescribeActionConfig describeActionConfig;
   DescribeAction describeAction;
-  Secret secret = new Secret(0, "secret", null, "c2VjcmV0MQ==", NOW,
-      null, NOW, null, null, null, ImmutableMap.of());
+  Secret secret = new Secret(0, "secret", null, () ->  "c2VjcmV0MQ==", NOW, null, NOW, null, null, null,
+      ImmutableMap.of());
   SanitizedSecret sanitizedSecret = SanitizedSecret.fromSecret(secret);
 
   @Before

diff --git a/cli/src/test/java/keywhiz/cli/commands/UnassignActionTest.java b/cli/src/test/java/keywhiz/cli/commands/UnassignActionTest.java
@@ -49,8 +49,8 @@ public class UnassignActionTest {
 
   Client client = new Client(11, "client-name", null, null, null, null, null, false, false);
   Group group = new Group(22, "group-name", null, null, null, null, null);
-  Secret secret = new Secret(33, "secret-name", null, "c2VjcmV0MQ==", NOW,
-      null, NOW, null, null, null, ImmutableMap.of());
+  Secret secret = new Secret(33, "secret-name", null, () -> "c2VjcmV0MQ==", NOW, null, NOW, null, null, null,
+      ImmutableMap.of());
   SanitizedSecret sanitizedSecret = SanitizedSecret.fromSecret(secret);
   GroupDetailResponse groupDetailResponse = GroupDetailResponse.fromGroup(group,
       ImmutableList.of(sanitizedSecret), ImmutableList.of(client));

diff --git a/server/src/main/java/keywhiz/service/crypto/SecretTransformer.java b/server/src/main/java/keywhiz/service/crypto/SecretTransformer.java
@@ -16,8 +16,6 @@
 
 package keywhiz.service.crypto;
 
-import java.util.List;
-import java.util.stream.Collectors;
 import javax.inject.Inject;
 import keywhiz.api.model.Secret;
 import keywhiz.api.model.SecretContent;
@@ -44,13 +42,11 @@ public Secret transform(SecretSeriesAndContent seriesAndContent) {
     SecretSeries series = seriesAndContent.series();
     SecretContent content = seriesAndContent.content();
 
-    final String secretContent = cryptographer.decrypt(content.encryptedContent());
-
     return new Secret(
         series.id(),
         series.name(),
         series.description(),
-        secretContent,
+        () -> cryptographer.decrypt(content.encryptedContent()),
         content.createdAt(),
         content.createdBy(),
         content.updatedAt(),
@@ -59,11 +55,4 @@ public Secret transform(SecretSeriesAndContent seriesAndContent) {
         series.type().orElse(null),
         series.generationOptions());
   }
-
-  /**
-   * Transform a list of DB content to Secret models.
-   */
-  public List<Secret> transform(List<SecretSeriesAndContent> seriesAndContents) {
-    return seriesAndContents.stream().map(this::transform).collect(Collectors.toList());
-  }
 }
diff --git a/server/src/test/java/keywhiz/service/resources/SecretDeliveryResourceIntegrationTest.java b/server/src/test/java/keywhiz/service/resources/SecretDeliveryResourceIntegrationTest.java
@@ -43,7 +43,7 @@ public class SecretDeliveryResourceIntegrationTest {
 
   @Before public void setUp() throws Exception {
     client = TestClients.mutualSslClient();
-    generalPassword = new Secret(0, "General_Password", null, "YXNkZGFz",
+    generalPassword = new Secret(0, "General_Password", null, () -> "YXNkZGFz",
         ApiDate.parse("2011-09-29T15:46:00Z"), null,
         ApiDate.parse("2011-09-29T15:46:00Z"), null, null, "upload", null);
   }

diff --git a/server/src/test/java/keywhiz/service/resources/SecretDeliveryResourceTest.java b/server/src/test/java/keywhiz/service/resources/SecretDeliveryResourceTest.java
@@ -47,17 +47,17 @@ public class SecretDeliveryResourceTest {
   SecretDeliveryResource secretDeliveryResource;
 
   final Client client = new Client(0, "principal", null, null, null, null, null, false, false);
-  final Secret secret = new Secret(0, "secret_name", null, "secret_value", NOW, null, NOW, null,
+  final Secret secret = new Secret(0, "secret_name", null, () -> "secret_value", NOW, null, NOW, null,
       null, null, null);
-  final Secret secretBase64 = new Secret(1, "Base64With=", null, "SGVsbG8=", NOW, null, NOW,
+  final Secret secretBase64 = new Secret(1, "Base64With=", null, () -> "SGVsbG8=", NOW, null, NOW,
       null, null, null, null);
 
   @Before public void setUp() {
     secretDeliveryResource = new SecretDeliveryResource(secretController, aclDAO, clientDAO);
   }
 
   @Test public void returnsSecretWhenAllowed() throws Exception {
-    Secret secret = new Secret(0, "secret_name", null, "unused_secret", NOW, null, NOW, null, null, null, null);
+    Secret secret = new Secret(0, "secret_name", null, () -> "unused_secret", NOW, null, NOW, null, null, null, null);
     SanitizedSecret sanitizedSecret = SanitizedSecret.fromSecret(secret);
     String name = sanitizedSecret.name();
 
@@ -72,7 +72,7 @@ public class SecretDeliveryResourceTest {
 
   @Test public void returnsVersionedSecretWhenAllowed() throws Exception {
     String name = "secret_name";
-    Secret versionedSecret = new Secret(2, name, null, "U3BpZGVybWFu", NOW, null, NOW,
+    Secret versionedSecret = new Secret(2, name, null, () -> "U3BpZGVybWFu", NOW, null, NOW,
         null, null, null, null);
 
     when(aclDAO.getSanitizedSecretFor(client, name))

diff --git a/server/src/test/java/keywhiz/service/resources/SecretsDeliveryResourceIntegrationTest.java b/server/src/test/java/keywhiz/service/resources/SecretsDeliveryResourceIntegrationTest.java
@@ -53,24 +53,24 @@ public void setUp() throws Exception {
 
     generalPassword = SecretDeliveryResponse.fromSanitizedSecret(
         SanitizedSecret.fromSecret(
-            new Secret(0, "General_Password", null, "YXNkZGFz",
+            new Secret(0, "General_Password", null, () -> "YXNkZGFz",
                 ApiDate.parse("2011-09-29T15:46:00.312Z"), null,
                 ApiDate.parse("2011-09-29T15:46:00.312Z"), null, null, null, null)));
     databasePassword = SecretDeliveryResponse.fromSanitizedSecret(
         SanitizedSecret.fromSecret(
-            new Secret(1, "Database_Password", null, "MTIzNDU=",
+            new Secret(1, "Database_Password", null, () -> "MTIzNDU=",
                 ApiDate.parse("2011-09-29T15:46:00.232Z"), null,
                 ApiDate.parse("2011-09-29T15:46:00.232Z"), null, null, null, null)));
     nobodyPgPassPassword = SecretDeliveryResponse.fromSanitizedSecret(
         SanitizedSecret.fromSecret(
             new Secret(2, "Nobody_PgPass", null,
-                "c29tZWhvc3Quc29tZXBsYWNlLmNvbTo1NDMyOnNvbWVkYXRhYmFzZTptaXN0ZXJhd2Vzb21lOmhlbGwwTWNGbHkK",
+                () -> "c29tZWhvc3Quc29tZXBsYWNlLmNvbTo1NDMyOnNvbWVkYXRhYmFzZTptaXN0ZXJhd2Vzb21lOmhlbGwwTWNGbHkK",
                 ApiDate.parse("2011-09-29T15:46:00.232Z"), null,
                 ApiDate.parse("2011-09-29T15:46:00.232Z"), null,
                 ImmutableMap.of("owner", "nobody", "mode", "0400"), null, null)));
     nonExistentOwnerPass = SecretDeliveryResponse.fromSanitizedSecret(
         SanitizedSecret.fromSecret(
-            new Secret(3, "NonexistentOwner_Pass", null, "MTIzNDU=",
+            new Secret(3, "NonexistentOwner_Pass", null, () -> "MTIzNDU=",
                 ApiDate.parse("2011-09-29T15:46:00.232Z"), null,
                 ApiDate.parse("2011-09-29T15:46:00.232Z"), null,
                 ImmutableMap.of("owner", "NonExistent", "mode", "0400"), null, null)));

diff --git a/server/src/test/java/keywhiz/service/resources/SecretsDeliveryResourceTest.java b/server/src/test/java/keywhiz/service/resources/SecretsDeliveryResourceTest.java
@@ -44,11 +44,11 @@ public class SecretsDeliveryResourceTest {
   SecretsDeliveryResource secretsDeliveryResource;
 
   Secret firstSecret = new Secret(0, "first_secret_name", null,
-      Base64.getEncoder().encodeToString("first_secret_contents".getBytes(UTF_8)), NOW, null, NOW, null, null,
+      () -> Base64.getEncoder().encodeToString("first_secret_contents".getBytes(UTF_8)), NOW, null, NOW, null, null,
       null, null);
   SanitizedSecret sanitizedFirstSecret = SanitizedSecret.fromSecret(firstSecret);
   Secret secondSecret = new Secret(1, "second_secret_name", null,
-      Base64.getEncoder().encodeToString("second_secret_contents".getBytes(UTF_8)), NOW, null, NOW, null, null,
+      () -> Base64.getEncoder().encodeToString("second_secret_contents".getBytes(UTF_8)), NOW, null, NOW, null, null,
       null, null);
   SanitizedSecret sanitizedSecondSecret = SanitizedSecret.fromSecret(secondSecret);
   Client client;

diff --git a/server/src/test/java/keywhiz/service/resources/admin/ClientsResourceTest.java b/server/src/test/java/keywhiz/service/resources/admin/ClientsResourceTest.java
@@ -110,7 +110,7 @@ public class ClientsResourceTest {
   @Test public void includesAssociations() {
     Group group1 = new Group(0, "group1", null, null, null, null, null);
     Group group2 = new Group(0, "group2", null, null, null, null, null);
-    Secret secret = new Secret(15, "secret", null, "supersecretdata", now, "creator", now,
+    Secret secret = new Secret(15, "secret", null, () -> "supersecretdata", now, "creator", now,
         "updater", null, null, null);
 
     when(clientDAO.getClientById(1)).thenReturn(Optional.of(client));

diff --git a/server/src/test/java/keywhiz/service/resources/admin/MembershipResourceTest.java b/server/src/test/java/keywhiz/service/resources/admin/MembershipResourceTest.java
@@ -45,7 +45,7 @@ public class MembershipResourceTest {
   User user = User.named("user");
   Client client = new Client(44, "client", "desc", NOW, "creator", NOW, "updater", true, false);
   Group group = new Group(55, "group", null, null, null, null, null);
-  Secret secret = new Secret(66, "secret", null, "shush", NOW, null, NOW, null, null, null, null);
+  Secret secret = new Secret(66, "secret", null, () -> "shush", NOW, null, NOW, null, null, null, null);
 
   MembershipResource resource;
 

diff --git a/server/src/test/java/keywhiz/service/resources/admin/SecretsResourceTest.java b/server/src/test/java/keywhiz/service/resources/admin/SecretsResourceTest.java
@@ -67,7 +67,7 @@ public class SecretsResourceTest {
   User user = User.named("user");
   ImmutableMap<String, String> emptyMap = ImmutableMap.of();
 
-  Secret secret = new Secret(22, "name", "desc", "secret", NOW, "creator", NOW,
+  Secret secret = new Secret(22, "name", "desc", () -> "secret", NOW, "creator", NOW,
       "updater", emptyMap, null, null);
 
   SecretsResource resource;

diff --git a/server/src/test/java/keywhiz/service/resources/automation/AutomationSecretResourceTest.java b/server/src/test/java/keywhiz/service/resources/automation/AutomationSecretResourceTest.java
@@ -77,7 +77,7 @@ public void addSecret() {
     Secret secret = new Secret(0, /* Set by DB */
         request.name,
         request.description,
-        Base64.getUrlEncoder().encodeToString(request.content.getBytes(UTF_8)),
+        () -> Base64.getUrlEncoder().encodeToString(request.content.getBytes(UTF_8)),
         NOW,
         automation.getName(),
         NOW, /* updatedAt set by DB */