diff options
3 files changed, 81 insertions, 36 deletions
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java index 93805f5f7b6..95ebbb7362b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java @@ -1,55 +1,100 @@ -package com.yahoo.vespa.hosted.controller.restapi.application; +package com.yahoo.vespa.hosted.controller.restapi.filter; +import ai.vespa.hosted.api.Method; +import ai.vespa.hosted.api.RequestSigner; +import com.yahoo.application.container.handler.Request; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.TenantName; +import com.yahoo.jdisc.http.filter.DiscFilterRequest; +import com.yahoo.vespa.hosted.controller.ApplicationController; +import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.role.Role; -import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; -import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerCloudTest; -import com.yahoo.vespa.hosted.controller.security.CloudTenantSpec; -import com.yahoo.vespa.hosted.controller.security.Credentials; +import com.yahoo.vespa.hosted.controller.api.role.SecurityContext; +import com.yahoo.vespa.hosted.controller.restapi.ApplicationRequestToDiscFilterRequestWrapper; +import org.junit.Before; import org.junit.Test; -import java.io.File; -import java.util.Optional; +import java.net.URI; +import java.net.http.HttpRequest; import java.util.Set; -import static com.yahoo.application.container.handler.Request.Method.PATCH; -import static com.yahoo.application.container.handler.Request.Method.POST; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; -public class CloudApplicationApiTest extends ControllerContainerCloudTest { - - private static final String responseFiles = "src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/"; +public class SignatureFilterTest { private static final String publicKey = "-----BEGIN PUBLIC KEY-----\n" + "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEuKVFA8dXk43kVfYKzkUqhEY2rDT9\n" + "z/4jKSTHwbYR8wdsOSrJGVEUPbS2nguIJ64OJH7gFnxM6sxUVj+Nm2HlXw==\n" + "-----END PUBLIC KEY-----\n"; + private static final String otherPublicKey = "-----BEGIN PUBLIC KEY-----\n" + + "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEFELzPyinTfQ/sZnTmRp5E4Ve/sbE\n" + + "pDhJeqczkyFcT2PysJ5sZwm7rKPEeXDOhzTPCyRvbUqc2SGdWbKUGGa/Yw==\n" + + "-----END PUBLIC KEY-----\n"; + private static final String privateKey = "-----BEGIN EC PRIVATE KEY-----\n" + "MHcCAQEEIJUmbIX8YFLHtpRgkwqDDE3igU9RG6JD9cYHWAZii9j7oAoGCCqGSM49\n" + "AwEHoUQDQgAEuKVFA8dXk43kVfYKzkUqhEY2rDT9z/4jKSTHwbYR8wdsOSrJGVEU\n" + "PbS2nguIJ64OJH7gFnxM6sxUVj+Nm2HlXw==\n" + "-----END EC PRIVATE KEY-----\n"; + private static final ApplicationId id = ApplicationId.from("my-tenant", "my-app", "default"); + + private ControllerTester tester; + private ApplicationController applications; + private SignatureFilter filter; + private RequestSigner signer; + + @Before + public void setup() { + tester = new ControllerTester(); + applications = tester.controller().applications(); + filter = new SignatureFilter(tester.controller()); + signer = new RequestSigner(privateKey, id.serializedForm(), tester.clock()); + + tester.createApplication(tester.createTenant(id.tenant().value(), "unused", 496L), + id.application().value(), + id.instance().value(), + 28L); + } + @Test - public void testResponses() { - ContainerTester tester = new ContainerTester(container, responseFiles); - ApplicationId id = ApplicationId.from("my-tenant", "my-app", "default"); - Optional<Credentials> credentials = Optional.of(new Credentials(() -> "user")); - - // Create an application. - tester.controller().tenants().create(new CloudTenantSpec(TenantName.from("my-tenant"), "token"), credentials.get()); - tester.controller().applications().createApplication(id, credentials); - - // PATCH in a pem deploy key. - tester.assertResponse(request("/application/v4/tenant/my-tenant/application/my-app", PATCH) - .roles(Set.of(Role.hostedOperator())) - .data("{\"pemDeployKey\":\"" + publicKey + "\"}"), - "{\"message\":\"Set pem deploy key to " + - publicKey.replaceAll("\\n", "\\\\n") + "\"}"); - - tester.assertResponse(request("/application/v4/tenant/my-tenant/application/my-application/instance/default/submit", POST) - .roles()); + public void testFilter() { + // Unsigned request is rejected. + HttpRequest.Builder request = HttpRequest.newBuilder(URI.create("https://host:123/path/./..//..%2F?query=empty&%3F=%26")); + byte[] emptyBody = new byte[0]; + DiscFilterRequest unsigned = requestOf(request.method("GET", HttpRequest.BodyPublishers.ofByteArray(emptyBody)).build(), emptyBody); + assertFalse(filter.filter(unsigned).isEmpty()); + + // Signed request is rejected when no key is stored for the application. + DiscFilterRequest signed = requestOf(signer.signed(request, Method.GET), emptyBody); + assertFalse(filter.filter(signed).isEmpty()); + + // Signed request is rejected when a non-matching key is stored for the application. + applications.lockOrThrow(id, application -> applications.store(application.withPemDeployKey(otherPublicKey))); + assertFalse(filter.filter(signed).isEmpty()); + + // Signed request is accepted when a matching key is stored for the application. + applications.lockOrThrow(id, application -> applications.store(application.withPemDeployKey(publicKey))); + assertTrue(filter.filter(signed).isEmpty()); + SecurityContext securityContext = (SecurityContext) signed.getAttribute(SecurityContext.ATTRIBUTE_NAME); + assertEquals("buildService@my-tenant.my-app", securityContext.principal().getName()); + assertEquals(Set.of(Role.buildService(id.tenant(), id.application())), securityContext.roles()); + + // Signed POST request is also accepted. + byte[] hiBytes = new byte[]{0x48, 0x69}; + signed = requestOf(signer.signed(request, Method.POST), hiBytes); + assertTrue(filter.filter(signed).isEmpty()); + + // Unsigned requests are still rejected. + assertFalse(filter.filter(unsigned).isEmpty()); + } + + private static DiscFilterRequest requestOf(HttpRequest request, byte[] body) { + Request converted = new Request(request.uri().toString(), body, Request.Method.valueOf(request.method())); + converted.getHeaders().addAll(request.headers().map()); + return new ApplicationRequestToDiscFilterRequestWrapper(converted); } } diff --git a/hosted-api/src/test/java/ai/vespa/hosted/api/SignaturesTest.java b/hosted-api/src/test/java/ai/vespa/hosted/api/SignaturesTest.java index 23d29dbeb55..0ac46a51a43 100644 --- a/hosted-api/src/test/java/ai/vespa/hosted/api/SignaturesTest.java +++ b/hosted-api/src/test/java/ai/vespa/hosted/api/SignaturesTest.java @@ -32,7 +32,7 @@ import static org.junit.Assert.assertTrue; */ public class SignaturesTest { - private static final String emPemPublicKey = "-----BEGIN PUBLIC KEY-----\n" + + private static final String ecPemPublicKey = "-----BEGIN PUBLIC KEY-----\n" + "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEuKVFA8dXk43kVfYKzkUqhEY2rDT9\n" + "z/4jKSTHwbYR8wdsOSrJGVEUPbS2nguIJ64OJH7gFnxM6sxUVj+Nm2HlXw==\n" + "-----END PUBLIC KEY-----\n"; @@ -77,7 +77,7 @@ public class SignaturesTest { HttpRequest request = signer.signed(builder, Method.GET); // GET request with correct signature and URI as-is. - RequestVerifier verifier = new RequestVerifier(emPemPublicKey, clock); + RequestVerifier verifier = new RequestVerifier(ecPemPublicKey, clock); assertTrue(verifier.verify(Method.valueOf(request.method()), request.uri(), request.headers().firstValue("X-Timestamp").get(), @@ -138,7 +138,7 @@ public class SignaturesTest { request.headers().firstValue("X-Authorization").get())); // Too old request. - verifier = new RequestVerifier(emPemPublicKey, Clock.fixed(Instant.EPOCH.plusSeconds(301), ZoneOffset.UTC)); + verifier = new RequestVerifier(ecPemPublicKey, Clock.fixed(Instant.EPOCH.plusSeconds(301), ZoneOffset.UTC)); assertFalse(verifier.verify(Method.valueOf(request.method()), request.uri().normalize(), request.headers().firstValue("X-Timestamp").get(), @@ -146,7 +146,7 @@ public class SignaturesTest { request.headers().firstValue("X-Authorization").get())); // Too new request. - verifier = new RequestVerifier(emPemPublicKey, Clock.fixed(Instant.EPOCH.minusSeconds(301), ZoneOffset.UTC)); + verifier = new RequestVerifier(ecPemPublicKey, Clock.fixed(Instant.EPOCH.minusSeconds(301), ZoneOffset.UTC)); assertFalse(verifier.verify(Method.valueOf(request.method()), request.uri().normalize(), request.headers().firstValue("X-Timestamp").get(), diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzPrincipalFilter.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzPrincipalFilter.java index a30ca654b83..e51fb1a5c9a 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzPrincipalFilter.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzPrincipalFilter.java @@ -61,7 +61,7 @@ public class AthenzPrincipalFilter extends JsonSecurityRequestFilterBase { Optional<AthenzPrincipal> nTokenPrincipal = getPrincipalToken(request, principalTokenHeader) .map(validator::validate); - if (!certificatePrincipal.isPresent() && !nTokenPrincipal.isPresent()) { + if (certificatePrincipal.isEmpty() && nTokenPrincipal.isEmpty()) { String errorMessage = "Unable to authenticate Athenz identity. " + "Either client certificate or principal token is required."; return createResponse(request, Response.Status.UNAUTHORIZED, errorMessage); |