Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Tr default cert #3392

Merged
rawlinp merged 10 commits into from
Apr 10, 2019
Merged

Tr default cert #3392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6539c1c
Initial commit of default cert changes
Mar 6, 2019
cd2b13d
Changed error message for Certificate Expired
Mar 7, 2019
ac74234
corrections from PR Review
Mar 12, 2019
c38a291
Added CertificateExtensions to default SSL cert
Mar 21, 2019
98617f9
Added fix for Tomcat bug related to error state of sockets
Apr 2, 2019
98c10ca
fixed some spacing and made getInstance synchronized
Apr 5, 2019
21f3806
fixed some spacing and made getInstance synchronized
Apr 5, 2019
db4e04c
added an if condition to 'getInstance' to prevent double initialization
Apr 5, 2019
cfbfc48
Moved default certificate lifecycle into `importCertificateDataList`
Apr 8, 2019
9fe3742
Reformatted `RouterNIOEndpoint`
Apr 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).

## [Unreleased]
### Added
- Traffic Router: TR now generates a self-signed certificate at startup and uses it as the default TLS cert.
The default certificate is used whenever a client attempts an SSL handshake for an SNI host which does not match
any of the other certificates.
- Traffic Ops Golang Endpoints
- /api/1.4/users `(GET,POST,PUT)`
- /api/1.1/deliveryservices/xmlId/:xmlid/sslkeys `GET`
Expand Down
186 changes: 94 additions & 92 deletions .../main/java/com/comcast/cdn/traffic_control/traffic_router/protocol/RouterNioEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,103 +19,105 @@
import com.comcast.cdn.traffic_control.traffic_router.secure.HandshakeData;
import com.comcast.cdn.traffic_control.traffic_router.secure.KeyManager;
import org.apache.log4j.Logger;
import org.apache.tomcat.util.modeler.Registry;
import org.apache.tomcat.jni.SSL;
import org.apache.tomcat.util.net.NioChannel;
import org.apache.tomcat.util.net.NioEndpoint;
import org.apache.tomcat.util.net.SSLHostConfig;
import org.apache.tomcat.util.net.SSLHostConfigCertificate;
import org.apache.tomcat.util.net.SocketEvent;
import org.apache.tomcat.util.net.SocketProcessorBase;
import org.apache.tomcat.util.net.SocketWrapperBase;

import java.util.Map;
import java.util.Set;

public class RouterNioEndpoint extends NioEndpoint {
private static final Logger LOGGER = Logger.getLogger(RouterNioEndpoint.class);
// Grabs the aliases from our custom certificate registry, creates a sslHostConfig for them
// and adds the newly created config to the list of sslHostConfigs. We also remove the default config
// since it won't be found in our registry. This allows OpenSSL to start successfully and serve our
// certificates. When we are done we call the parent classes initialiseSsl.
@SuppressWarnings({"PMD.SignatureDeclareThrowsException"})
@Override
protected void initialiseSsl() throws Exception {
if (isSSLEnabled()) {
destroySsl();
sslHostConfigs.clear();
final KeyManager keyManager = new KeyManager();
final CertificateRegistry certificateRegistry = keyManager.getCertificateRegistry();
replaceSSLHosts(certificateRegistry.getHandshakeData());

//Now let initialiseSsl do it's thing.
super.initialiseSsl();
certificateRegistry.setEndPoint(this);
}
}

synchronized private void replaceSSLHosts(final Map<String, HandshakeData> sslHostsData) {
final Set<String> aliases = sslHostsData.keySet();
boolean firstAlias = true;
String lastHostName = "";

for (final String alias : aliases) {
final SSLHostConfig sslHostConfig = new SSLHostConfig();
final SSLHostConfigCertificate cert = new SSLHostConfigCertificate(sslHostConfig, SSLHostConfigCertificate.Type.RSA);
cert.setCertificateKeyAlias(alias);
sslHostConfig.addCertificate(cert);
sslHostConfig.setCertificateKeyAlias(alias);
sslHostConfig.setHostName(sslHostsData.get(alias).getHostname());
sslHostConfig.setProtocols("all");
sslHostConfig.setConfigType(getSslConfigType());
sslHostConfig.setCertificateVerification("none");
LOGGER.info("sslHostConfig: "+sslHostConfig.getHostName()+" "+sslHostConfig.getTruststoreAlgorithm());

if (!sslHostConfig.getHostName().equals(lastHostName)) {
addSslHostConfig(sslHostConfig, true);
lastHostName = sslHostConfig.getHostName();
}

if (firstAlias && ! "".equals(alias)) {
// One of the configs must be set as the default
setDefaultSSLHostConfigName(sslHostConfig.getHostName());
firstAlias = false;
}
}

}

synchronized public void reloadSSLHosts(final Map<String, HandshakeData> cr) {
replaceSSLHosts(cr);

for (final HandshakeData data : cr.values()) {
Comment thread
rawlinp marked this conversation as resolved.
Outdated
final SSLHostConfig sslHostConfig = sslHostConfigs.get(data.getHostname());
sslHostConfig.setConfigType(getSslConfigType());
createSSLContext(sslHostConfig);
}
}

@Override
protected SSLHostConfig getSSLHostConfig(final String sniHostName) {
return super.getSSLHostConfig(sniHostName.toLowerCase());
}

private void unregisterJmx(final SSLHostConfig sslHostConfig) {
final Registry registry = Registry.getRegistry(null, null);
registry.unregisterComponent(sslHostConfig.getObjectName());
for (final SSLHostConfigCertificate sslHostConfigCert : sslHostConfig.getCertificates()) {
registry.unregisterComponent(sslHostConfigCert.getObjectName());
}
}

@Override
public void addSslHostConfig(final SSLHostConfig sslHostConfig, final boolean replace) throws IllegalArgumentException {
final String key = sslHostConfig.getHostName();
if (key == null || key.length() == 0) {
throw new IllegalArgumentException(sm.getString("endpoint.noSslHostName"));
}

SSLHostConfig previous = null;
if (replace) {
previous = sslHostConfigs.get(key);
}
super.addSslHostConfig(sslHostConfig, replace);
if (previous != null) {
unregisterJmx(previous);
}
}
private static final Logger LOGGER = Logger.getLogger(RouterNioEndpoint.class);

// Grabs the aliases from our custom certificate registry, creates a sslHostConfig for them
// and adds the newly created config to the list of sslHostConfigs. We also remove the default config
// since it won't be found in our registry. This allows OpenSSL to start successfully and serve our
// certificates. When we are done we call the parent classes initialiseSsl.
@SuppressWarnings({"PMD.SignatureDeclareThrowsException"})
@Override
protected void initialiseSsl() throws Exception{
if (isSSLEnabled()){
destroySsl();
sslHostConfigs.clear();
final KeyManager keyManager = new KeyManager();
final CertificateRegistry certificateRegistry = keyManager.getCertificateRegistry();
replaceSSLHosts(certificateRegistry.getHandshakeData());

//Now let initialiseSsl do it's thing.
super.initialiseSsl();
certificateRegistry.setEndPoint(this);
}
}

synchronized public void replaceSSLHosts(final Map<String, HandshakeData> sslHostsData){
final Set<String> aliases = sslHostsData.keySet();
String lastHostName = "";

for (final String alias : aliases){
final SSLHostConfig sslHostConfig = new SSLHostConfig();
final SSLHostConfigCertificate cert = new SSLHostConfigCertificate(sslHostConfig, SSLHostConfigCertificate.Type.RSA);
cert.setCertificateKeyAlias(alias);
sslHostConfig.addCertificate(cert);
sslHostConfig.setCertificateKeyAlias(alias);
sslHostConfig.setHostName(sslHostsData.get(alias).getHostname());
sslHostConfig.setProtocols("all");
sslHostConfig.setCertificateVerification("none");
LOGGER.info("sslHostConfig: " + sslHostConfig.getHostName() + " " + sslHostConfig.getTruststoreAlgorithm());

if (!sslHostConfig.getHostName().equals(lastHostName)){
addSslHostConfig(sslHostConfig, true);
lastHostName = sslHostConfig.getHostName();
}

if (CertificateRegistry.DEFAULT_SSL_KEY.equals(alias)){
// One of the configs must be set as the default
setDefaultSSLHostConfigName(sslHostConfig.getHostName());
}
}
}

@Override
protected SSLHostConfig getSSLHostConfig(final String sniHostName){
return super.getSSLHostConfig(sniHostName.toLowerCase());
}

@Override
protected SocketProcessorBase<NioChannel> createSocketProcessor(
final SocketWrapperBase<NioChannel> socketWrapper, final SocketEvent event){
return new RouterSocketProcessor(socketWrapper, event);
}

/**
* This class is the equivalent of the Worker, but will simply use in an
* external Executor thread pool.
*/
protected class RouterSocketProcessor extends SocketProcessor {

public RouterSocketProcessor(final SocketWrapperBase<NioChannel> socketWrapper, final SocketEvent event){
super(socketWrapper, event);
}

/* This override has been added as a temporary hack to resolve an issue in Tomcat.
Once the issue has been corrected in Tomcat then this can be removed. The
'SSL.getLastErrorNumber()' removes an unwanted error condition from the error stack
in those cases where some error condition has caused the socket to get closed and
then the processor was put back on the processor stack for reuse in a future connection.
*/
@Override
protected void doRun(){
final SocketWrapperBase<NioChannel> localWrapper = socketWrapper;
Copy link
Copy Markdown
Contributor

@rawlinp rawlinp Apr 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line is still way over-indented -- its using tabs and it looks like the other lines use spaces

Copy link
Copy Markdown
Contributor Author

@ajschmidt ajschmidt Apr 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. It should be better now. There seems to be an issue with one of my code templates.

Copy link
Copy Markdown
Contributor

@rawlinp rawlinp Apr 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it looks much better now.

final NioChannel socket = localWrapper.getSocket();
super.doRun();
if (!socket.isOpen()){
SSL.getLastErrorNumber();
}
}
}

}

16 changes: 9 additions & 7 deletions .../java/com/comcast/cdn/traffic_control/traffic_router/secure/CertificateDataConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,18 +64,20 @@ else if (!hostMatch) {
log.warn("Service name doesn't match the subject of the certificate = "+certificateData.getHostname());
}
else if (!modMatch) {
log.error("Modulus of the private key does not match the public key modulus for certificate host: "+certificateData.getHostname());
log.warn("Modulus of the private key does not match the public key modulus for certificate host: "+certificateData.getHostname());
}

} catch (CertificateNotYetValidException er) {
log.error("Failed to convert certificate data for delivery service = " + certificateData.getHostname()
+ ", because the certificate is not valid yet. ");
log.warn("Failed to convert certificate data for delivery service = " + certificateData.getHostname()
+ ", because the certificate is not valid yet. This certificate will not be used by " +
"Traffic Router.");
} catch (CertificateExpiredException ex ) {
log.error("Failed to convert certificate data for delivery service = " + certificateData.getHostname()
+ ", because the certificate has expired. ");
log.warn("Failed to convert certificate data for delivery service = " + certificateData.getHostname()
+ ", because the certificate has expired. This certificate will not be used by Traffic Router.");
} catch (Exception e) {
log.error("Failed to convert certificate data (delivery service = " + certificateData.getDeliveryservice()
+ ", hostname = " + certificateData.getHostname() + ") from traffic ops to handshake data! "
log.warn("Failed to convert certificate data (delivery service = " + certificateData.getDeliveryservice()
+ ", hostname = " + certificateData.getHostname() + ") from traffic ops to handshake data! This " +
"certificate will not be used by Traffic Router. "
+ e.getClass().getSimpleName() + ": " + e.getMessage(), e);
}
return null;
Expand Down
73 changes: 67 additions & 6 deletions .../main/java/com/comcast/cdn/traffic_control/traffic_router/secure/CertificateRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,26 @@
import com.comcast.cdn.traffic_control.traffic_router.protocol.RouterNioEndpoint;
import com.comcast.cdn.traffic_control.traffic_router.shared.CertificateData;
import org.apache.log4j.Logger;

import sun.security.tools.keytool.CertAndKeyGen;
import sun.security.util.ObjectIdentifier;
import sun.security.x509.BasicConstraintsExtension;
import sun.security.x509.CertificateExtensions;
import sun.security.x509.ExtendedKeyUsageExtension;
import sun.security.x509.KeyUsageExtension;

import java.security.PrivateKey;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.HashMap;
import java.util.Map;
import java.util.Vector;

import sun.security.x509.X500Name;
import java.security.cert.X509Certificate;
import java.util.Date;

public class CertificateRegistry {
public static final String DEFAULT_SSL_KEY = "default.invalid";
private static final Logger log = Logger.getLogger(CertificateRegistry.class);
private CertificateDataConverter certificateDataConverter = new CertificateDataConverter();
volatile private Map<String, HandshakeData> handshakeDataMap = new HashMap<>();
Expand All @@ -40,6 +53,41 @@ public static CertificateRegistry getInstance() {
return CertificateRegistryHolder.DELIVERY_SERVICE_CERTIFICATES;
}

@SuppressWarnings("PMD.UseArrayListInsteadOfVector")
private static HandshakeData createDefaultSsl() {
try {
final CertificateExtensions extensions = new CertificateExtensions();
final KeyUsageExtension keyUsageExtension = new KeyUsageExtension();
keyUsageExtension.set(KeyUsageExtension.DIGITAL_SIGNATURE, true);
keyUsageExtension.set(KeyUsageExtension.KEY_ENCIPHERMENT, true);
keyUsageExtension.set(KeyUsageExtension.KEY_CERTSIGN, true);
extensions.set(keyUsageExtension.getExtensionId().toString(), keyUsageExtension);
final Vector<ObjectIdentifier> objectIdentifiers = new Vector<>();
objectIdentifiers.add(new ObjectIdentifier("1.3.6.1.5.5.7.3.1"));
objectIdentifiers.add(new ObjectIdentifier("1.3.6.1.5.5.7.3.2"));
final ExtendedKeyUsageExtension extendedKeyUsageExtension = new ExtendedKeyUsageExtension( true,
objectIdentifiers);
extensions.set(extendedKeyUsageExtension.getExtensionId().toString(), extendedKeyUsageExtension);
extensions.set(BasicConstraintsExtension.NAME, new BasicConstraintsExtension(true,
new BasicConstraintsExtension(true,-1).getExtensionValue()));
final CertAndKeyGen certGen = new CertAndKeyGen("RSA", "SHA1WithRSA", null);
certGen.generate(2048);

//Generate self signed certificate
final X509Certificate[] chain = new X509Certificate[1];
chain[0] = certGen.getSelfCertificate(new X500Name("C=US; ST=CO; L=Denver; " +
"O=Apache Traffic Control; OU=Apache Foundation; OU=Hosted by Traffic Control; " +
"OU=CDNDefault; CN="+DEFAULT_SSL_KEY), new Date(System.currentTimeMillis() - 1000L * 60 ),
(long) 3 * 365 * 24 * 3600, extensions);
final PrivateKey serverPrivateKey = certGen.getPrivateKey();
return new HandshakeData(DEFAULT_SSL_KEY, DEFAULT_SSL_KEY, chain, serverPrivateKey);
}
catch (Exception e) {
log.error("Could not generate the default certificate: "+e.getMessage(),e);
return null;
}
}

public List<String> getAliases() {
return new ArrayList<>(handshakeDataMap.keySet());
}
Expand Down Expand Up @@ -70,7 +118,6 @@ synchronized public void importCertificateDataList(final List<CertificateData> c
for (final CertificateData certificateData : certificateDataList) {
try {
final String alias = certificateData.alias();

if (!master.containsKey(alias)) {
final HandshakeData handshakeData = certificateDataConverter.toHandshakeData(certificateData);
if (handshakeData != null) {
Expand All @@ -89,7 +136,6 @@ synchronized public void importCertificateDataList(final List<CertificateData> c
log.error("Failed to import certificate data for delivery service: '" + certificateData.getDeliveryservice() + "', hostname: '" + certificateData.getHostname() + "'");
}
}

// find CertificateData which has been removed
for (final String alias : previousData.keySet())
{
Expand All @@ -110,12 +156,27 @@ synchronized public void importCertificateDataList(final List<CertificateData> c
}
}

// Check to see if a Default cert has been provided by Traffic Ops
if (!master.containsKey(DEFAULT_SSL_KEY)){
// Check to see if a Default cert has been provided/created previously
if (handshakeDataMap.containsKey(DEFAULT_SSL_KEY)) {
master.put(DEFAULT_SSL_KEY, handshakeDataMap.get(DEFAULT_SSL_KEY));
}else{
// create a new default certificate
final HandshakeData defaultHd = createDefaultSsl();
if (defaultHd == null){
log.error("Failed to initialize the CertificateRegistry because of a problem with the 'default' " +
"certificate. Returning the Certificate Registry without a default.");
return;
Copy link
Copy Markdown
Contributor

@rawlinp rawlinp Apr 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this ends up being the case, would we still want to do handshakeDataMap = master before returning so that all the newly-processed certs would still be loaded (just without a default cert), or would it never make sense to keep the rest of the certs without the default cert in that case?

Copy link
Copy Markdown
Contributor Author

@ajschmidt ajschmidt Apr 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In both situation I think it will fail in either case when it goes to RouterNioEndpoint because there has to be a default SSLHost. This condition should only occur when TR is being initiated. I did perform a test and made sure that these lines only get called once. After that they are skipped because here is always a default cert.

Copy link
Copy Markdown
Contributor Author

@ajschmidt ajschmidt Apr 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words. I think this is the appropriate behavior. If it can't create the default certificate, something bigger is wrong and TR should not start.

Copy link
Copy Markdown
Contributor

@rawlinp rawlinp Apr 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, you make a fair point.

}
master.put(DEFAULT_SSL_KEY, defaultHd);
}
}
handshakeDataMap = master;

if (sslEndpoint != null) {
sslEndpoint.reloadSSLHosts(changes);
sslEndpoint.replaceSSLHosts(changes);
}

}

public CertificateDataConverter getCertificateDataConverter() {
Expand Down
3 changes: 2 additions & 1 deletion traffic_router/connector/src/test/java/secure/CertificateRegistryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public void itImportsCertificateData() throws Exception {
verify(certificateDataConverter).toHandshakeData(certificateData3);

assertThat(certificateRegistry.getAliases(),
containsInAnyOrder("ds-1.some-cdn.example.com", "ds-2.some-cdn.example.com", "ds-3.some-cdn.example.com"));
containsInAnyOrder(CertificateRegistry.DEFAULT_SSL_KEY, "ds-1.some-cdn.example.com",
"ds-2.some-cdn.example.com", "ds-3.some-cdn.example.com"));
}
}
Loading