[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #3382: Add enhanced X509 Certificate/Private RSA Key validation to Traffic Ops (AddSSLKeys Endpoint)

[GitBox <gi...@apache.org>]

rawlinp commented on a change in pull request #3382: Add enhanced X509 Certificate/Private RSA Key validation to Traffic Ops (AddSSLKeys Endpoint)
URL: https://github.com/apache/trafficcontrol/pull/3382#discussion_r263600095
 
 

 ##########
 File path: traffic_ops/traffic_ops_golang/deliveryservice/keys.go
 ##########
 @@ -268,29 +273,107 @@ WHERE r.pattern = $2
 	return xmlID, true, nil
 }
 
+// decode the private key
+// check for proper algorithm.
+// check for correct number of keys
+// return private key object, cleaned private key PEM, or any errors.
+func decodePrivateKey(pemPrivateKey string) (*rsa.PrivateKey, string, error) {
+
+	// Remove any white space before decoding
+	trimmedPrivateKey := strings.TrimSpace(pemPrivateKey)
+
+	// Check for proper key count before attempting to decode.
+	blockCount := strings.Count(trimmedPrivateKey, "\n-----END")
+	if blockCount < 1 {
+		return nil, "", errors.New("no private keys found")
+	} else if blockCount > 1 {
+		return nil, "", errors.New("multiple private keys found")
+	}
+
+	// Attempt to decode pem encoded text into PEM block.
+	block, _ := pem.Decode([]byte(trimmedPrivateKey))
+	if block == nil {
+		return nil, "", errors.New("could not decode pem-encoded private key")
+	}
+
+	// Check for encrypted keys or other unsupported key types
+	if strings.Contains(block.Type, "ENCRYPTED") {
+		return nil, "", errors.New("encrypted private key not supported")
+	}
+
+	// Check block headers for encryption.
+	for _, value := range block.Headers {
+		if strings.Contains(value, "ENCRYPTED") {
+			return nil, "", errors.New("encrypted private key not supported")
+		}
+	}
+
+	// Attempt to parse PEM block as a PKCS#1 or PKCS#8 formatted RSA Private Key.
+	if privateKey, err := x509.ParsePKCS8PrivateKey(block.Bytes); privateKey != nil && err == nil {
 
 Review comment:
   Not really, if there is an error parsing the first type of key, it will attempt parsing the second type of key. _Then_ if both failed to parse then it returns that "unk" error.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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