Skip to content

Commit

Permalink
Revert "Include instance hostname in Athenz node certificates"
Browse files Browse the repository at this point in the history
This reverts commit aca45ba.
  • Loading branch information
bjorncs committed Aug 28, 2019
1 parent ec8efeb commit 8b37b6e
Show file tree
Hide file tree
Showing 12 changed files with 16 additions and 118 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;

/**
* InstanceConfirmation object as per Athenz InstanceConfirmation API.
Expand All @@ -29,8 +28,6 @@
*/
public class InstanceConfirmation {

static final String HOSTNAME_ATTRIBUTE = "hostname";

@JsonProperty("provider") public final String provider;
@JsonProperty("domain") public final String domain;
@JsonProperty("service") public final String service;
Expand All @@ -56,10 +53,6 @@ public void set(String name, String value) {
attributes.put(name, value);
}

public Optional<String> getInstanceHostname() {
return Optional.ofNullable(attributes.get(HOSTNAME_ATTRIBUTE));
}

@Override
public String toString() {
return "InstanceConfirmation{" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import com.yahoo.log.LogLevel;
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;
Expand Down Expand Up @@ -159,34 +158,6 @@ private boolean validateAttributes(InstanceConfirmation confirmation, VespaUniqu
log.log(LogLevel.WARNING, "Invalid InstanceConfirmation, wrong ip in : " + vespaUniqueInstanceId);
return false;
}

// Validate hostname
boolean hasValidHostname =
confirmation.getInstanceHostname()
.map(requestHostname -> validateHostname(vespaUniqueInstanceId, node, requestHostname))
.orElse(true);
if (!hasValidHostname) {
return false;
}

return true;
}

private static boolean validateHostname(VespaUniqueInstanceId vespaUniqueInstanceId, Node node, String requestedHostname) {
String nodeHostname = node.hostname();
if (vespaUniqueInstanceId.type() == IdentityType.TENANT) {
log.log(LogLevel.WARNING, "Instance hostname not allowed in tenant certificates");
return false;
}
if (!nodeHostname.equals(requestedHostname)) {
log.log(LogLevel.WARNING,
String.format(
"Invalid instance confirmation: request instance hostname is '%s', but node repository has '%s'",
requestedHostname,
nodeHostname));

return false;
}
return true;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.vespa.hosted.athenz.instanceproviderservice.instanceconfirmation;

import com.google.common.collect.ImmutableList;
import com.yahoo.component.Version;
import com.yahoo.config.model.api.ApplicationInfo;
import com.yahoo.config.model.api.HostInfo;
Expand Down Expand Up @@ -122,7 +123,7 @@ public void accepts_valid_refresh_requests() {
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(applicationId, domain, service, IdentityType.NODE, node.hostname(), List.of(nodeIp));
InstanceConfirmation instanceConfirmation = createRefreshInstanceConfirmation(applicationId, domain, service, ImmutableList.of(nodeIp));

assertTrue(instanceValidator.isValidRefresh(instanceConfirmation));
}
Expand All @@ -139,41 +140,7 @@ public void rejects_refresh_on_ip_mismatch() {
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(applicationId, domain, service, IdentityType.NODE, node.hostname(), List.of(nodeIp, "::ff"));

assertFalse(instanceValidator.isValidRefresh(instanceConfirmation));
}

@Test
public void rejects_invalid_hostname() {
NodeRepository nodeRepository = mock(NodeRepository.class);
InstanceValidator instanceValidator = new InstanceValidator(null, null, nodeRepository);

List<Node> nodeList = createNodes(10);
Node node = nodeList.get(0);
nodeList = allocateNode(nodeList, node, applicationId);
when(nodeRepository.getNodes()).thenReturn(nodeList);
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(applicationId, domain, service, IdentityType.NODE, "invalidhostname", List.of(nodeIp));

assertFalse(instanceValidator.isValidRefresh(instanceConfirmation));
}

@Test
public void rejects_hostname_for_tenant_certificates() {
NodeRepository nodeRepository = mock(NodeRepository.class);
InstanceValidator instanceValidator = new InstanceValidator(null, null, nodeRepository);

List<Node> nodeList = createNodes(10);
Node node = nodeList.get(0);
nodeList = allocateNode(nodeList, node, applicationId);
when(nodeRepository.getNodes()).thenReturn(nodeList);
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(applicationId, domain, service, IdentityType.TENANT, node.hostname(), List.of(nodeIp));
InstanceConfirmation instanceConfirmation = createRefreshInstanceConfirmation(applicationId, domain, service, ImmutableList.of(nodeIp, "::ff"));

assertFalse(instanceValidator.isValidRefresh(instanceConfirmation));
}
Expand All @@ -185,7 +152,7 @@ public void rejects_refresh_when_node_is_not_allocated() {

List<Node> nodeList = createNodes(10);
when(nodeRepository.getNodes()).thenReturn(nodeList);
InstanceConfirmation instanceConfirmation = createRefreshInstanceConfirmation(applicationId, domain, service, IdentityType.NODE, nodeList.get(0).hostname(), List.of("::11"));
InstanceConfirmation instanceConfirmation = createRefreshInstanceConfirmation(applicationId, domain, service, ImmutableList.of("::11"));

assertFalse(instanceValidator.isValidRefresh(instanceConfirmation));

Expand All @@ -206,11 +173,10 @@ private InstanceConfirmation createRegisterInstanceConfirmation(ApplicationId ap
return createInstanceConfirmation(vespaUniqueInstanceId, domain, service, signedIdentityDocument);
}

private InstanceConfirmation createRefreshInstanceConfirmation(ApplicationId applicationId, String domain, String service, IdentityType identityType, String hostname, List<String> ips) {
VespaUniqueInstanceId vespaUniqueInstanceId = new VespaUniqueInstanceId(0, "default", applicationId.instance().value(), applicationId.application().value(), applicationId.tenant().value(), "us-north-1", "dev", identityType);
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));
instanceConfirmation.set(InstanceConfirmation.HOSTNAME_ATTRIBUTE, hostname);
return instanceConfirmation;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ public List<AthenzDomain> getTenantDomains(AthenzIdentity providerIdentity, Athe
}

@Override
public InstanceIdentity registerInstance(AthenzIdentity providerIdentity, AthenzIdentity instanceIdentity, String hostname, String attestationData, Pkcs10Csr csr) {
public InstanceIdentity registerInstance(AthenzIdentity providerIdentity, AthenzIdentity instanceIdentity, String attestationData, Pkcs10Csr csr) {
throw new UnsupportedOperationException();
}

@Override
public InstanceIdentity refreshInstance(AthenzIdentity providerIdentity, AthenzIdentity instanceIdentity, String instanceId, String hostname, Pkcs10Csr csr) {
public InstanceIdentity refreshInstance(AthenzIdentity providerIdentity, AthenzIdentity instanceIdentity, String instanceId, Pkcs10Csr csr) {
throw new UnsupportedOperationException();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,19 +155,13 @@ private boolean shouldThrottleRefreshAttempts(ContainerName containerName, Insta
private void registerIdentity(NodeAgentContext context, Path privateKeyFile, Path certificateFile, Path identityDocumentFile) {
KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA);
SignedIdentityDocument signedIdentityDocument = identityDocumentClient.getNodeIdentityDocument(context.hostname().value());
Pkcs10Csr csr =
csrGenerator.generateInstanceCsr(
context.identity(),
signedIdentityDocument.providerUniqueId(),
signedIdentityDocument.instanceHostname(),
signedIdentityDocument.ipAddresses(),
keyPair);
Pkcs10Csr csr = csrGenerator.generateInstanceCsr(
context.identity(), signedIdentityDocument.providerUniqueId(), signedIdentityDocument.ipAddresses(), keyPair);
try (ZtsClient ztsClient = new DefaultZtsClient(ztsEndpoint, hostIdentityProvider)) {
InstanceIdentity instanceIdentity =
ztsClient.registerInstance(
configserverIdentity,
context.identity(),
signedIdentityDocument.instanceHostname(),
EntityBindingsMapper.toAttestationData(signedIdentityDocument),
csr);
EntityBindingsMapper.writeSignedIdentityDocumentToFile(identityDocumentFile, signedIdentityDocument);
Expand All @@ -180,13 +174,8 @@ private void registerIdentity(NodeAgentContext context, Path privateKeyFile, Pat
private void refreshIdentity(NodeAgentContext context, Path privateKeyFile, Path certificateFile, Path identityDocumentFile) {
SignedIdentityDocument identityDocument = EntityBindingsMapper.readSignedIdentityDocumentFromFile(identityDocumentFile);
KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA);
Pkcs10Csr csr = csrGenerator
.generateInstanceCsr(
context.identity(),
identityDocument.providerUniqueId(),
identityDocument.instanceHostname(),
identityDocument.ipAddresses(),
keyPair);
Pkcs10Csr csr = csrGenerator.generateInstanceCsr(
context.identity(), identityDocument.providerUniqueId(), identityDocument.ipAddresses(), keyPair);
SSLContext containerIdentitySslContext =
new SslContextBuilder()
.withKeyStore(privateKeyFile, certificateFile)
Expand All @@ -199,7 +188,6 @@ private void refreshIdentity(NodeAgentContext context, Path privateKeyFile, Path
configserverIdentity,
context.identity(),
identityDocument.providerUniqueId().asDottedString(),
identityDocument.instanceHostname(),
csr);
writePrivateKeyAndCertificate(context.vespaUserOnHost(), privateKeyFile, keyPair.getPrivate(),
certificateFile, instanceIdentity.certificate());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,9 @@ private DefaultZtsClient(URI ztsUrl, AthenzIdentity identity, Supplier<SSLContex
public InstanceIdentity registerInstance(AthenzIdentity providerIdentity,
AthenzIdentity instanceIdentity,
String attestationData,
String hostname,
Pkcs10Csr csr) {
InstanceRegisterInformation payload =
new InstanceRegisterInformation(providerIdentity, instanceIdentity, attestationData, hostname, csr);
new InstanceRegisterInformation(providerIdentity, instanceIdentity, attestationData, csr);
HttpUriRequest request = RequestBuilder.post()
.setUri(ztsUrl.resolve("instance/"))
.setEntity(toJsonStringEntity(payload))
Expand All @@ -82,9 +81,8 @@ public InstanceIdentity registerInstance(AthenzIdentity providerIdentity,
public InstanceIdentity refreshInstance(AthenzIdentity providerIdentity,
AthenzIdentity instanceIdentity,
String instanceId,
String hostname,
Pkcs10Csr csr) {
InstanceRefreshInformation payload = new InstanceRefreshInformation(csr, hostname);
InstanceRefreshInformation payload = new InstanceRefreshInformation(csr);
URI uri = ztsUrl.resolve(
String.format("instance/%s/%s/%s/%s",
providerIdentity.getFullName(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ public interface ZtsClient extends AutoCloseable {
*/
InstanceIdentity registerInstance(AthenzIdentity providerIdentity,
AthenzIdentity instanceIdentity,
String hostname,
String attestationData,
Pkcs10Csr csr);

Expand All @@ -41,7 +40,6 @@ InstanceIdentity registerInstance(AthenzIdentity providerIdentity,
InstanceIdentity refreshInstance(AthenzIdentity providerIdentity,
AthenzIdentity instanceIdentity,
String instanceId,
String hostname,
Pkcs10Csr csr);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,8 @@ public class InstanceRefreshInformation {
@JsonProperty("csr")
@JsonSerialize(using = Pkcs10CsrSerializer.class)
private final Pkcs10Csr csr;
@JsonProperty("hostname")
private final String hostname;

public InstanceRefreshInformation(Pkcs10Csr csr, String hostname) {
public InstanceRefreshInformation(Pkcs10Csr csr) {
this.csr = csr;
this.hostname = hostname;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,17 @@ public class InstanceRegisterInformation {
private final String service;
@JsonProperty("attestationData")
private final String attestationData;
@JsonProperty("hostname")
private final String hostname;
@JsonProperty("csr")
private final String csr;

public InstanceRegisterInformation(AthenzIdentity providerIdentity,
AthenzIdentity instanceIdentity,
String attestationData,
String hostname,
Pkcs10Csr csr) {
this.provider = providerIdentity.getFullName();
this.domain = instanceIdentity.getDomain().getName();
this.service = instanceIdentity.getName();
this.attestationData = attestationData;
this.csr = Pkcs10CsrUtils.toPem(csr);
this.hostname = hostname;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ AthenzCredentials registerInstance() {
Pkcs10Csr csr = csrGenerator.generateInstanceCsr(
tenantIdentity,
document.providerUniqueId(),
/*hostname*/null, // no hostname in tenant certificates
document.ipAddresses(),
keyPair);

Expand All @@ -84,7 +83,6 @@ AthenzCredentials registerInstance() {
ztsClient.registerInstance(
configserverIdentity,
tenantIdentity,
/*hostname*/null,
EntityBindingsMapper.toAttestationData(document),
csr);
X509Certificate certificate = instanceIdentity.certificate();
Expand All @@ -98,7 +96,6 @@ AthenzCredentials updateCredentials(SignedIdentityDocument document, SSLContext
Pkcs10Csr csr = csrGenerator.generateInstanceCsr(
tenantIdentity,
document.providerUniqueId(),
/*hostname*/null, // no hostname in tenant certificates
document.ipAddresses(),
newKeyPair);

Expand All @@ -107,7 +104,6 @@ AthenzCredentials updateCredentials(SignedIdentityDocument document, SSLContext
ztsClient.refreshInstance(
configserverIdentity,
tenantIdentity,
/*hostname*/null,
document.providerUniqueId().asDottedString(),
csr);
X509Certificate certificate = instanceIdentity.certificate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,11 @@ public CsrGenerator(String dnsSuffix, String providerService) {

public Pkcs10Csr generateInstanceCsr(AthenzIdentity instanceIdentity,
VespaUniqueInstanceId instanceId,
String hostname,
Set<String> ipAddresses,
KeyPair keyPair) {
X500Principal subject = new X500Principal(String.format("OU=%s, CN=%s", providerService, instanceIdentity.getFullName()));
// Add SAN dnsname <service>.<domain-with-dashes>.<provider-dnsname-suffix>
// and SAN dnsname <provider-unique-instance-id>.instanceid.athenz.<provider-dnsname-suffix>
// and SAN dnsname <hostname> (note: ZTS will verify that there is a DNS A record with hostname having the remote ip)
Pkcs10CsrBuilder pkcs10CsrBuilder = Pkcs10CsrBuilder.fromKeypair(subject, keyPair, SHA256_WITH_RSA)
.addSubjectAlternativeName(
DNS_NAME,
Expand All @@ -50,9 +48,6 @@ public Pkcs10Csr generateInstanceCsr(AthenzIdentity instanceIdentity,
instanceIdentity.getDomainName().replace(".", "-"),
dnsSuffix))
.addSubjectAlternativeName(DNS_NAME, getIdentitySAN(instanceId));
if (hostname != null) {
pkcs10CsrBuilder.addSubjectAlternativeName(DNS_NAME, hostname);
}
ipAddresses.forEach(ip -> pkcs10CsrBuilder.addSubjectAlternativeName(new SubjectAlternativeName(IP_ADDRESS, ip)));
return pkcs10CsrBuilder.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public void it_generates_csr_with_correct_subject() {
VespaUniqueInstanceId vespaUniqueInstanceId = VespaUniqueInstanceId.fromDottedString("0.default.default.foo-app.vespa.us-north-1.prod.node");
KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA);

Pkcs10Csr csr = csrGenerator.generateInstanceCsr(service, vespaUniqueInstanceId, "myhostname", Collections.emptySet(), keyPair);
Pkcs10Csr csr = csrGenerator.generateInstanceCsr(service, vespaUniqueInstanceId, Collections.emptySet(), keyPair);
assertEquals(new X500Principal(String.format("OU=%s, CN=%s", PROVIDER_SERVICE, ATHENZ_SERVICE)), csr.getSubject());
}
}

0 comments on commit 8b37b6e

Please sign in to comment.