Skip to content

Commit

Permalink
Sanitize TLS config that has key bundled with cert
Browse files Browse the repository at this point in the history
Modify the extended validation logic to copy any private key found in the
certificate data of a route's TLS configuration to its key data.

Extended validation validates a route's TLS configuration using any key
that is specified in either the TLS configuration's certificate data or its
key data.  As a result, a route that specifies the certificate and key
together in the certificate data may pass extended validation and be
admitted.  However, the certificate manager only wrote the certificate out
if the TLS configuration had nonempty key data.  As a result, a route with
a valid certificate and key could be admitted but the certificate and key
not written out, which would cause HAProxy to fail to load.

This commit fixes bug 1843856.

https://bugzilla.redhat.com/show_bug.cgi?id=1843856

* pkg/router/routeapihelpers/validation.go (splitCertKey): New function.
Take sanitized PEM data and split it into public and private parts.
(ExtendedValidateRoute): Use splitCertKey to parse out any private parts
that are in the certificate data.  Prepend any private parts found in the
certificate data to the TLS configuration's key.
* pkg/router/routeapihelpers/validation_test.go: Add test case.
  • Loading branch information
Miciah committed Jun 5, 2020
1 parent d7c7dea commit 74185d8
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 11 deletions.
60 changes: 49 additions & 11 deletions pkg/router/routeapihelpers/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,33 @@ func sanitizePEM(data []byte) ([]byte, error) {
return buf.Bytes(), nil
}

// splitCertKey takes a slice of bytes containing sanitized PEM data and returns
// two slices of bytes containing PEM data: one slice with the public
// certificate block or blocks from the input PEM data and one slice with any
// private key blocks.
func splitCertKey(data []byte) ([]byte, []byte, error) {
var block *pem.Block
publicBuf := &bytes.Buffer{}
privateBuf := &bytes.Buffer{}
for len(data) > 0 {
block, data = pem.Decode(data)
if block == nil {
break
}
switch block.Type {
case "PUBLIC KEY", "CERTIFICATE":
if err := pem.Encode(publicBuf, block); err != nil {
return nil, nil, err
}
case "EC PRIVATE KEY", "RSA PRIVATE KEY":
if err := pem.Encode(privateBuf, block); err != nil {
return nil, nil, err
}
}
}
return publicBuf.Bytes(), privateBuf.Bytes(), nil
}

// ExtendedValidateRoute performs an extended validation on the route
// including checking that the TLS config is valid. It also sanitizes
// the contents of valid certificates by removing any data that
Expand Down Expand Up @@ -179,17 +206,6 @@ func ExtendedValidateRoute(route *routev1.Route) field.ErrorList {
tlsConfig.Certificate = string(data)
}
}

certKeyBytes := []byte{}
certKeyBytes = append(certKeyBytes, []byte(tlsConfig.Certificate)...)
if len(tlsConfig.Key) > 0 {
certKeyBytes = append(certKeyBytes, byte('\n'))
certKeyBytes = append(certKeyBytes, []byte(tlsConfig.Key)...)
}

if _, err := tls.X509KeyPair(certKeyBytes, certKeyBytes); err != nil {
result = append(result, field.Invalid(tlsFieldPath.Child("key"), "redacted key data", err.Error()))
}
}

if len(tlsConfig.Key) > 0 {
Expand All @@ -200,6 +216,28 @@ func ExtendedValidateRoute(route *routev1.Route) field.ErrorList {
}
}

if len(tlsConfig.Certificate) > 0 {
if certBytes, keyBytes, err := splitCertKey([]byte(tlsConfig.Certificate)); err != nil {
result = append(result, field.Invalid(tlsFieldPath.Child("certificate"), "redacted key data", err.Error()))
} else {
if len(tlsConfig.Key) > 0 {
if len(keyBytes) > 0 {
keyBytes = append(keyBytes, byte('\n'))
}
keyBytes = append(keyBytes, []byte(tlsConfig.Key)...)
}
if len(keyBytes) == 0 {
result = append(result, field.Invalid(tlsFieldPath.Child("key"), "", "no key specified"))
} else {
if _, err := tls.X509KeyPair(certBytes, keyBytes); err != nil {
result = append(result, field.Invalid(tlsFieldPath.Child("key"), "redacted key data", err.Error()))
} else {
tlsConfig.Certificate, tlsConfig.Key = string(certBytes), string(keyBytes)
}
}
}
}

if len(tlsConfig.DestinationCACertificate) > 0 {
if _, err := cert.ParseCertsPEM([]byte(tlsConfig.DestinationCACertificate)); err != nil {
errmsg := fmt.Sprintf("failed to parse destination CA certificate: %v", err)
Expand Down
21 changes: 21 additions & 0 deletions pkg/router/routeapihelpers/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1624,6 +1624,27 @@ func TestExtendedValidateRoute(t *testing.T) {
},
expectedErrors: 0,
},
{
name: "bz1843856",
route: &routev1.Route{
Spec: routev1.RouteSpec{
TLS: &routev1.TLSConfig{
Termination: routev1.TLSTerminationEdge,
Certificate: testCertificate + "\n" + testPrivateKey,
},
},
},
expectRoute: &routev1.Route{
Spec: routev1.RouteSpec{
TLS: &routev1.TLSConfig{
Termination: routev1.TLSTerminationEdge,
Certificate: testCertificate + "\n",
Key: testPrivateKey + "\n",
},
},
},
expectedErrors: 0,
},
}

for _, tc := range tests {
Expand Down

0 comments on commit 74185d8

Please sign in to comment.