chg: enhance type safety of serials

Change-Id: I07cebd21bd795803fb5f6e42dc18990918cb8c9c

diff --git a/src/club/wpia/gigi/Gigi.java b/src/club/wpia/gigi/Gigi.java
index 00993b52aa06fa889cc4185207ca87778479a2a2..de3a5d0fa19f3d928a07d83babd492783da200e9 100644 (file)
--- a/src/club/wpia/gigi/Gigi.java
+++ b/src/club/wpia/gigi/Gigi.java
@@ -3,6 +3,7 @@ package club.wpia.gigi;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.UnsupportedEncodingException;
+import java.math.BigInteger;
 import java.security.KeyStore;
 import java.security.cert.X509Certificate;
 import java.util.Calendar;
@@ -357,11 +358,11 @@ public final class Gigi extends HttpServlet {
             return;
         }
         HttpSession hs = req.getSession();
-        String clientSerial = (String) hs.getAttribute(CERT_SERIAL);
+        BigInteger clientSerial = (BigInteger) hs.getAttribute(CERT_SERIAL);
         if (clientSerial != null) {
             X509Certificate[] cert = (X509Certificate[]) req.getAttribute("javax.servlet.request.X509Certificate");
             if (cert == null || cert[0] == null//
-                    || !cert[0].getSerialNumber().toString(16).toLowerCase().equals(clientSerial) //
+                    || !cert[0].getSerialNumber().equals(clientSerial) //
                     || !cert[0].getIssuerDN().equals(hs.getAttribute(CERT_ISSUER))) {
                 hs.invalidate();
                 resp.sendError(403, "Certificate mismatch.");

diff --git a/src/club/wpia/gigi/api/APIPoint.java b/src/club/wpia/gigi/api/APIPoint.java
index 737426a733fa8bf7d22cc21f267e3911bd509693..f089e84ec9bcc3b916c9b575515f70b7ca6353cb 100644 (file)
--- a/src/club/wpia/gigi/api/APIPoint.java
+++ b/src/club/wpia/gigi/api/APIPoint.java
@@ -1,6 +1,7 @@
 package club.wpia.gigi.api;
 
 import java.io.IOException;
+import java.math.BigInteger;
 import java.security.cert.X509Certificate;
 
 import javax.servlet.http.HttpServletRequest;
@@ -19,7 +20,7 @@ public abstract class APIPoint {
             resp.sendError(403, "Error, cert authing required. No cert found.");
             return;
         }
-        String serial = LoginPage.extractSerialFormCert(cert);
+        BigInteger serial = LoginPage.extractSerialFormCert(cert);
         Certificate clientCert = Certificate.getBySerial(serial);
         CertificateOwner u = CertificateOwner.getByEnabledSerial(serial);
         if (u == null || clientCert == null) {

diff --git a/src/club/wpia/gigi/api/CATSResolve.java b/src/club/wpia/gigi/api/CATSResolve.java
index f326fb4e45c2fd4efb09be45dc3b1e758910ee50..51165d1c3f34d69d0afbd83adcdd17354a756d6e 100644 (file)
--- a/src/club/wpia/gigi/api/CATSResolve.java
+++ b/src/club/wpia/gigi/api/CATSResolve.java
@@ -1,10 +1,12 @@
 package club.wpia.gigi.api;
 
 import java.io.IOException;
+import java.math.BigInteger;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import club.wpia.gigi.GigiApiException;
 import club.wpia.gigi.dbObjects.Certificate;
 import club.wpia.gigi.dbObjects.CertificateOwner;
 import club.wpia.gigi.dbObjects.Organisation;
@@ -22,13 +24,19 @@ public class CATSResolve extends CATSRestrictedApi {
             resp.sendError(500, "Error, requires a serial parameter");
             return;
         }
-        target = target.toLowerCase();
-        Certificate clientCert = Certificate.getBySerial(target);
+        BigInteger targetSerial;
+        try {
+            targetSerial = Certificate.normalizeSerial(target);
+        } catch (GigiApiException e) {
+            resp.sendError(500, "Error, requires valid serial");
+            return;
+        }
+        Certificate clientCert = Certificate.getBySerial(targetSerial);
         if (clientCert == null) {
             resp.sendError(500, "Error, requires valid serial");
             return;
         }
-        CertificateOwner o = CertificateOwner.getByEnabledSerial(target);
+        CertificateOwner o = CertificateOwner.getByEnabledSerial(targetSerial);
         if (o instanceof Organisation) {
             Organisation org = (Organisation) o;
             if (org.isSelfOrganisation()) {

diff --git a/src/club/wpia/gigi/api/FindAgent.java b/src/club/wpia/gigi/api/FindAgent.java
index 5ebf15ab8e23e6c082b073301c340da2540f6bdc..d37ccbb73b0455cc7d4da87a02d1f3407b6e7112 100644 (file)
--- a/src/club/wpia/gigi/api/FindAgent.java
+++ b/src/club/wpia/gigi/api/FindAgent.java
@@ -2,6 +2,7 @@ package club.wpia.gigi.api;
 
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.math.BigInteger;
 import java.util.HashMap;
 
 import javax.servlet.http.HttpServletRequest;
@@ -49,11 +50,11 @@ public class FindAgent extends APIPoint {
         String pi = req.getPathInfo();
         if (pi.equals(PATH_RESOLVE)) {
             String serial = req.getParameter("serial");
-            if (serial == null) {
+            if (serial == null || serial.isEmpty()) {
                 resp.sendError(500, "Error, requires serial");
                 return;
             }
-            Certificate c = Certificate.getBySerial(serial);
+            Certificate c = Certificate.getBySerial(new BigInteger(serial, 16));
             if (c == null) {
                 resp.sendError(500, "Error, requires serial");
                 return;

diff --git a/src/club/wpia/gigi/api/RevokeCertificate.java b/src/club/wpia/gigi/api/RevokeCertificate.java
index 5259fb02370ede6b48cc952128847e75831ba35b..81e57cfbbf3ec23a184bf4f7710ffad0d960b4c1 100644 (file)
--- a/src/club/wpia/gigi/api/RevokeCertificate.java
+++ b/src/club/wpia/gigi/api/RevokeCertificate.java
@@ -1,6 +1,7 @@
 package club.wpia.gigi.api;
 
 import java.io.IOException;
+import java.math.BigInteger;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -29,12 +30,12 @@ public class RevokeCertificate extends APIPoint {
         }
 
         String tserial = req.getParameter("serial");
-        if (tserial == null) {
+        if (tserial == null || tserial.isEmpty()) {
             resp.sendError(500, "Error, no Serial found");
             return;
         }
 
-        Certificate c = Certificate.getBySerial(tserial);
+        Certificate c = Certificate.getBySerial(new BigInteger(tserial, 16));
         if (c == null || c.getOwner() != u) {
             resp.sendError(403, "Access Denied");
             return;

diff --git a/src/club/wpia/gigi/dbObjects/Certificate.java b/src/club/wpia/gigi/dbObjects/Certificate.java
index eae8aab3af48b39b6db624e8947dc9d1e67509c2..014c697f8fd5628d52a466817efbb9bf211949fa 100644 (file)
--- a/src/club/wpia/gigi/dbObjects/Certificate.java
+++ b/src/club/wpia/gigi/dbObjects/Certificate.java
@@ -2,6 +2,7 @@ package club.wpia.gigi.dbObjects;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.math.BigInteger;
 import java.security.GeneralSecurityException;
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
@@ -412,12 +413,12 @@ public class Certificate implements IdCachable {
 
     private static final String CONCAT = "string_agg(concat('/', `name`, '=', REPLACE(REPLACE(value, '\\\\', '\\\\\\\\'), '/', '\\\\/')), '')";
 
-    public synchronized static Certificate getBySerial(String serial) {
-        if (serial == null || "".equals(serial)) {
+    public synchronized static Certificate getBySerial(BigInteger serial) {
+        if (serial == null) {
             return null;
         }
         try (GigiPreparedStatement ps = new GigiPreparedStatement("SELECT certs.id, " + CONCAT + " as `subject`, `md`,`memid`, `profile`, `certs`.`serial`, `certs`.`description` FROM `certs` LEFT JOIN `certAvas` ON `certAvas`.`certId`=`certs`.`id` WHERE `serial`=? GROUP BY `certs`.`id`")) {
-            ps.setString(1, serial);
+            ps.setString(1, serial.toString(16));
             GigiResultSet rs = ps.executeQuery();
             if ( !rs.next()) {
                 return null;
@@ -610,7 +611,7 @@ public class Certificate implements IdCachable {
                 throw new GigiApiException(NOT_PARSED);
             }
             try {
-                c = getBySerial(c0.getSerialNumber().toString(16));
+                c = getBySerial(c0.getSerialNumber());
                 if (c == null) {
                     return null;
                 }
@@ -630,7 +631,7 @@ public class Certificate implements IdCachable {
         return c;
     }
 
-    public static String normalizeSerial(String serial) throws GigiApiException {
+    public static BigInteger normalizeSerial(String serial) throws GigiApiException {
         serial = serial.replace(" ", "");
         serial = serial.toLowerCase();
         if (serial.matches("[0-9a-f]{2}(:[0-9a-f]{2})*")) {
@@ -644,6 +645,6 @@ public class Certificate implements IdCachable {
         if ( !serial.matches("[0-9a-f]+")) {
             throw new GigiApiException("Malformed serial");
         }
-        return serial;
+        return new BigInteger(serial, 16);
     }
 }

diff --git a/src/club/wpia/gigi/dbObjects/CertificateOwner.java b/src/club/wpia/gigi/dbObjects/CertificateOwner.java
index 007d98d3a1092c7d5401c9ba3014f0e676f62bef..72e5a81672762a74510cfbe7743dda61f19d2231 100644 (file)
--- a/src/club/wpia/gigi/dbObjects/CertificateOwner.java
+++ b/src/club/wpia/gigi/dbObjects/CertificateOwner.java
@@ -5,6 +5,7 @@ import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.io.ObjectStreamException;
 import java.io.Serializable;
+import java.math.BigInteger;
 import java.util.LinkedList;
 import java.util.List;
 
@@ -139,9 +140,9 @@ public abstract class CertificateOwner implements IdCachable, Serializable {
         }
     }
 
-    public static CertificateOwner getByEnabledSerial(String serial) {
+    public static CertificateOwner getByEnabledSerial(BigInteger serial) {
         try (GigiPreparedStatement prep = new GigiPreparedStatement("SELECT `memid` FROM `certs` INNER JOIN `logincerts` ON `logincerts`.`id`=`certs`.`id` WHERE serial=? AND `revoked` is NULL")) {
-            prep.setString(1, serial);
+            prep.setString(1, serial.toString(16));
             GigiResultSet res = prep.executeQuery();
             if (res.next()) {
                 return getById(res.getInt(1));

diff --git a/src/club/wpia/gigi/ocsp/OCSPIssuer.java b/src/club/wpia/gigi/ocsp/OCSPIssuer.java
index 29fb527c50708887ad388f176ad2c5830307c8b4..23bbdbc974926e871a15a61b6f6e04fe4c0020db 100644 (file)
--- a/src/club/wpia/gigi/ocsp/OCSPIssuer.java
+++ b/src/club/wpia/gigi/ocsp/OCSPIssuer.java
@@ -80,7 +80,7 @@ public class OCSPIssuer {
      *             if encoding fails
      */
     public byte[] respondBytes(OCSPRequest req, CertId id) throws GeneralSecurityException, IOException {
-        Certificate tcert = Certificate.getBySerial(id.getSerialNumber().toString(16).toLowerCase());
+        Certificate tcert = Certificate.getBySerial(id.getSerialNumber());
         if (tcert == null) {
             return OCSPResponse.invalid();
         }

diff --git a/src/club/wpia/gigi/pages/LoginPage.java b/src/club/wpia/gigi/pages/LoginPage.java
index 69b93863366d5b1928cee228812756e169478022..b0ed6e69397dcaa4cd313752eed3e473c405f643 100644 (file)
--- a/src/club/wpia/gigi/pages/LoginPage.java
+++ b/src/club/wpia/gigi/pages/LoginPage.java
@@ -4,6 +4,7 @@ import static club.wpia.gigi.Gigi.*;
 
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.math.BigInteger;
 import java.security.cert.X509Certificate;
 import java.util.Map;
 
@@ -152,7 +153,7 @@ public class LoginPage extends Page {
     }
 
     private void tryAuthWithCertificate(HttpServletRequest req, X509Certificate x509Certificate) {
-        String serial = extractSerialFormCert(x509Certificate);
+        BigInteger serial = extractSerialFormCert(x509Certificate);
         User user = fetchUserBySerial(serial);
         if (user == null) {
             return;
@@ -163,15 +164,11 @@ public class LoginPage extends Page {
         req.getSession().setAttribute(LOGIN_METHOD, new TranslateCommand("Certificate"));
     }
 
-    public static String extractSerialFormCert(X509Certificate x509Certificate) {
-        return x509Certificate.getSerialNumber().toString(16).toLowerCase();
+    public static BigInteger extractSerialFormCert(X509Certificate x509Certificate) {
+        return x509Certificate.getSerialNumber();
     }
 
-    public static User fetchUserBySerial(String serial) {
-        if ( !serial.matches("[a-f0-9]+")) {
-            throw new Error("serial malformed.");
-        }
-
+    public static User fetchUserBySerial(BigInteger serial) {
         CertificateOwner o = CertificateOwner.getByEnabledSerial(serial);
         if (o == null || !(o instanceof User)) {
             return null;

diff --git a/src/club/wpia/gigi/pages/account/certs/CertificateModificationForm.java b/src/club/wpia/gigi/pages/account/certs/CertificateModificationForm.java
index 6d6cc8b510491526b0fce20264569bf816a5d593..7ca73eb05908c3bc3c4c999f70fa0e9194855312 100644 (file)
--- a/src/club/wpia/gigi/pages/account/certs/CertificateModificationForm.java
+++ b/src/club/wpia/gigi/pages/account/certs/CertificateModificationForm.java
@@ -1,6 +1,7 @@
 package club.wpia.gigi.pages.account.certs;
 
 import java.io.PrintWriter;
+import java.math.BigInteger;
 import java.util.LinkedList;
 import java.util.Map;
 
@@ -45,7 +46,7 @@ public class CertificateModificationForm extends Form {
         }
         LinkedList<Job> revokes = new LinkedList<Job>();
         for (String serial : certs) {
-            Certificate c = Certificate.getBySerial(serial);
+            Certificate c = Certificate.getBySerial(new BigInteger(serial, 16));
             if (c == null || c.getOwner() != target) {
                 continue;
             }

diff --git a/src/club/wpia/gigi/pages/account/certs/Certificates.java b/src/club/wpia/gigi/pages/account/certs/Certificates.java
index 62322554361348c2d91a0cedef772b52b1e054e9..40faa1f147960aac822378e0b746109abc59ba3f 100644 (file)
--- a/src/club/wpia/gigi/pages/account/certs/Certificates.java
+++ b/src/club/wpia/gigi/pages/account/certs/Certificates.java
@@ -2,6 +2,7 @@ package club.wpia.gigi.pages.account.certs;
 
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.math.BigInteger;
 import java.net.URLEncoder;
 import java.security.GeneralSecurityException;
 import java.security.cert.X509Certificate;
@@ -72,7 +73,7 @@ public class Certificates extends Page implements HandlesMixedRequest {
             cer = true;
             pi = pi.substring(0, pi.length() - 4);
         }
-        String serial = pi;
+        BigInteger serial = new BigInteger(pi, 16);
         try {
             Certificate c = Certificate.getBySerial(serial);
             if (c == null || ( !support && LoginPage.getAuthorizationContext(req).getTarget().getId() != c.getOwner().getId())) {
@@ -144,7 +145,7 @@ public class Certificates extends Page implements HandlesMixedRequest {
             pi = pi.substring(1);
 
             String serial = pi;
-            Certificate c = Certificate.getBySerial(serial);
+            Certificate c = Certificate.getBySerial(new BigInteger(serial, 16));
             Language l = LoginPage.getLanguage(req);
 
             if (c == null || ( !support && LoginPage.getAuthorizationContext(req).getTarget().getId() != c.getOwner().getId())) {

diff --git a/src/club/wpia/gigi/ping/SSLPinger.java b/src/club/wpia/gigi/ping/SSLPinger.java
index 97fb30da00538095c10a90c2bf20659e6337b9de..a2de227f85037c7586faa5e05add7949761458e3 100644 (file)
--- a/src/club/wpia/gigi/ping/SSLPinger.java
+++ b/src/club/wpia/gigi/ping/SSLPinger.java
@@ -259,7 +259,7 @@ public class SSLPinger extends DomainPinger {
             }
 
             BigInteger serial = first.getSerialNumber();
-            Certificate c = Certificate.getBySerial(serial.toString(16));
+            Certificate c = Certificate.getBySerial(serial);
             if (c == null) {
                 return "Certificate not found: Serial " + serial.toString(16) + " missing.";
             }

diff --git a/tests/club/wpia/gigi/TestCertificate.java b/tests/club/wpia/gigi/TestCertificate.java
index d76c5140cc348b6f4a4fe34f486dc4b3f25bf68d..fb5ca6f6667ee2de8f818ca4afd3c795f3491e19 100644 (file)
--- a/tests/club/wpia/gigi/TestCertificate.java
+++ b/tests/club/wpia/gigi/TestCertificate.java
@@ -3,6 +3,7 @@ package club.wpia.gigi;
 import static org.junit.Assert.*;
 
 import java.io.IOException;
+import java.math.BigInteger;
 import java.security.GeneralSecurityException;
 import java.security.KeyPair;
 import java.security.PrivateKey;
@@ -80,7 +81,7 @@ public class TestCertificate extends ManagedTest {
 
         testFails(CertificateStatus.ISSUED, c);
 
-        Certificate c2 = Certificate.getBySerial(c.getSerial());
+        Certificate c2 = Certificate.getBySerial(new BigInteger(c.getSerial(), 16));
         assertNotNull(c2);
         assertEquals(2, c2.getSANs().size());
         assertEquals(c.getSANs().get(0).getName(), c2.getSANs().get(0).getName());

diff --git a/tests/club/wpia/gigi/dbObjects/TestSerialNormalization.java b/tests/club/wpia/gigi/dbObjects/TestSerialNormalization.java
index f2b9a945edcb72ae3c254156b109b911399994c6..276a8abb115da19aa5e91d923f28950cb0841403 100644 (file)
--- a/tests/club/wpia/gigi/dbObjects/TestSerialNormalization.java
+++ b/tests/club/wpia/gigi/dbObjects/TestSerialNormalization.java
@@ -71,6 +71,6 @@ public class TestSerialNormalization {
                 return;
             }
         }
-        assertEquals(normalized, Certificate.normalizeSerial(input));
+        assertEquals(normalized, Certificate.normalizeSerial(input).toString(16));
     }
 }

diff --git a/tests/club/wpia/gigi/pages/account/TestCertificateAdd.java b/tests/club/wpia/gigi/pages/account/TestCertificateAdd.java
index 949085c47487a4d94a2253c759dd0fac5749f48e..0d83f301f0320db78b5717647f29609df5557017 100644 (file)
--- a/tests/club/wpia/gigi/pages/account/TestCertificateAdd.java
+++ b/tests/club/wpia/gigi/pages/account/TestCertificateAdd.java
@@ -401,10 +401,10 @@ public class TestCertificateAdd extends ClientTest {
     @Test
     public void testSetLoginEnabled() throws IOException, GeneralSecurityException {
         X509Certificate parsedLoginNotEnabled = createCertWithValidity("&validFrom=now&validity=1m", false);
-        assertNull(CertificateOwner.getByEnabledSerial(parsedLoginNotEnabled.getSerialNumber().toString(16).toLowerCase()));
+        assertNull(CertificateOwner.getByEnabledSerial(parsedLoginNotEnabled.getSerialNumber()));
 
         X509Certificate parsedLoginEnabled = createCertWithValidity("&validFrom=now&validity=1m", true);
-        assertEquals(u, CertificateOwner.getByEnabledSerial(parsedLoginEnabled.getSerialNumber().toString(16).toLowerCase()));
+        assertEquals(u, CertificateOwner.getByEnabledSerial(parsedLoginEnabled.getSerialNumber()));
     }
 
     @Test