You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2018/12/07 20:49:10 UTC
[GitHub] dneuman64 closed pull request #3072: Emit warning when adding
self-signed certs using the API rather than prohibiting them
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:
users@infra.apache.org
With regards,
Apache Git Services