diff options
author | Morten Tokle <mortent@oath.com> | 2018-08-21 15:19:51 +0200 |
---|---|---|
committer | Morten Tokle <mortent@oath.com> | 2018-08-21 15:19:51 +0200 |
commit | c5431df535cf7a95d44d61652e3a1e151585f14c (patch) | |
tree | cdccad834e041d44821ec06507a1d8cd9aa1be47 /athenz-identity-provider-service | |
parent | f7015e9c2d4614797f20672da2ac89f31f8ed37a (diff) |
Validate provider unique id in register
Diffstat (limited to 'athenz-identity-provider-service')
2 files changed, 87 insertions, 26 deletions
diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java index 3d575bddcf8..e5ab125227d 100644 --- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java +++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidator.java @@ -30,6 +30,7 @@ import java.util.stream.Stream; * Verifies that the instance's identity document is valid * * @author bjorncs + * @author mortent */ public class InstanceValidator { @@ -39,7 +40,7 @@ public class InstanceValidator { static final String SERVICE_PROPERTIES_SERVICE_KEY = "identity.service"; static final String INSTANCE_ID_DELIMITER = ".instanceid.athenz."; - private final IdentityDocumentSigner signer = new IdentityDocumentSigner(); + private final IdentityDocumentSigner signer; private final KeyProvider keyProvider; private final SuperModelProvider superModelProvider; private final NodeRepository nodeRepository; @@ -48,9 +49,17 @@ public class InstanceValidator { public InstanceValidator(KeyProvider keyProvider, SuperModelProvider superModelProvider, NodeRepository nodeRepository) { + this(keyProvider, superModelProvider, nodeRepository, new IdentityDocumentSigner()); + } + + public InstanceValidator(KeyProvider keyProvider, + SuperModelProvider superModelProvider, + NodeRepository nodeRepository, + IdentityDocumentSigner identityDocumentSigner){ this.keyProvider = keyProvider; this.superModelProvider = superModelProvider; this.nodeRepository = nodeRepository; + this.signer = identityDocumentSigner; } public boolean isValidInstance(InstanceConfirmation instanceConfirmation) { @@ -59,6 +68,12 @@ public class InstanceValidator { ApplicationId applicationId = ApplicationId.from( providerUniqueId.tenant(), providerUniqueId.application(), providerUniqueId.instance()); + VespaUniqueInstanceId csrProviderUniqueId = getVespaUniqueInstanceId(instanceConfirmation); + if(! providerUniqueId.equals(csrProviderUniqueId)) { + log.log(LogLevel.WARNING, String.format("Instance %s has invalid provider unique ID in CSR (%s)", providerUniqueId, csrProviderUniqueId)); + return false; + } + if (! isSameIdentityAsInServicesXml(applicationId, instanceConfirmation.domain, instanceConfirmation.service)) { return false; } @@ -83,28 +98,31 @@ public class InstanceValidator { confirmation.provider, confirmation.attributes.get("sanDNS"))); try { - return validateAttributes(confirmation); + return validateAttributes(confirmation, getVespaUniqueInstanceId(confirmation)); } catch (Exception e) { - log.log(LogLevel.INFO, "Encountered exception while refreshing certificate for confirmation: " + confirmation, e); - return true; + log.log(LogLevel.WARNING, "Encountered exception while refreshing certificate for confirmation: " + confirmation, e); + return false; } } - private boolean validateAttributes(InstanceConfirmation confirmation) { + private VespaUniqueInstanceId getVespaUniqueInstanceId(InstanceConfirmation instanceConfirmation) { // Find a list of SAN DNS - List<String> sanDNS = Optional.ofNullable(confirmation.attributes.get("sanDNS")) + List<String> sanDNS = Optional.ofNullable(instanceConfirmation.attributes.get("sanDNS")) .map(s -> s.split(",")) .map(Arrays::asList) .map(List::stream) .orElse(Stream.empty()) .collect(Collectors.toList()); - VespaUniqueInstanceId vespaUniqueInstanceId = sanDNS.stream() + return sanDNS.stream() .filter(dns -> dns.contains(INSTANCE_ID_DELIMITER)) .findFirst() .map(s -> s.replaceAll(INSTANCE_ID_DELIMITER + ".*", "")) .map(VespaUniqueInstanceId::fromDottedString) .orElse(null); + } + + private boolean validateAttributes(InstanceConfirmation confirmation, VespaUniqueInstanceId vespaUniqueInstanceId) { if(vespaUniqueInstanceId == null) { log.log(LogLevel.WARNING, "Unabe to find unique instance ID in refresh request: " + confirmation.toString()); return false; @@ -135,7 +153,12 @@ public class InstanceValidator { .collect(Collectors.toList()); // Validate that ipaddresses in request are valid for node - return nodeIpAddresses.containsAll(ips); + + if(! nodeIpAddresses.containsAll(ips)) { + log.log(LogLevel.WARNING, "Invalid InstanceConfirmation, wrong ip in : " + vespaUniqueInstanceId); + return false; + } + return true; } private boolean nodeMatchesVespaUniqueId(Node node, VespaUniqueInstanceId vespaUniqueInstanceId) { diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java index 8beb8bda99f..140a7f71c16 100644 --- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java +++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/instanceconfirmation/InstanceValidatorTest.java @@ -13,8 +13,13 @@ import com.yahoo.config.model.api.SuperModelProvider; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.NodeType; +import com.yahoo.vespa.athenz.api.AthenzService; +import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; 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.hosted.athenz.instanceproviderservice.KeyProvider; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.testutils.MockNodeFlavors; @@ -36,6 +41,7 @@ import static com.yahoo.vespa.hosted.athenz.instanceproviderservice.instanceconf import static com.yahoo.vespa.hosted.athenz.instanceproviderservice.instanceconfirmation.InstanceValidator.SERVICE_PROPERTIES_SERVICE_KEY; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -53,9 +59,8 @@ public class InstanceValidatorTest { @Test public void application_does_not_exist() { SuperModelProvider superModelProvider = mockSuperModelProvider(); - InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null); - - assertFalse(instanceValidator.isSameIdentityAsInServicesXml(applicationId, domain, service)); + InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null, null); + assertFalse(instanceValidator.isValidInstance(createRegisterInstanceConfirmation(applicationId, domain, service))); } @Test @@ -64,7 +69,7 @@ public class InstanceValidatorTest { mockApplicationInfo(applicationId, 5, Collections.emptyList())); InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null); - assertFalse(instanceValidator.isSameIdentityAsInServicesXml(applicationId, domain, service)); + assertFalse(instanceValidator.isValidInstance(createRegisterInstanceConfirmation(applicationId, domain, service))); } @Test @@ -74,9 +79,9 @@ public class InstanceValidatorTest { SuperModelProvider superModelProvider = mockSuperModelProvider( mockApplicationInfo(applicationId, 5, Collections.singletonList(serviceInfo))); - InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null); + InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null, null); - assertFalse(instanceValidator.isSameIdentityAsInServicesXml(applicationId, domain, service)); + assertFalse(instanceValidator.isValidInstance(createRegisterInstanceConfirmation(applicationId, domain, service))); } @Test @@ -90,9 +95,21 @@ public class InstanceValidatorTest { SuperModelProvider superModelProvider = mockSuperModelProvider( mockApplicationInfo(applicationId, 5, Collections.singletonList(serviceInfo))); - InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null); + IdentityDocumentSigner signer = mock(IdentityDocumentSigner.class); + when(signer.hasValidSignature(any(), any())).thenReturn(true); + InstanceValidator instanceValidator = new InstanceValidator(mock(KeyProvider.class), superModelProvider, null, signer); + + assertTrue(instanceValidator.isValidInstance(createRegisterInstanceConfirmation(applicationId, domain, service))); + } - assertTrue(instanceValidator.isSameIdentityAsInServicesXml(applicationId, domain, service)); + @Test + public void rejects_invalid_provider_unique_id_in_csr() { + SuperModelProvider superModelProvider = mockSuperModelProvider(); + InstanceValidator instanceValidator = new InstanceValidator(null, superModelProvider, null, null); + 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"); + assertFalse(instanceValidator.isValidInstance(instanceConfirmation)); } @Test @@ -105,7 +122,7 @@ public class InstanceValidatorTest { nodeList = allocateNode(nodeList, node, applicationId); when(nodeRepository.getNodes()).thenReturn(nodeList); String nodeIp = node.ipAddresses().stream().findAny().orElseThrow(() -> new RuntimeException("No ipaddress for mocked node")); - InstanceConfirmation instanceConfirmation = createRefreshInstanceConfirmation(ImmutableList.of(nodeIp), applicationId); + InstanceConfirmation instanceConfirmation = createRefreshInstanceConfirmation(applicationId, domain, service, ImmutableList.of(nodeIp)); assertTrue(instanceValidator.isValidRefresh(instanceConfirmation)); } @@ -122,7 +139,7 @@ public class InstanceValidatorTest { String nodeIp = node.ipAddresses().stream().findAny().orElseThrow(() -> new RuntimeException("No ipaddress for mocked node")); // Add invalid ip to list of ip addresses - InstanceConfirmation instanceConfirmation = createRefreshInstanceConfirmation(ImmutableList.of(nodeIp, "::ff"), applicationId); + InstanceConfirmation instanceConfirmation = createRefreshInstanceConfirmation(applicationId, domain, service, ImmutableList.of(nodeIp, "::ff")); assertFalse(instanceValidator.isValidRefresh(instanceConfirmation)); } @@ -134,21 +151,42 @@ public class InstanceValidatorTest { List<Node> nodeList = createNodes(10); when(nodeRepository.getNodes()).thenReturn(nodeList); - InstanceConfirmation instanceConfirmation = createRefreshInstanceConfirmation(ImmutableList.of("::11"), applicationId); + InstanceConfirmation instanceConfirmation = createRefreshInstanceConfirmation(applicationId, domain, service, ImmutableList.of("::11")); assertFalse(instanceValidator.isValidRefresh(instanceConfirmation)); } - private InstanceConfirmation createRefreshInstanceConfirmation(List<String> ips, ApplicationId applicationId) { - InstanceConfirmation instanceConfirmation = new InstanceConfirmation( - "vespa.vespa.cd.provider_dev_us-north-1", - "vespa.vespa.cd", - "tenant", - null); + private InstanceConfirmation createRegisterInstanceConfirmation(ApplicationId applicationId, String domain, String service) { + VespaUniqueInstanceId vespaUniqueInstanceId = new VespaUniqueInstanceId(0, "default", applicationId.instance().value(), applicationId.application().value(), applicationId.tenant().value(), "us-north-1", "dev", IdentityType.NODE); + SignedIdentityDocument signedIdentityDocument = new SignedIdentityDocument(null, + 0, + vespaUniqueInstanceId, + new AthenzService(domain, service), + 0, + "localhost", + "localhost", + Instant.now(), + Collections.emptySet(), + IdentityType.NODE); + return createInstanceConfirmation(vespaUniqueInstanceId, domain, service, signedIdentityDocument); + } - instanceConfirmation.set("sanIP", String.join(",", ips)); + private InstanceConfirmation createRefreshInstanceConfirmation(ApplicationId applicationId, String domain, String service, List<String> ips) { VespaUniqueInstanceId vespaUniqueInstanceId = new VespaUniqueInstanceId(0, "default", applicationId.instance().value(), applicationId.application().value(), applicationId.tenant().value(), "us-north-1", "dev", IdentityType.NODE); + InstanceConfirmation instanceConfirmation = createInstanceConfirmation(vespaUniqueInstanceId, domain, service, null); + instanceConfirmation.set("sanIP", String.join(",", ips)); + return instanceConfirmation; + } + + private InstanceConfirmation createInstanceConfirmation(VespaUniqueInstanceId vespaUniqueInstanceId, String domain, String service, SignedIdentityDocument identityDocument) { + InstanceConfirmation instanceConfirmation = new InstanceConfirmation( + "vespa.vespa.cd.provider_dev_us-north-1", + domain, + service, + Optional.ofNullable(identityDocument) + .map(EntityBindingsMapper::toSignedIdentityDocumentEntity) + .orElse(null)); instanceConfirmation.set("sanDNS", vespaUniqueInstanceId.asDottedString() + ".instanceid.athenz.dev-us-north-1.vespa.yahoo.cloud"); return instanceConfirmation; } |