diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go index 7a820146ea..1923d3eebd 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/keys.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/keys.go @@ -59,7 +59,7 @@ func AddSSLKeys(w http.ResponseWriter, r *http.Request) { api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) return } - certChain, isUnknownAuth, err := verifyCertificate(req.Certificate.Crt, "") + certChain, isUnknownAuth, isVerifiedChainNotEqual, 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,7 +83,11 @@ func AddSSLKeys(w http.ResponseWriter, r *http.Request) { 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") + api.WriteRespAlert(w, r, tc.WarnLevel, "WARNING: SSL keys were successfully added for '"+*req.DeliveryService+"', but the input certificate may be invalid (certificate is signed by an unknown authority)") + return + } + if isVerifiedChainNotEqual { + api.WriteRespAlert(w, r, tc.WarnLevel, "WARNING: SSL keys were successfully added for '"+*req.DeliveryService+"', but the input certificate may be invalid (certificate verification produced a different chain)") return } api.WriteResp(w, r, "Successfully added ssl keys for "+*req.DeliveryService) @@ -272,25 +276,27 @@ WHERE r.pattern = $2 // certificate and its chain in the proper order. Returns a verified // and ordered certificate and CA chain. // 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) { +// indicate that the certs are signed by an unknown authority (e.g. self-signed). Otherwise, return false. +// If the chain returned from Certificate.Verify() does not match the input chain, +// return true. Otherwise, return false. +func verifyCertificate(certificate string, rootCA string) (string, bool, bool, error) { // decode, verify, and order certs for storage certs := strings.SplitAfter(certificate, PemCertEndMarker) if len(certs) <= 1 { - return "", false, errors.New("no certificate chain to verify") + return "", false, false, errors.New("no certificate chain to verify") } // decode and verify the server certificate block, _ := pem.Decode([]byte(certs[0])) if block == nil { - return "", false, errors.New("could not decode pem-encoded server certificate") + return "", false, false, errors.New("could not decode pem-encoded server certificate") } cert, err := x509.ParseCertificate(block.Bytes) if err != nil { - return "", false, errors.New("could not parse the server certificate: " + err.Error()) + return "", false, false, errors.New("could not parse the server certificate: " + err.Error()) } if !(cert.KeyUsage&x509.KeyUsageKeyEncipherment > 0) { - return "", false, errors.New("no key encipherment usage for the server certificate") + return "", false, false, errors.New("no key encipherment usage for the server certificate") } bundle := "" for i := 0; i < len(certs)-1; i++ { @@ -299,7 +305,7 @@ func verifyCertificate(certificate string, rootCA string) (string, bool, error) intermediatePool := x509.NewCertPool() if !intermediatePool.AppendCertsFromPEM([]byte(bundle)) { - return "", false, errors.New("certificate CA bundle is empty") + return "", false, false, errors.New("certificate CA bundle is empty") } opts := x509.VerifyOptions{ @@ -309,7 +315,7 @@ func verifyCertificate(certificate string, rootCA string) (string, bool, error) // verify the certificate chain. rootPool := x509.NewCertPool() if !rootPool.AppendCertsFromPEM([]byte(rootCA)) { - return "", false, errors.New("unable to parse root CA certificate") + return "", false, false, errors.New("unable to parse root CA certificate") } opts.Roots = rootPool } @@ -317,12 +323,12 @@ func verifyCertificate(certificate string, rootCA string) (string, bool, error) chain, err := cert.Verify(opts) if err != nil { if _, ok := err.(x509.UnknownAuthorityError); ok { - return certificate, true, nil + return certificate, true, false, nil } - return "", false, errors.New("could not verify the certificate chain: " + err.Error()) + return "", false, false, errors.New("could not verify the certificate chain: " + err.Error()) } if len(chain) < 1 { - return "", false, errors.New("can't find valid chain for cert in file in request") + return "", false, false, errors.New("can't find valid chain for cert in file in request") } pemEncodedChain := "" for _, link := range chain[0] { @@ -332,8 +338,12 @@ func verifyCertificate(certificate string, rootCA string) (string, bool, error) } if len(pemEncodedChain) < 1 { - return "", false, errors.New("Invalid empty certicate chain in request") + return "", false, false, errors.New("Invalid empty certicate chain in request") + } + + if pemEncodedChain != certificate { + return certificate, false, true, nil } - return pemEncodedChain, false, nil + return certificate, false, 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 659d221814..24c11f5c5e 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/keys_test.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/keys_test.go @@ -199,19 +199,19 @@ 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 against this rootCA - dat, _, err = verifyCertificate(SelfSignedCertOnly, 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, "") + dat, unknownAuth, _, err := verifyCertificate(GoodTLSKeys, "") if err != nil { t.Errorf("Test failure: %s", err) } @@ -220,7 +220,7 @@ func TestVerifyAndEncodeCertificate(t *testing.T) { } // should pass - dat, _, err = verifyCertificate(GoodTLSKeys, rootCA) + dat, _, _, err = verifyCertificate(GoodTLSKeys, rootCA) if err != nil { t.Errorf("Test failure: %s", err) } @@ -230,4 +230,9 @@ func TestVerifyAndEncodeCertificate(t *testing.T) { if length != 3 { // rootCA now included in certChain t.Errorf("Test failure: expected 2 certs from verifyCertificate(), got: %d ", length) } + + // certificate returned from verifyCertificate should always be the same as the input certificate + if dat != GoodTLSKeys { + t.Errorf("expected input certificate to match the output certificate") + } }