diff options
author | Morten Tokle <mortent@yahooinc.com> | 2023-02-28 09:24:03 +0100 |
---|---|---|
committer | Andreas Eriksen <andreer@yahooinc.com> | 2023-03-02 15:07:40 +0100 |
commit | bbec3fdb956f1af1a9beaabf15c1426726cebb92 (patch) | |
tree | 319713965c5e19d0465de602d1a1b977f651594e | |
parent | 572476ad52aff07de49ce56297954871696ea86c (diff) |
Validate using correct key
3 files changed, 115 insertions, 33 deletions
diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/InstanceValidator.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/InstanceValidator.java index d8bbf743d8c..a349bddc76b 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/InstanceValidator.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/InstanceValidator.java @@ -7,6 +7,9 @@ import com.yahoo.config.model.api.ApplicationInfo; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.model.api.SuperModelProvider; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.Zone; +import com.yahoo.container.jdisc.secretstore.SecretStore; +import com.yahoo.security.KeyUtils; import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.athenz.identityprovider.api.ClusterType; import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; @@ -52,25 +55,32 @@ public class InstanceValidator { private final KeyProvider keyProvider; private final SuperModelProvider superModelProvider; private final NodeRepository nodeRepository; + private final SecretStore secretStore; + private final String sisSecretName; @Inject public InstanceValidator(KeyProvider keyProvider, SuperModelProvider superModelProvider, NodeRepository nodeRepository, - AthenzProviderServiceConfig config) { - this(keyProvider, superModelProvider, nodeRepository, new IdentityDocumentSigner(), new AthenzService(config.tenantService())); + AthenzProviderServiceConfig config, + SecretStore secretStore) { + this(keyProvider, superModelProvider, nodeRepository, new IdentityDocumentSigner(), new AthenzService(config.tenantService()), secretStore, config.sisSecretName()); } public InstanceValidator(KeyProvider keyProvider, SuperModelProvider superModelProvider, NodeRepository nodeRepository, IdentityDocumentSigner identityDocumentSigner, - AthenzService tenantIdentity){ + AthenzService tenantIdentity, + SecretStore secretStore, + String sisSecretName) { this.keyProvider = keyProvider; this.superModelProvider = superModelProvider; this.nodeRepository = nodeRepository; this.signer = identityDocumentSigner; this.tenantDockerContainerIdentity = tenantIdentity; + this.secretStore = secretStore; + this.sisSecretName = sisSecretName; } public boolean isValidInstance(InstanceConfirmation instanceConfirmation) { @@ -102,16 +112,30 @@ public class InstanceValidator { log.log(Level.FINE, () -> String.format("Validating instance %s.", providerUniqueId)); - PublicKey publicKey = keyProvider.getPublicKey(signedIdentityDocument.signingKeyVersion()); + // Find node matching vespa unique id + Node node = getNode(providerUniqueId); + + PublicKey publicKey = publicKey(node, signedIdentityDocument.signingKeyVersion()); if (! signer.hasValidSignature(signedIdentityDocument, publicKey)) { var msg = String.format("Instance %s has invalid signature.", providerUniqueId); throw new ValidationException(Level.SEVERE, () -> msg); } - validateAttributes(req, providerUniqueId); + validateAttributes(node, req, providerUniqueId); log.log(Level.FINE, () -> String.format("Instance %s is valid.", providerUniqueId)); } + private PublicKey publicKey(Node node, int version) { + // return sisSecret for public non-enclave hosts. + Zone zone = nodeRepository.zone(); + if (zone.system().isPublic() && !node.cloudAccount().isEnclave(zone)) { + String keyPem = secretStore.getSecret(sisSecretName, version); + return KeyUtils.extractPublicKey(KeyUtils.fromPemEncodedPrivateKey(keyPem)); + } else { + return keyProvider.getPublicKey(version); + } + } + // TODO Add actual validation. Cannot reuse isValidInstance as identity document is not part of the refresh request. // We'll have to perform some validation on the instance id and other fields of the attribute map. // Separate between tenant and node certificate as well. @@ -121,7 +145,8 @@ public class InstanceValidator { confirmation.provider, confirmation.attributes.get(SAN_DNS_ATTRNAME))); try { - validateAttributes(confirmation, getVespaUniqueInstanceId(confirmation)); + VespaUniqueInstanceId vespaUniqueInstanceId = getVespaUniqueInstanceId(confirmation); + validateAttributes(getNode(vespaUniqueInstanceId), confirmation, vespaUniqueInstanceId); return true; } catch (ValidationException e) { log.log(e.logLevel(), e.messageSupplier()); @@ -146,24 +171,13 @@ public class InstanceValidator { .orElse(null); } - private void validateAttributes(InstanceConfirmation confirmation, VespaUniqueInstanceId vespaUniqueInstanceId) + private void validateAttributes(Node node, InstanceConfirmation confirmation, VespaUniqueInstanceId vespaUniqueInstanceId) throws ValidationException { if(vespaUniqueInstanceId == null) { var msg = "Unable to find unique instance ID in refresh request: " + confirmation.toString(); throw new ValidationException(Level.WARNING, () -> msg); } - // Find node matching vespa unique id - Node node = nodeRepository.nodes().list().stream() - .filter(n -> n.allocation().isPresent()) - .filter(n -> nodeMatchesVespaUniqueId(n, vespaUniqueInstanceId)) - .findFirst() // Should be only one - .orElse(null); - if(node == null) { - var msg = "Invalid InstanceConfirmation, No nodes matching uniqueId: " + vespaUniqueInstanceId; - throw new ValidationException(Level.WARNING, () -> msg); - } - // Find list of ipaddresses List<InetAddress> ips = Optional.ofNullable(confirmation.attributes.get(SAN_IPS_ATTRNAME)) .map(s -> s.split(",")) @@ -199,6 +213,20 @@ public class InstanceValidator { } } + private Node getNode(VespaUniqueInstanceId vespaUniqueInstanceId) throws ValidationException { + // Find node matching vespa unique id + Node node = nodeRepository.nodes().list().stream() + .filter(n -> n.allocation().isPresent()) + .filter(n -> nodeMatchesVespaUniqueId(n, vespaUniqueInstanceId)) + .findFirst() // Should be only one + .orElse(null); + if(node == null) { + var msg = "Invalid InstanceConfirmation, No nodes matching uniqueId: " + vespaUniqueInstanceId; + throw new ValidationException(Level.WARNING, () -> msg); + } + return node; + } + private boolean nodeMatchesVespaUniqueId(Node node, VespaUniqueInstanceId vespaUniqueInstanceId) { return node.allocation().map(allocation -> allocation.membership().index() == vespaUniqueInstanceId.clusterIndex() && diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/InstanceValidatorTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/InstanceValidatorTest.java index e7355c75d8e..42d6b92dea1 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/InstanceValidatorTest.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/InstanceValidatorTest.java @@ -9,9 +9,16 @@ import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.model.api.SuperModel; import com.yahoo.config.model.api.SuperModelProvider; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterMembership; +import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.Zone; +import com.yahoo.security.KeyAlgorithm; +import com.yahoo.security.KeyUtils; import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.athenz.identityprovider.api.ClusterType; import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; @@ -19,15 +26,19 @@ import com.yahoo.vespa.athenz.identityprovider.api.IdentityType; import com.yahoo.vespa.athenz.identityprovider.api.SignedIdentityDocument; import com.yahoo.vespa.athenz.identityprovider.api.VespaUniqueInstanceId; import com.yahoo.vespa.athenz.identityprovider.client.IdentityDocumentSigner; +import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.hosted.athenz.instanceproviderservice.InstanceValidator.ValidationException; +import com.yahoo.vespa.hosted.ca.restapi.mock.SecretStoreMock; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.node.Nodes; import com.yahoo.vespa.hosted.provision.testutils.MockNodeFlavors; +import com.yahoo.vespa.hosted.provision.testutils.MockNodeRepository; import org.junit.jupiter.api.Test; +import java.security.PrivateKey; import java.time.Instant; import java.util.ArrayList; import java.util.Collections; @@ -64,11 +75,13 @@ public class InstanceValidatorTest { private final AthenzService vespaTenantDomain = new AthenzService("vespa.vespa.tenant"); private final AutoGeneratedKeyProvider keyProvider = new AutoGeneratedKeyProvider(); + private final SecretStoreMock secretStore = new SecretStoreMock(); + private final String sisSecretName = "sis-secret-name"; @Test void application_does_not_exist() { SuperModelProvider superModelProvider = mockSuperModelProvider(); - InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null, null, vespaTenantDomain); + InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null, null, vespaTenantDomain, secretStore, sisSecretName); assertFalse(instanceValidator.isValidInstance(createRegisterInstanceConfirmation(applicationId, domain, service))); } @@ -76,7 +89,7 @@ public class InstanceValidatorTest { void application_does_not_have_domain_set() { SuperModelProvider superModelProvider = mockSuperModelProvider( mockApplicationInfo(applicationId, 5, Collections.emptyList())); - InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null, new IdentityDocumentSigner(), vespaTenantDomain); + InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null, new IdentityDocumentSigner(), vespaTenantDomain, secretStore, sisSecretName); assertFalse(instanceValidator.isValidInstance(createRegisterInstanceConfirmation(applicationId, domain, service))); } @@ -88,7 +101,7 @@ public class InstanceValidatorTest { SuperModelProvider superModelProvider = mockSuperModelProvider( mockApplicationInfo(applicationId, 5, Collections.singletonList(serviceInfo))); - InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null, null, vespaTenantDomain); + InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null, null, vespaTenantDomain, secretStore, sisSecretName); assertFalse(instanceValidator.isValidInstance(createRegisterInstanceConfirmation(applicationId, domain, service))); } @@ -106,7 +119,7 @@ public class InstanceValidatorTest { mockApplicationInfo(applicationId, 5, Collections.singletonList(serviceInfo))); IdentityDocumentSigner signer = mock(IdentityDocumentSigner.class); when(signer.hasValidSignature(any(), any())).thenReturn(true); - InstanceValidator instanceValidator = new InstanceValidator(mock(KeyProvider.class), superModelProvider, mockNodeRepo(), signer, vespaTenantDomain); + InstanceValidator instanceValidator = new InstanceValidator(mock(KeyProvider.class), superModelProvider, mockNodeRepo(), signer, vespaTenantDomain, secretStore, sisSecretName); assertTrue(instanceValidator.isValidInstance(createRegisterInstanceConfirmation(applicationId, domain, service))); } @@ -114,7 +127,7 @@ public class InstanceValidatorTest { @Test void rejects_invalid_provider_unique_id_in_csr() { SuperModelProvider superModelProvider = mockSuperModelProvider(); - InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null, null, vespaTenantDomain); + InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null, null, vespaTenantDomain, secretStore, sisSecretName); InstanceConfirmation instanceConfirmation = createRegisterInstanceConfirmation(applicationId, domain, service); VespaUniqueInstanceId tamperedId = new VespaUniqueInstanceId(0, "default", "instance", "app", "tenant", "us-north-1", "dev", IdentityType.NODE); instanceConfirmation.set("sanDNS", tamperedId.asDottedString() + ".instanceid.athenz.dev-us-north-1.vespa.yahoo.cloud"); @@ -124,7 +137,7 @@ public class InstanceValidatorTest { @Test void rejects_unknown_ips_in_csr() { NodeRepository nodeRepository = mockNodeRepo(); - InstanceValidator instanceValidator = new InstanceValidator(null, mockSuperModelProvider(), nodeRepository, null, vespaTenantDomain); + InstanceValidator instanceValidator = new InstanceValidator(null, mockSuperModelProvider(), nodeRepository, null, vespaTenantDomain, secretStore, sisSecretName); InstanceConfirmation instanceConfirmation = createRegisterInstanceConfirmation(applicationId, domain, service); Set<String> nodeIp = nodeRepository.nodes().list().owner(applicationId).stream().findFirst() .map(Node::ipConfig) @@ -142,7 +155,7 @@ public class InstanceValidatorTest { var props = Map.of(SERVICE_PROPERTIES_DOMAIN_KEY, domain, SERVICE_PROPERTIES_SERVICE_KEY, service); var info = new ServiceInfo("serviceName", "type", List.of(), props, "confId", "hostName"); var provider = mockSuperModelProvider(mockApplicationInfo(applicationId, 5, List.of(info))); - var instanceValidator = new InstanceValidator(keyProvider, provider, mockNodeRepo(), new IdentityDocumentSigner(), vespaTenantDomain); + var instanceValidator = new InstanceValidator(keyProvider, provider, mockNodeRepo(), new IdentityDocumentSigner(), vespaTenantDomain, secretStore, sisSecretName); var instanceConfirmation = createRegisterInstanceConfirmation(applicationId, domain, service); instanceConfirmation.set("sanURI", "vespa://cluster-type/content"); var exception = assertThrows(ValidationException.class, () -> instanceValidator.validateInstance(instanceConfirmation)); @@ -155,7 +168,7 @@ public class InstanceValidatorTest { NodeRepository nodeRepository = mock(NodeRepository.class); Nodes nodes = mock(Nodes.class); when(nodeRepository.nodes()).thenReturn(nodes); - InstanceValidator instanceValidator = new InstanceValidator(null, null, nodeRepository, new IdentityDocumentSigner(), vespaTenantDomain); + InstanceValidator instanceValidator = new InstanceValidator(null, null, nodeRepository, new IdentityDocumentSigner(), vespaTenantDomain, secretStore, sisSecretName); List<Node> nodeList = createNodes(10); Node node = nodeList.get(0); @@ -170,7 +183,7 @@ public class InstanceValidatorTest { @Test void rejects_refresh_on_ip_mismatch() { NodeRepository nodeRepository = mockNodeRepo(); - InstanceValidator instanceValidator = new InstanceValidator(null, null, nodeRepository, new IdentityDocumentSigner(), vespaTenantDomain); + InstanceValidator instanceValidator = new InstanceValidator(null, null, nodeRepository, new IdentityDocumentSigner(), vespaTenantDomain, secretStore, sisSecretName); Set<String> nodeIp = nodeRepository.nodes().list().owner(applicationId).stream().findFirst() .map(Node::ipConfig) @@ -191,7 +204,7 @@ public class InstanceValidatorTest { Nodes nodes = mock(Nodes.class); when(nodeRepository.nodes()).thenReturn(nodes); - InstanceValidator instanceValidator = new InstanceValidator(null, null, nodeRepository, new IdentityDocumentSigner(), vespaTenantDomain); + InstanceValidator instanceValidator = new InstanceValidator(null, null, nodeRepository, new IdentityDocumentSigner(), vespaTenantDomain, secretStore, sisSecretName); List<Node> nodeList = createNodes(10); @@ -202,11 +215,40 @@ public class InstanceValidatorTest { } + @Test + public void uses_correct_keys_in_public() throws ValidationException { + var sisKeyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA, 2048); + secretStore.setSecret(sisSecretName, KeyUtils.toPem(sisKeyPair.getPrivate())); + + var props = Map.of(SERVICE_PROPERTIES_DOMAIN_KEY, domain, SERVICE_PROPERTIES_SERVICE_KEY, service); + var info = new ServiceInfo("serviceName", "type", List.of(), props, "confId", "hostName"); + var provider = mockSuperModelProvider(mockApplicationInfo(applicationId, 5, List.of(info))); + + { + // non Enclave hosts in public should use the SIS key + var nonEnclaveHosts = createNodes(10, Optional.empty()); + var instanceValidator = new InstanceValidator(keyProvider, provider, mockNodeRepo(nonEnclaveHosts, SystemName.PublicCd), new IdentityDocumentSigner(), vespaTenantDomain, secretStore, sisSecretName); + var instanceConfirmation = createRegisterInstanceConfirmation(applicationId, domain, service, sisKeyPair.getPrivate()); + instanceValidator.validateInstance(instanceConfirmation); + } + { + // Enclave hosts should use the Vespa provider key + var enclaveHosts = createNodes(10, Optional.of(CloudAccount.from("123456789012"))); + var instanceValidator = new InstanceValidator(keyProvider, provider, mockNodeRepo(enclaveHosts, SystemName.PublicCd), new IdentityDocumentSigner(), vespaTenantDomain, secretStore, sisSecretName); + var instanceConfirmation = createRegisterInstanceConfirmation(applicationId, domain, service, keyProvider.getPrivateKey(0)); + instanceValidator.validateInstance(instanceConfirmation); + } + } + private NodeRepository mockNodeRepo() { + return mockNodeRepo(createNodes(10), SystemName.cd); + } + + private NodeRepository mockNodeRepo(List<Node> nodeList, SystemName systemName) { NodeRepository nodeRepository = mock(NodeRepository.class); Nodes nodes = mock(Nodes.class); when(nodeRepository.nodes()).thenReturn(nodes); - List<Node> nodeList = createNodes(10); + when(nodeRepository.zone()).thenReturn(new Zone(systemName, Environment.dev, RegionName.from("us-north-1"))); Node node = nodeList.get(0); nodeList = allocateNode(nodeList, node, applicationId); when(nodes.list()).thenReturn(NodeList.copyOf(nodeList)); @@ -215,6 +257,11 @@ public class InstanceValidatorTest { private InstanceConfirmation createRegisterInstanceConfirmation( ApplicationId applicationId, String domain, String service) { + return createRegisterInstanceConfirmation(applicationId, domain, service, keyProvider.getPrivateKey(0)); + } + + private InstanceConfirmation createRegisterInstanceConfirmation( + ApplicationId applicationId, String domain, String service, PrivateKey signingKey) { VespaUniqueInstanceId vespaUniqueInstanceId = new VespaUniqueInstanceId(0, "default", applicationId.instance().value(), applicationId.application().value(), applicationId.tenant().value(), "us-north-1", "dev", IdentityType.NODE); var domainService = new AthenzService(domain, service); var clock = Instant.now(); @@ -222,11 +269,12 @@ public class InstanceValidatorTest { var signature = new IdentityDocumentSigner() .generateSignature( vespaUniqueInstanceId, domainService, "localhost", "localhost", clock, Set.of(), - IdentityType.NODE, keyProvider.getPrivateKey(0)); + IdentityType.NODE, signingKey); SignedIdentityDocument signedIdentityDocument = new SignedIdentityDocument( signature, 0, vespaUniqueInstanceId, domainService, 0, "localhost", "localhost", clock, Collections.emptySet(), IdentityType.NODE, clusterType, "https://zts.url"); return createInstanceConfirmation(vespaUniqueInstanceId, domain, service, signedIdentityDocument); + } private InstanceConfirmation createRefreshInstanceConfirmation(ApplicationId applicationId, String domain, String service, List<String> ips) { @@ -275,11 +323,17 @@ public class InstanceValidatorTest { } private List<Node> createNodes(int num) { + return createNodes(num, Optional.empty()); + } + + private List<Node> createNodes(int num, Optional<CloudAccount> cloudAccount) { MockNodeFlavors flavors = new MockNodeFlavors(); List<Node> nodeList = new ArrayList<>(); for (int i = 0; i < num; i++) { - Node node = Node.create("foo" + i, new IP.Config(Set.of("::1" + i, "::2" + i, "::3" + i), Set.of()), - "foo" + i, flavors.getFlavorOrThrow("default"), NodeType.tenant).build(); + Node.Builder builder = Node.create("foo" + i, new IP.Config(Set.of("::1" + i, "::2" + i, "::3" + i), Set.of()), + "foo" + i, flavors.getFlavorOrThrow("default"), NodeType.tenant); + cloudAccount.ifPresent(builder::cloudAccount); + Node node = builder.build(); nodeList.add(node); } return nodeList; diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/mock/InstanceValidatorMock.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/mock/InstanceValidatorMock.java index 4151c1f15d7..163db3e107e 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/mock/InstanceValidatorMock.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/mock/InstanceValidatorMock.java @@ -10,7 +10,7 @@ import com.yahoo.vespa.hosted.athenz.instanceproviderservice.InstanceValidator; public class InstanceValidatorMock extends InstanceValidator { public InstanceValidatorMock() { - super(null, null, null, null, null); + super(null, null, null, null, null, null,""); } @Override |