Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gracefully handle certificate renewals #396

Merged
merged 3 commits into from Dec 21, 2018
Merged
Changes from 2 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+37 −14
Diff settings

Always

Just for now

Copy path View file
@@ -7,6 +7,7 @@
import org.apache.commons.codec.binary.StringUtils;
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.ssl.X509CertificateChainBuilder;
import org.bouncycastle.asn1.ASN1ObjectIdentifier;
import org.bouncycastle.asn1.x509.*;
import org.bouncycastle.jce.PrincipalUtil;
import org.slf4j.Logger;
@@ -26,10 +27,7 @@
import java.security.cert.X509Certificate;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Properties;
import java.util.*;

/**
* Created by Steven on 1/27/2015. Package: qz.auth Project: qz-print
@@ -147,18 +145,16 @@ public Certificate(String in) throws CertificateParsingException {

//Strip beginning and end
String[] split = in.split("--START INTERMEDIATE CERT--");
byte[] serverCertificate = Base64.decode(split[0].replaceAll(X509Constants.BEGIN_CERT, "").replaceAll(X509Constants.END_CERT, ""));

X509Certificate theIntermediateCertificate;
if (split.length == 2) {
byte[] intermediateCertificate = Base64.decode(split[1].replaceAll(X509Constants.BEGIN_CERT, "").replaceAll(X509Constants.END_CERT, ""));
theIntermediateCertificate = (X509Certificate)cf.generateCertificate(new ByteArrayInputStream(intermediateCertificate));
theIntermediateCertificate = (X509Certificate)cf.generateCertificate(new StringBufferInputStream(split[1]));

This comment has been minimized.

Copy link
@tresf

tresf Dec 21, 2018

Contributor

According to Oracle, this should be completely valid and return either a casted X509Certifcate or raise a CertificateException. In reality, the library fails to parse the cert without \r and/or \n per http://www.doublecloud.org/2014/03/reading-x-509-certificate-in-java-how-to-handle-format-issue/. I will be reverting to the Base64 code to avoid any backwards compatibility issues.

} else {
theIntermediateCertificate = null; //Self-signed
}

//Generate cert
theCertificate = (X509Certificate)cf.generateCertificate(new ByteArrayInputStream(serverCertificate));
theCertificate = (X509Certificate)cf.generateCertificate(new StringBufferInputStream(split[0]));
commonName = String.valueOf(PrincipalUtil.getSubjectX509Principal(theCertificate).getValues(X509Name.CN).get(0));
fingerprint = makeThumbPrint(theCertificate);
organization = String.valueOf(PrincipalUtil.getSubjectX509Principal(theCertificate).getValues(X509Name.O).get(0));
@@ -184,6 +180,12 @@ public Certificate(String in) throws CertificateParsingException {
}
}

try {
readRenewalInfo();
} catch (Exception e) {
log.error("Error reading certificate renewal info", e);
}

// Only do CRL checks on QZ-issued certificates
if (trustedRootCert != null && !overrideTrustedRootCert) {
CRL qzCrl = CRL.getInstance();
@@ -205,6 +207,29 @@ public Certificate(String in) throws CertificateParsingException {
}
}

private void readRenewalInfo() throws Exception {
Vector values = PrincipalUtil.getSubjectX509Principal(theCertificate).getValues(new ASN1ObjectIdentifier("2.5.4.13"));
if (values.isEmpty()) {
return;
}
String renewalInfo = String.valueOf(values.get(0));

String renewalPrefix = "renewal-of-";
if (! renewalInfo.startsWith(renewalPrefix)) {
throw new CertificateParsingException("Illformed renewal info");
}
String previousFingerprint = renewalInfo.substring(renewalPrefix.length());
if (previousFingerprint.length() != 40) {
throw new CertificateParsingException("Illformed renewal fingerprint");
}

// Add this certificate to the whitelist if the previous certificate was whitelisted
File allowed = FileUtilities.getFile(Constants.ALLOW_FILE);
if (existsInFile(previousFingerprint, allowed)) {
FileUtilities.printLineToFile(Constants.ALLOW_FILE, data());
}
}

private Certificate() {}


@@ -281,22 +306,22 @@ public boolean isSignatureValid(String signature, String data) {
/** Checks if the certificate has been added to the local trusted store */
public boolean isSaved() {
File allowed = FileUtilities.getFile(Constants.ALLOW_FILE);
return existsInFile(allowed);
return existsInFile(getFingerprint(), allowed);
}

/** Checks if the certificate has been added to the local blocked store */
public boolean isBlocked() {
File blocks = FileUtilities.getFile(Constants.BLOCK_FILE);
return existsInFile(blocks);
return existsInFile(getFingerprint(), blocks);
}

private boolean existsInFile(File file) {
private static boolean existsInFile(String fingerprint, File file) {
try(BufferedReader br = new BufferedReader(new FileReader(file))) {
String line;
while((line = br.readLine()) != null) {
if (line.contains("\t")) {
String print = line.substring(0, line.indexOf("\t"));
if (print.equals(getFingerprint())) {
if (print.equals(fingerprint)) {
return true;
}
}
@@ -4,8 +4,6 @@
* Created by Tres on 3/3/2015.
*/
public class X509Constants {
public static final String BEGIN_CERT = "-----BEGIN CERTIFICATE-----";
public static final String END_CERT = "-----END CERTIFICATE-----";
public static final String INTERMEDIATE_CERT = "--START INTERMEDIATE CERT--";
public static final String BEGIN_CRL = "-----BEGIN X509 CRL-----";
public static final String END_CRL = "-----END X509 CRL-----";
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.