dneuman64 closed pull request #3072: Emit warning when adding self-signed certs
using the API rather than prohibiting them
URL: https://github.com/apache/trafficcontrol/pull/3072
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/lib/go-tc/constants.go b/lib/go-tc/constants.go
index a72aefbcb..9f7cee834 100644
--- a/lib/go-tc/constants.go
+++ b/lib/go-tc/constants.go
@@ -42,7 +42,7 @@ const (
ErrorLevel
)
-var alertLevels = [4]string{"success", "info", "warn", "error"}
+var alertLevels = [4]string{"success", "info", "warning", "error"}
func (a AlertLevel) String() string {
return alertLevels[a]
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go
b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go
index 0c5a1a9ee..3dd2577e2 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go
@@ -60,7 +60,7 @@ func AddSSLKeys(w http.ResponseWriter, r *http.Request) {
api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
- certChain, err := verifyCertificate(req.Certificate.Crt, "")
+ certChain, isUnknownAuth, err := verifyCertificate(req.Certificate.Crt,
"")
if err != nil {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest,
errors.New("verifying certificate: "+err.Error()), nil)
return
@@ -83,6 +83,10 @@ func AddSSLKeys(w http.ResponseWriter, r *http.Request) {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError,
nil, errors.New("adding SSL keys to delivery service
'"+*req.DeliveryService+"': "+err.Error()))
return
}
+ if isUnknownAuth {
+ api.WriteRespAlert(w, r, tc.WarnLevel, "WARNING: SSL keys were
successfully added for '"+*req.DeliveryService+"', but the certificate is
signed by an unknown authority and may be invalid")
+ return
+ }
api.WriteResp(w, r, "Successfully added ssl keys for
"+*req.DeliveryService)
}
@@ -268,24 +272,26 @@ WHERE r.pattern = $2
// verify the server certificate chain and return the
// certificate and its chain in the proper order. Returns a verified
// and ordered certificate and CA chain.
-func verifyCertificate(certificate string, rootCA string) (string, error) {
+// If the cert verification returns UnknownAuthorityError, return true to
+// indicate that the certs are signed by an unknown authority (e.g.
self-signed).
+func verifyCertificate(certificate string, rootCA string) (string, bool,
error) {
// decode, verify, and order certs for storage
certs := strings.SplitAfter(certificate, PemCertEndMarker)
if len(certs) <= 1 {
- return "", errors.New("no certificate chain to verify")
+ return "", false, errors.New("no certificate chain to verify")
}
// decode and verify the server certificate
block, _ := pem.Decode([]byte(certs[0]))
if block == nil {
- return "", errors.New("could not decode pem-encoded server
certificate")
+ return "", false, errors.New("could not decode pem-encoded
server certificate")
}
cert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
- return "", errors.New("could not parse the server certificate:
" + err.Error())
+ return "", false, errors.New("could not parse the server
certificate: " + err.Error())
}
if !(cert.KeyUsage&x509.KeyUsageKeyEncipherment > 0) {
- return "", errors.New("no key encipherment usage for the server
certificate")
+ return "", false, errors.New("no key encipherment usage for the
server certificate")
}
bundle := ""
for i := 0; i < len(certs)-1; i++ {
@@ -294,7 +300,7 @@ func verifyCertificate(certificate string, rootCA string)
(string, error) {
intermediatePool := x509.NewCertPool()
if !intermediatePool.AppendCertsFromPEM([]byte(bundle)) {
- return "", errors.New("certificate CA bundle is empty")
+ return "", false, errors.New("certificate CA bundle is empty")
}
opts := x509.VerifyOptions{
@@ -304,17 +310,20 @@ func verifyCertificate(certificate string, rootCA string)
(string, error) {
// verify the certificate chain.
rootPool := x509.NewCertPool()
if !rootPool.AppendCertsFromPEM([]byte(rootCA)) {
- return "", errors.New("unable to parse root CA
certificate")
+ return "", false, errors.New("unable to parse root CA
certificate")
}
opts.Roots = rootPool
}
chain, err := cert.Verify(opts)
if err != nil {
- return "", errors.New("could not verify the certificate chain:
" + err.Error())
+ if _, ok := err.(x509.UnknownAuthorityError); ok {
+ return certificate, true, nil
+ }
+ return "", false, errors.New("could not verify the certificate
chain: " + err.Error())
}
if len(chain) < 1 {
- return "", errors.New("can't find valid chain for cert in file
in request")
+ return "", false, errors.New("can't find valid chain for cert
in file in request")
}
pemEncodedChain := ""
for _, link := range chain[0] {
@@ -325,5 +334,5 @@ func verifyCertificate(certificate string, rootCA string)
(string, error) {
}
}
- return pemEncodedChain, nil
+ return pemEncodedChain, false, nil
}
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/keys_test.go
b/traffic_ops/traffic_ops_golang/deliveryservice/keys_test.go
index 14b30a393..1bc2bd250 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/keys_test.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/keys_test.go
@@ -27,7 +27,7 @@ import (
const (
BadData = "This is bad data and it is not pem encoded"
- SelfSigneCertOnly = `
+ SelfSignedCertOnly = `
-----BEGIN CERTIFICATE-----
MIIDkjCCAnoCCQCfwd219JKpUDANBgkqhkiG9w0BAQsFADCBijELMAkGA1UEBhMC
VVMxETAPBgNVBAgMCENvbG9yYWRvMQ8wDQYDVQQHDAZEZW52ZXIxEDAOBgNVBAoM
@@ -199,19 +199,28 @@ OEUjfakK71+V/HbQt477zR4k7cRbiA==
func TestVerifyAndEncodeCertificate(t *testing.T) {
// should fail bad base64 data
- dat, err := verifyCertificate(BadData, "")
+ dat, _, err := verifyCertificate(BadData, "")
if err == nil {
t.Errorf("Unexpected result, there should have been a base64
decoding failure")
}
- // should fail, can't verify self signed cert
- dat, err = verifyCertificate(SelfSigneCertOnly, rootCA)
+ // should fail, can't verify self signed cert against this rootCA
+ dat, _, err = verifyCertificate(SelfSignedCertOnly, rootCA)
if err == nil {
t.Errorf("Unexpected result, a certificate verification error
should have occured")
}
+ // should pass, unknown authority is just a warning not an error
+ dat, unknownAuth, err := verifyCertificate(GoodTLSKeys, "")
+ if err != nil {
+ t.Errorf("Test failure: %s", err)
+ }
+ if !unknownAuth {
+ t.Errorf("Unexpected result, certificate verification should
have detected unknown authority")
+ }
+
// should pass
- dat, err = verifyCertificate(GoodTLSKeys, rootCA)
+ dat, _, err = verifyCertificate(GoodTLSKeys, rootCA)
if err != nil {
t.Errorf("Test failure: %s", err)
}
diff --git a/traffic_portal/app/src/common/api/DeliveryServiceSslKeysService.js
b/traffic_portal/app/src/common/api/DeliveryServiceSslKeysService.js
index 46a8ec009..31f1631a6 100644
--- a/traffic_portal/app/src/common/api/DeliveryServiceSslKeysService.js
+++ b/traffic_portal/app/src/common/api/DeliveryServiceSslKeysService.js
@@ -60,7 +60,7 @@ var DeliveryServiceSslKeysService = function($http, $q,
locationUtils, messageMo
$http.post(ENV.api['root'] + "deliveryservices/sslkeys/add", sslKeys)
.then(
function(result) {
- messageModel.setMessages([ { level: 'success', text: 'SSL Keys
updated for ' + deliveryService.xmlId } ], false);
+ messageModel.setMessages(result.data.alerts, false);
request.resolve(result.data.response);
},
function(fault) {
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
