Validate provider unique id in register

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;
     }