descriptive error message for cert refresh request (#372)

* descriptive error message for cert refresh request * throw 403 for ip mismatch
AthenZ · Dec 30, 2017 · 3c811ba · 3c811ba
1 parent ac3c093
commit 3c811ba
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 14 deletions.
diff --git a/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSImpl.java b/servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSImpl.java
@@ -181,6 +181,10 @@ void convertToLowerCase(Object obj) {
         abstract void convertToLowerCase(Object obj);
     }
 
+    enum ServiceX509RefreshRequestStatus {
+        SUCCESS, DNS_NAME_MISMATCH, PUBLIC_KEY_MISMATCH, IP_NOT_ALLOWED
+     }
+
     public ZTSImpl() {
         this(null, null);
     }
@@ -2369,10 +2373,15 @@ public Identity postInstanceRefreshRequest(ResourceContext ctx, String domain,
         // to do further validation based on the certificate we authenticated
 
         if (refreshOperation) {
-            if (!validateServiceX509RefreshRequest(principal, x509CertReq,
-                    ServletRequestUtil.getRemoteAddress(ctx.request()))) {
-                throw requestError("postInstanceRefreshRequest: dns name or public key mismatch",
-                        caller, domain);
+            ServiceX509RefreshRequestStatus status =  validateServiceX509RefreshRequest(principal, x509CertReq,
+                    ServletRequestUtil.getRemoteAddress(ctx.request()));
+            if (status == ServiceX509RefreshRequestStatus.IP_NOT_ALLOWED) {
+                throw forbiddenError("postInstanceRefreshRequest: " + status,
+                        caller, domain); 
+            }
+            if (status != ServiceX509RefreshRequestStatus.SUCCESS) {
+                throw requestError("postInstanceRefreshRequest: " + status,
+                        caller, domain); 
             }
         }
 
@@ -2404,7 +2413,7 @@ public Identity postInstanceRefreshRequest(ResourceContext ctx, String domain,
         return identity;
     }
 
-    boolean validateServiceX509RefreshRequest(final Principal principal, final X509CertRequest certReq,
+    ServiceX509RefreshRequestStatus validateServiceX509RefreshRequest(final Principal principal, final X509CertRequest certReq,
             final String ipAddress) {
 
         // retrieve the certificate that was used for authentication
@@ -2413,22 +2422,22 @@ boolean validateServiceX509RefreshRequest(final Principal principal, final X509C
 
         X509Certificate cert = principal.getX509Certificate();
         if (!certReq.compareDnsNames(cert)) {
-            return false;
+            return ServiceX509RefreshRequestStatus.DNS_NAME_MISMATCH;
         }
 
         // validate that the certificate and csr both are based
         // on the same public key
 
         if (!certReq.comparePublicKeys(cert)) {
-            return false;
+            return ServiceX509RefreshRequestStatus.PUBLIC_KEY_MISMATCH;
         }
 
         // finally verify that the ip address is in the allowed range
 
-        return verifyIPAddressAccess(ipAddress);
+        return verifyIPAddressAccess(ipAddress) ? ServiceX509RefreshRequestStatus.SUCCESS : ServiceX509RefreshRequestStatus.IP_NOT_ALLOWED;
     }
 
-    boolean verifyIPAddressAccess(final String ipAddress) {
+    final boolean verifyIPAddressAccess(final String ipAddress) {
         long ipAddr = IPBlock.convertToLong(ipAddress);
         for (IPBlock ipBlock : certRefreshIPBlocks) {
             if (ipBlock.ipCheck(ipAddr)) {

diff --git a/servers/zts/src/test/java/com/yahoo/athenz/zts/ZTSImplTest.java b/servers/zts/src/test/java/com/yahoo/athenz/zts/ZTSImplTest.java
@@ -78,6 +78,7 @@
 import com.yahoo.athenz.zms.ServiceIdentity;
 import com.yahoo.athenz.zms.SignedDomain;
 import com.yahoo.athenz.zts.ZTSImpl.AthenzObject;
+import com.yahoo.athenz.zts.ZTSImpl.ServiceX509RefreshRequestStatus;
 import com.yahoo.athenz.zts.ZTSAuthorizer.AccessStatus;
 import com.yahoo.athenz.zts.cache.DataCache;
 import com.yahoo.athenz.zts.cert.CertRecordStore;
@@ -6663,7 +6664,7 @@ public void testValidateServiceX509RefreshRequest() throws IOException {
                 "syncer", "v=S1,d=athenz;n=syncer;s=sig", 0, new CertificateAuthority());
         principal.setX509Certificate(cert);
 
-        assertTrue(ztsImpl.validateServiceX509RefreshRequest(principal, certReq, "10.0.0.1"));
+        assertTrue(ztsImpl.validateServiceX509RefreshRequest(principal, certReq, "10.0.0.1") == ServiceX509RefreshRequestStatus.SUCCESS);
     }
 
     @Test
@@ -6693,7 +6694,7 @@ public void testValidateServiceX509RefreshRequestMismatchPublicKeys() throws IOE
                 "syncer", "v=S1,d=athenz;n=syncer;s=sig", 0, new CertificateAuthority());
         principal.setX509Certificate(cert);
 
-        assertFalse(ztsImpl.validateServiceX509RefreshRequest(principal, certReq, "10.0.0.1"));
+        assertTrue(ztsImpl.validateServiceX509RefreshRequest(principal, certReq, "10.0.0.1") == ServiceX509RefreshRequestStatus.PUBLIC_KEY_MISMATCH);
     }
 
     @Test
@@ -6724,7 +6725,7 @@ public void testValidateServiceX509RefreshRequestNotAllowedIP() throws IOExcepti
 
         // our ip will not match 10.0.0.1 thus failure
 
-        assertFalse(ztsImpl.validateServiceX509RefreshRequest(principal, certReq, "10.0.0.2"));
+        assertTrue(ztsImpl.validateServiceX509RefreshRequest(principal, certReq, "10.0.0.2") == ServiceX509RefreshRequestStatus.IP_NOT_ALLOWED);
     }
 
     @Test
@@ -6753,7 +6754,7 @@ public void testValidateServiceX509RefreshRequestMismatchDns() throws IOExceptio
                 "syncer", "v=S1,d=athenz;n=syncer;s=sig", 0, new CertificateAuthority());
         principal.setX509Certificate(cert);
 
-        assertFalse(ztsImpl.validateServiceX509RefreshRequest(principal, certReq, "10.0.0.1"));
+        assertTrue(ztsImpl.validateServiceX509RefreshRequest(principal, certReq, "10.0.0.1") == ServiceX509RefreshRequestStatus.DNS_NAME_MISMATCH);
     }
 
     @Test
@@ -6847,7 +6848,7 @@ public void testPostInstanceRefreshRequestByServiceCertValidateFail() throws IOE
             ztsImpl.postInstanceRefreshRequest(context, "athenz", "syncer", req);
             fail();
         } catch (ResourceException ex) {
-            assertTrue(ex.getMessage().contains("dns name or public key mismatch"), ex.getMessage());
+            assertTrue(ex.getMessage().contains("DNS_NAME_MISMATCH"), ex.getMessage());
         }
     }