Merge pull request #5342 from vespa-engine/mpolden/enforce-authorization

Enforce config server API authorization in CD

3 files changed, 39 insertions, 14 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/Authorizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/Authorizer.java
index e69d551c86a..9559b2e59e4 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/Authorizer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/Authorizer.java
@@ -56,7 +56,7 @@ public class Authorizer implements BiPredicate<Principal, URI> {
         }
 
         // Nodes of a specific type can access whitelisted resources
-        if (canAccess(nodeTypesFrom(uri), principal, this::isNodeType)) {
+        if (canAccess(nodeTypesFor(uri), principal, this::isNodeType)) {
             return true;
         }
 
@@ -151,7 +151,7 @@ public class Authorizer implements BiPredicate<Principal, URI> {
     }
 
     /** Returns node types which can access given URI */
-    private static List<NodeType> nodeTypesFrom(URI uri) {
+    private static List<NodeType> nodeTypesFor(URI uri) {
         if (isChildOf("/routing/v1/", uri.getPath())) {
             return Collections.singletonList(NodeType.proxy);
         }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java
index 41f6d0ef4bc..d4435e84de9 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.provision.restapi.v2.filter;
 
 import com.google.inject.Inject;
+import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.jdisc.handler.ResponseHandler;
 import com.yahoo.jdisc.http.filter.DiscFilterRequest;
@@ -34,17 +35,17 @@ public class AuthorizationFilter implements SecurityRequestFilter {
     private static final Logger log = Logger.getLogger(AuthorizationFilter.class.getName());
 
     private final BiPredicate<Principal, URI> authorizer;
-    private final BiConsumer<ErrorResponse, ResponseHandler> responseWriter;
+    private final BiConsumer<ErrorResponse, ResponseHandler> rejectAction;
 
     @Inject
     public AuthorizationFilter(Zone zone, NodeRepository nodeRepository) {
-         this(new Authorizer(zone.system(), nodeRepository), AuthorizationFilter::log); // TODO: Use write method once all clients are using certificates
+        this(new Authorizer(zone.system(), nodeRepository), rejectActionIn(zone.system()));
     }
 
     AuthorizationFilter(BiPredicate<Principal, URI> authorizer,
-                        BiConsumer<ErrorResponse, ResponseHandler> responseWriter) {
+                        BiConsumer<ErrorResponse, ResponseHandler> rejectAction) {
         this.authorizer = authorizer;
-        this.responseWriter = responseWriter;
+        this.rejectAction = rejectAction;
     }
 
     @Override
@@ -52,24 +53,35 @@ public class AuthorizationFilter implements SecurityRequestFilter {
         Optional<X509Certificate> cert = request.getClientCertificateChain().stream().findFirst();
         if (cert.isPresent()) {
             if (!authorizer.test(() -> commonName(cert.get()), request.getUri())) {
-                responseWriter.accept(ErrorResponse.forbidden(
+                rejectAction.accept(ErrorResponse.forbidden(
                         String.format("%s %s denied for %s: Invalid credentials", request.getMethod(),
                                       request.getUri().getPath(), request.getRemoteAddr())), handler
                 );
             }
         } else {
-            responseWriter.accept(ErrorResponse.unauthorized(
+            rejectAction.accept(ErrorResponse.unauthorized(
                     String.format("%s %s denied for %s: Missing credentials", request.getMethod(),
                                   request.getUri().getPath(), request.getRemoteAddr())), handler
             );
         }
     }
 
-    /** Log error response without writing anything */
+    private static BiConsumer<ErrorResponse, ResponseHandler> rejectActionIn(SystemName system) {
+        if (system == SystemName.cd) {
+            return AuthorizationFilter::logAndReject;
+        }
+        return AuthorizationFilter::log;
+    }
+
     private static void log(ErrorResponse response, @SuppressWarnings("unused") ResponseHandler handler) {
         log.warning("Would reject request: " + response.getStatus() + " - " + response.message());
     }
 
+    private static void logAndReject(ErrorResponse response, ResponseHandler handler) {
+        log.warning(response.message());
+        FilterUtils.write(response, handler);
+    }
+
     /** Read common name (CN) from certificate */
     private static String commonName(X509Certificate certificate) {
         try {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java
index 18a036a98ab..1d59ed52b67 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java
@@ -2,9 +2,11 @@
 package com.yahoo.vespa.hosted.provision.restapi.v2.filter;
 
 import com.yahoo.application.container.handler.Request.Method;
+import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.curator.mock.MockCurator;
-import com.yahoo.vespa.hosted.provision.restapi.v2.Authorizer;
 import com.yahoo.vespa.hosted.provision.restapi.v2.filter.FilterTester.Request;
 import com.yahoo.vespa.hosted.provision.testutils.MockNodeFlavors;
 import com.yahoo.vespa.hosted.provision.testutils.MockNodeRepository;
@@ -20,10 +22,7 @@ public class AuthorizationFilterTest {
 
     @Before
     public void before() {
-        tester = new FilterTester(new AuthorizationFilter(new Authorizer(SystemName.main,
-                                                                         new MockNodeRepository(new MockCurator(),
-                                                                                                new MockNodeFlavors())),
-                                                          FilterUtils::write));
+        tester = filterTester(SystemName.cd);
     }
 
     @Test
@@ -44,4 +43,18 @@ public class AuthorizationFilterTest {
         tester.assertSuccess(new Request(Method.GET, "/nodes/v2/node/foo").commonName("foo"));
     }
 
+    // TODO: Remove once filter applies to all systems
+    @Test
+    public void filter_does_nothing_in_main_system() {
+        FilterTester tester = filterTester(SystemName.main);
+        tester.assertSuccess(new Request(Method.GET, "/").commonName("foo"));
+        tester.assertSuccess(new Request(Method.GET, "/nodes/v2/node/bar").commonName("foo"));
+    }
+
+    private static FilterTester filterTester(SystemName system) {
+        Zone zone = new Zone(system, Environment.prod, RegionName.defaultName());
+        return new FilterTester(new AuthorizationFilter(zone, new MockNodeRepository(new MockCurator(),
+                                                                                     new MockNodeFlavors())));
+    }
+
 }