Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Snapshotting the CRConfig now deletes HTTPS certificates in Riak for delivery services which have been deleted in Traffic Ops.

### Changed
- Traffic Router, added TLS certificate validation on certificates imported from Traffic Ops
- validates modulus of private and public keys
- validates current timestamp falls within the certificate date bracket
- validates certificate subjects against the DS URL
- Traffic Ops Golang Endpoints
- Updated /api/1.1/cachegroups: Cache Group Fallbacks are included
- Updated /api/1.1/cachegroups: fixed so fallbackToClosest can be set through API
Expand Down
2 changes: 2 additions & 0 deletions infrastructure/docker/traffic_router/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ ARG TC_REPO=traffic-control.repo
ADD $TMCAT /
ADD $RPM /
ADD $TC_REPO /etc/yum.repos.d/
ADD starttr.sh /
ADD shutdowntr.sh /
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How come? Are you docker execing into the container to start and stop Traffic Router? What do you gain by doing that without making any changes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

systemd has been disabled in the centos 7 docker container so we can no longer use the unit files to start/stop processes. The starttr.sh gets called by the run.sh, and yes, I do docker exec to run the shutdown. I also copy in new TR rpms so I can test code changes without having to rebuild the whole Docker image. We have done something similar in CIAB for the TR and ATS containers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well typically you're not supposed to do that with containers, but I'd like the opinion of someone who actually uses these things - @rob05c , thoughts?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this goes back to the whole "containers as VMs" vs "one process per container" thing, which really depends on how the container was designed. These scripts make it seem more like a "container as a VM". That said, I don't really see a problem with adding these scripts to this TR container, but it probably should've been done in a different PR since they don't seem to be required for adding TR SSL cert validation.


### Common for all sub-component builds
RUN yum -y install \
Expand Down
2 changes: 1 addition & 1 deletion infrastructure/docker/traffic_router/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
# ORIGIN_URI # origin server (e.g. hotair), used to create a delivery service

start() {
systemctl start traffic_router
./starttr.sh
touch /opt/traffic_router/var/log/traffic_router.log
exec tail -f /opt/traffic_router/var/log/traffic_router.log
}
Expand Down
36 changes: 36 additions & 0 deletions infrastructure/docker/traffic_router/shutdowntr.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/bin/bash
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

# Script for running the Dockerfile for Traffic Router.
# The Dockerfile sets up a Docker image which can be used for any new container;
# This script, which should be run when the container is run (it's the ENTRYPOINT), will configure the container.
#
# The following environment variables must be set (ordinarily by `docker run -e` arguments):
# TRAFFIC_OPS_URI
# TRAFFIC_OPS_USER
# TRAFFIC_OPS_PASS
# TRAFFIC_MONITORS # list of semicolon-delimited FQDN:port monitors. E.g. `monitor.foo.com:80;monitor2.bar.org:80`
# ORIGIN_URI # origin server (e.g. hotair), used to create a delivery service

export JAVA_HOME=/usr/java/jdk1.8.0_92/jre
export CATALINA_PID=/opt/traffic_router/temp/tomcat.pid
export CATALINA_HOME=/opt/tomcat
export CATALINA_BASE=/opt/traffic_router
export CATALINA_OUT=/opt/tomcat/logs/catalina.log
source /opt/traffic_router/conf/startup.properties
/opt/tomcat/bin/shutdown.sh
44 changes: 44 additions & 0 deletions infrastructure/docker/traffic_router/starttr.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#!/bin/bash
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

# Script for running the Dockerfile for Traffic Router.
# The Dockerfile sets up a Docker image which can be used for any new container;
# This script simulates the systemd unit file that is used to start traffic router on
# servers in the real world, but in Docker containers systemd is disabled.
# Therefore it is important to keep this script up to date with any changes that are
# made to traffic_router/build/build_rpm.sh and traffic_router/build/pom.xml

export JAVA_HOME=/usr/java/jdk1.8.0_92/jre
export CATALINA_PID=/opt/traffic_router/temp/tomcat.pid
export CATALINA_HOME=/opt/tomcat
export CATALINA_BASE=/opt/traffic_router
export CATALINA_OUT=/opt/tomcat/logs/catalina.log
export CATALINA_OPTS="\
-server -Xms512m -Xmx1g \
-Dlog4j.configuration=file://$CATALINA_BASE/conf/log4j.properties \
-Djava.library.path=/usr/lib64 \
-Dorg.apache.catalina.connector.Response.ENFORCE_ENCODING_IN_GET_WRITER=false \
-XX:+UseG1GC \
-XX:+UnlockExperimentalVMOptions \
-XX:InitiatingHeapOccupancyPercent=30"
export JAVA_OPTS="\
-Djava.awt.headless=true \
-Djava.security.egd=file:/dev/./urandom"

ulimit -c unlimited
/opt/tomcat/bin/startup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
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.util.net.NioEndpoint;
import org.apache.tomcat.util.net.SSLHostConfig;
import org.apache.tomcat.util.net.SSLHostConfigCertificate;
Expand Down Expand Up @@ -92,4 +93,29 @@ synchronized public void reloadSSLHosts(final Map<String, HandshakeData> cr) {
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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,62 @@

import com.comcast.cdn.traffic_control.traffic_router.shared.CertificateData;
import org.apache.log4j.Logger;
import org.bouncycastle.jcajce.provider.asymmetric.rsa.BCRSAPrivateCrtKey;
import sun.security.rsa.RSAPrivateCrtKeyImpl;
Comment thread
rawlinp marked this conversation as resolved.
import sun.security.rsa.RSAPublicKeyImpl;

import java.math.BigInteger;
import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.cert.CertificateExpiredException;
import java.security.cert.CertificateNotYetValidException;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.List;
Comment thread
rawlinp marked this conversation as resolved.
import java.util.stream.Collectors;

public class CertificateDataConverter {
private static final Logger log = Logger.getLogger(CertificateDataConverter.class);

private PrivateKeyDecoder privateKeyDecoder = new PrivateKeyDecoder();
private CertificateDecoder certificateDecoder = new CertificateDecoder();

@SuppressWarnings({"PMD.CyclomaticComplexity"})
public HandshakeData toHandshakeData(final CertificateData certificateData) {
try {
final PrivateKey privateKey = privateKeyDecoder.decode(certificateData.getCertificate().getKey());
final List<String> encodedCertificates = certificateDecoder.doubleDecode(certificateData.getCertificate().getCrt());

final List<X509Certificate> x509Chain = encodedCertificates.stream()
.map(encodedCertificate -> certificateDecoder.toCertificate(encodedCertificate))
.collect(Collectors.toList());

return new HandshakeData(certificateData.getDeliveryservice(), certificateData.getHostname(),
x509Chain.toArray(new X509Certificate[x509Chain.size()]), privateKey);
final List<X509Certificate> x509Chain = new ArrayList<>();
boolean hostMatch = false;
boolean modMatch = false;
for (final String encodedCertificate : encodedCertificates) {
final X509Certificate certificate = certificateDecoder.toCertificate(encodedCertificate);
certificate.checkValidity();
if (!hostMatch && verifySubject(certificate, certificateData.alias())) {
hostMatch = true;
}
if (!modMatch && verifyModulus(privateKey, certificate)) {
modMatch = true;
}
x509Chain.add(certificate);
}
if (hostMatch && modMatch) {
return new HandshakeData(certificateData.getDeliveryservice(), certificateData.getHostname(),
x509Chain.toArray(new X509Certificate[x509Chain.size()]), privateKey);
}
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());
}

} catch (CertificateNotYetValidException er) {
log.error("Failed to convert certificate data for delivery service = " + certificateData.getHostname()
+ ", because the certificate is not valid yet. ");
} catch (CertificateExpiredException ex ) {
log.error("Failed to convert certificate data for delivery service = " + certificateData.getHostname()
+ ", because the certificate has expired. ");
} catch (Exception e) {
log.error("Failed to convert certificate data (delivery service = " + certificateData.getDeliveryservice()
+ ", hostname = " + certificateData.getHostname() + ") from traffic ops to handshake data! "
Expand All @@ -49,6 +81,78 @@ public HandshakeData toHandshakeData(final CertificateData certificateData) {
return null;
}

public boolean verifySubject(final X509Certificate certificate, final String hostAlias ) {
final String host = certificate.getSubjectDN().getName();
if (hostCompare(hostAlias,host)) {
return true;
}

try {
// This approach is probably the only one that is JDK independent
if (certificate.getSubjectAlternativeNames() != null) {
for (final List<?> altName : certificate.getSubjectAlternativeNames()) {
if (hostCompare(hostAlias, (String) altName.get(1))) {
return true;
}
}
}
}
catch (Exception e) {
log.error("Encountered an error while validating the certificate subject for service: "+hostAlias+", " +
"error: "+e.getClass().getSimpleName()+": " + e.getMessage(), e);
return false;
}

return false;
}

private boolean hostCompare(final String hostAlias, final String subject) {
if (hostAlias.contains(subject) || subject.contains(hostAlias)) {
return true;
}
final String[] chopped = subject.split("CN=", 2);
if (chopped != null && chopped.length > 1) {
String chop = chopped[1];
chop = chop.replaceFirst("\\*\\.", ".");
chop = chop.split(",", 2)[0];
if (chop.length()>0 && (hostAlias.contains(chop) || chop.contains(hostAlias))) {
return true;
}
}
return false;
}

public boolean verifyModulus(final PrivateKey privateKey, final X509Certificate certificate) {
BigInteger privModulus = null;
if (privateKey instanceof BCRSAPrivateCrtKey) {
privModulus = ((BCRSAPrivateCrtKey) privateKey).getModulus();
} else if (privateKey instanceof RSAPrivateCrtKeyImpl) {
Comment thread
rawlinp marked this conversation as resolved.
privModulus = ((RSAPrivateCrtKeyImpl) privateKey).getModulus();
} else {
return false;
}
BigInteger pubModulus = null;
final PublicKey publicKey = certificate.getPublicKey();
if ((publicKey instanceof RSAPublicKeyImpl)) {
pubModulus = ((RSAPublicKeyImpl) publicKey).getModulus();
} else {
final String[] keyparts = publicKey.toString().split(System.getProperty("line.separator"));
for (final String part : keyparts) {
final int start = part.indexOf("modulus: ") + 9;
if (start < 9) {
continue;
} else {
pubModulus = new BigInteger(part.substring(start));
break;
}
}
}
if (privModulus.equals(pubModulus)) {
Comment thread
rawlinp marked this conversation as resolved.
return true;
}
return false;
}

public PrivateKeyDecoder getPrivateKeyDecoder() {
return privateKeyDecoder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,12 @@ synchronized public void importCertificateDataList(final List<CertificateData> c

if (!master.containsKey(alias)) {
final HandshakeData handshakeData = certificateDataConverter.toHandshakeData(certificateData);
master.put(alias, handshakeData);
if (!certificateData.equals(previousData.get(alias))) {
changes.put(alias, handshakeData);
log.warn("Imported handshake data with alias " + alias);
if (handshakeData != null) {
master.put(alias, handshakeData);
if (!certificateData.equals(previousData.get(alias))) {
changes.put(alias, handshakeData);
log.warn("Imported handshake data with alias " + alias);
}
}
}
else {
Expand All @@ -103,7 +105,7 @@ synchronized public void importCertificateDataList(final List<CertificateData> c
previousData.clear();
for (final CertificateData certificateData : certificateDataList) {
final String alias = certificateData.alias();
if (!previousData.containsKey(alias)) {
if (!previousData.containsKey(alias) && master.containsKey(alias)) {
Comment thread
rawlinp marked this conversation as resolved.
previousData.put(alias, certificateData);
}
}
Expand Down
Loading