From 7fa55e578ad55705309a1f91b168ab1282c99679 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Sun, 14 Jan 2018 14:57:46 +0100 Subject: [PATCH] chg: factor out certificate locating logic Change-Id: I5436574b597ca5108b4badc093f93ec67193955b --- src/club/wpia/gigi/dbObjects/Certificate.java | 57 +++++++++++++++ .../gigi/pages/main/KeyCompromiseForm.java | 73 ++++--------------- .../gigi/pages/main/KeyCompromiseTest.java | 10 +-- 3 files changed, 76 insertions(+), 64 deletions(-) diff --git a/src/club/wpia/gigi/dbObjects/Certificate.java b/src/club/wpia/gigi/dbObjects/Certificate.java index 825b3392..5a02f477 100644 --- a/src/club/wpia/gigi/dbObjects/Certificate.java +++ b/src/club/wpia/gigi/dbObjects/Certificate.java @@ -3,6 +3,7 @@ package club.wpia.gigi.dbObjects; import java.io.ByteArrayInputStream; import java.io.IOException; import java.security.GeneralSecurityException; +import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.sql.Date; @@ -164,6 +165,10 @@ public class Certificate implements IdCachable { private String description = ""; + public static final TranslateCommand NOT_LOADED = new TranslateCommand("Certificate could not be loaded"); + + public static final TranslateCommand NOT_PARSED = new TranslateCommand("Certificate could not be parsed"); + /** * Creates a new Certificate. WARNING: this is an internal API. Creating * certificates for users must be done using the {@link CertificateRequest} @@ -582,4 +587,56 @@ public class Certificate implements IdCachable { public String getDescription() { return description; } + + public static Certificate locateCertificate(String serial, String certData) throws GigiApiException { + Certificate c = null; + + if (serial != null && !serial.isEmpty()) { + c = getBySerialFriendly(serial); + if (c == null) { + return null; + } + } + if (certData != null && !certData.isEmpty()) { + X509Certificate c0; + X509Certificate cert = null; + final byte[] supplied; + try { + supplied = PEM.decode("CERTIFICATE", certData); + c0 = (X509Certificate) CertificateFactory.getInstance("X509").generateCertificate(new ByteArrayInputStream(supplied)); + } catch (IllegalArgumentException e1) { + throw new GigiApiException(NOT_PARSED); + } catch (CertificateException e1) { + throw new GigiApiException(NOT_PARSED); + } + try { + c = getBySerialFriendly(c0.getSerialNumber().toString(16)); + if (c == null) { + return null; + } + cert = c.cert(); + if ( !Arrays.equals(supplied, cert.getEncoded())) { + return null; + } + } catch (IOException e) { + throw new GigiApiException(NOT_LOADED); + } catch (GeneralSecurityException e) { + throw new GigiApiException(NOT_LOADED); + } + } + if (c == null) { + throw new GigiApiException("No information to identify the correct certificate was provided."); + } + return c; + } + + private static Certificate getBySerialFriendly(String serial) throws GigiApiException { + serial = serial.trim().toLowerCase(); + int idx = 0; + while (idx < serial.length() && serial.charAt(idx) == '0') { + idx++; + } + serial = serial.substring(idx); + return Certificate.getBySerial(serial); + } } diff --git a/src/club/wpia/gigi/pages/main/KeyCompromiseForm.java b/src/club/wpia/gigi/pages/main/KeyCompromiseForm.java index 84af8c31..e0690844 100644 --- a/src/club/wpia/gigi/pages/main/KeyCompromiseForm.java +++ b/src/club/wpia/gigi/pages/main/KeyCompromiseForm.java @@ -1,6 +1,5 @@ package club.wpia.gigi.pages.main; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.PrintWriter; import java.io.UnsupportedEncodingException; @@ -8,12 +7,9 @@ import java.security.GeneralSecurityException; import java.security.KeyFactory; import java.security.PrivateKey; import java.security.Signature; -import java.security.cert.CertificateException; -import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.security.interfaces.RSAPrivateKey; import java.security.spec.PKCS8EncodedKeySpec; -import java.util.Arrays; import java.util.Base64; import java.util.HashMap; import java.util.Locale; @@ -52,8 +48,6 @@ public class KeyCompromiseForm extends Form { public static final String CHALLENGE_PREFIX = "This private key has been compromised. Challenge: "; - public static final TranslateCommand NOT_LOADED = new TranslateCommand("Certificate could not be loaded"); - public static final TranslateCommand NOT_FOUND = new TranslateCommand("Certificate to revoke not found"); private static final MailTemplate revocationNotice = new MailTemplate(KeyCompromiseForm.class.getResource("RevocationNotice.templ")); @@ -68,47 +62,23 @@ public class KeyCompromiseForm extends Form { if (RATE_LIMIT.isLimitExceeded(req.getRemoteAddr())) { throw new RateLimitException(); } - Certificate c = null; - X509Certificate cert = null; - String serial = req.getParameter("serial"); - String certData = req.getParameter("cert"); - if (serial != null && !serial.isEmpty()) { - c = fetchCertificate(serial); - try { - cert = c.cert(); - } catch (IOException e) { - throw new PermamentFormException(new GigiApiException(NOT_LOADED)); - } catch (GeneralSecurityException e) { - throw new PermamentFormException(new GigiApiException(NOT_LOADED)); - } - } - if (certData != null && !certData.isEmpty()) { - X509Certificate c0; - byte[] supplied; - try { - supplied = PEM.decode("CERTIFICATE", certData); - c0 = (X509Certificate) CertificateFactory.getInstance("X509").generateCertificate(new ByteArrayInputStream(supplied)); - } catch (IllegalArgumentException e1) { - throw new PermamentFormException(new GigiApiException("Your certificate could not be parsed")); - } catch (CertificateException e1) { - throw new PermamentFormException(new GigiApiException("Your certificate could not be parsed")); - } - try { - String ser = c0.getSerialNumber().toString(16); - c = fetchCertificate(ser); - cert = c.cert(); - if ( !Arrays.equals(supplied, cert.getEncoded())) { - throw new PermamentFormException(new GigiApiException(NOT_FOUND)); - } - } catch (IOException e) { - throw new PermamentFormException(new GigiApiException(NOT_LOADED)); - } catch (GeneralSecurityException e) { - throw new PermamentFormException(new GigiApiException(NOT_LOADED)); + Certificate c; + try { + c = Certificate.locateCertificate(req.getParameter("serial"), req.getParameter("cert")); + if (c == null) { + throw new GigiApiException(NOT_FOUND); } + } catch (GigiApiException e) { + throw new PermamentFormException(e); } - if (c == null) { - throw new PermamentFormException(new GigiApiException("No certificate identification information provided")); + + X509Certificate cert; + try { + cert = c.cert(); + } catch (IOException | GeneralSecurityException e) { + throw new PermamentFormException(new GigiApiException(Certificate.NOT_LOADED)); } + if (c.getStatus() == CertificateStatus.REVOKED) { return new SuccessMessageResult(new TranslateCommand("Certificate had already been revoked")); } @@ -219,21 +189,6 @@ public class KeyCompromiseForm extends Form { return signature; } - private Certificate fetchCertificate(String serial) { - Certificate c; - serial = serial.trim().toLowerCase(); - int idx = 0; - while (idx < serial.length() && serial.charAt(idx) == '0') { - idx++; - } - serial = serial.substring(idx); - c = Certificate.getBySerial(serial); - if (c == null) { - throw new PermamentFormException(new GigiApiException(NOT_FOUND)); - } - return c; - } - @Override protected void outputContent(PrintWriter out, Language l, Map vars) { vars.put("challenge", challenge); diff --git a/tests/club/wpia/gigi/pages/main/KeyCompromiseTest.java b/tests/club/wpia/gigi/pages/main/KeyCompromiseTest.java index 49ffd801..cf78945f 100644 --- a/tests/club/wpia/gigi/pages/main/KeyCompromiseTest.java +++ b/tests/club/wpia/gigi/pages/main/KeyCompromiseTest.java @@ -104,10 +104,10 @@ public class KeyCompromiseTest extends ClientTest { params("serial=1&priv=%priv", NOT_FOUND), params("serial=1%serial&priv=%priv", NOT_FOUND), // missing certificate identification - params("serial=&cert=&priv=%priv", "identification"), - params("cert=&priv=%priv", "identification"), - params("serial=&priv=%priv", "identification"), - params("priv=%priv", "identification"), + params("serial=&cert=&priv=%priv", "No information to identify"), + params("cert=&priv=%priv", "No information to identify"), + params("serial=&priv=%priv", "No information to identify"), + params("priv=%priv", "No information to identify"), // sign missing params("serial=%serial&priv=&signature=", "No verification"), params("serial=%serial&signature=", "No verification"), @@ -115,7 +115,7 @@ public class KeyCompromiseTest extends ClientTest { params("serial=%serial", "No verification"), params("cert=%cert&signature=%tamperedSignature", "Verification does not match"), - params("cert=-_&signature=%signature", "certificate could not be parsed"), + params("cert=-_&signature=%signature", "Certificate could not be parsed"), params("cert=%cert&signature=-_", "Signature is malformed"), params("cert=%cert&priv=-_", "Private Key is malformed"), }; -- 2.39.2