Pull request: tls: better err msg for ed25519

Updates #3737.

Squashed commit of the following:

commit 4dedd4690c49d7cbfd8c8e5d5b4c34d1a90705f1
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Nov 1 13:23:18 2021 +0300

    tls: better err msg for ed25519
This commit is contained in:
Ainar Garipov 2021-11-01 13:33:12 +03:00
parent 1e72960140
commit 2fc108486d
2 changed files with 25 additions and 8 deletions

View file

@ -46,6 +46,8 @@ and this project adheres to
### Changed ### Changed
- Better error message for ED25519 private keys, which are not widely supported
([#3737]).
- Cache now follows RFC more closely for negative answers ([#3707]). - Cache now follows RFC more closely for negative answers ([#3707]).
- `$dnsrewrite` rules and other DNS rewrites will now be applied even when the - `$dnsrewrite` rules and other DNS rewrites will now be applied even when the
protection is disabled ([#1558]). protection is disabled ([#1558]).
@ -119,7 +121,6 @@ In this release, the schema version has changed from 10 to 12.
### Fixed ### Fixed
- ED25519 private key validation ([#3737]).
- Incorrect assignment of explicitly configured DHCP options ([#3744]). - Incorrect assignment of explicitly configured DHCP options ([#3744]).
- Occasional panic during shutdown ([#3655]). - Occasional panic during shutdown ([#3655]).
- Addition of IPs into only one as opposed to all matching ipsets on Linux - Addition of IPs into only one as opposed to all matching ipsets on Linux

View file

@ -450,27 +450,36 @@ func validatePkey(data *tlsConfigStatus, pkey string) error {
if decoded == nil { if decoded == nil {
break break
} }
if decoded.Type == "PRIVATE KEY" || strings.HasSuffix(decoded.Type, " PRIVATE KEY") { if decoded.Type == "PRIVATE KEY" || strings.HasSuffix(decoded.Type, " PRIVATE KEY") {
key = decoded key = decoded
break break
} }
} }
if key == nil { if key == nil {
data.WarningValidation = "No valid keys were found" data.WarningValidation = "No valid keys were found"
return errors.Error(data.WarningValidation) return errors.Error(data.WarningValidation)
} }
// parse the decoded key // parse the decoded key
_, keytype, err := parsePrivateKey(key.Bytes) _, keyType, err := parsePrivateKey(key.Bytes)
if err != nil { if err != nil {
data.WarningValidation = fmt.Sprintf("Failed to parse private key: %s", err) data.WarningValidation = fmt.Sprintf("Failed to parse private key: %s", err)
return errors.Error(data.WarningValidation)
} else if keyType == keyTypeED25519 {
data.WarningValidation = "ED25519 keys are not supported by browsers; " +
"did you mean to use X25519 for key exchange?"
return errors.Error(data.WarningValidation) return errors.Error(data.WarningValidation)
} }
data.ValidKey = true data.ValidKey = true
data.KeyType = keytype data.KeyType = keyType
return nil return nil
} }
@ -508,6 +517,13 @@ func validateCertificates(certChain, pkey, serverName string) tlsConfigStatus {
return data return data
} }
// Key types.
const (
keyTypeECDSA = "ECDSA"
keyTypeED25519 = "ED25519"
keyTypeRSA = "RSA"
)
// Attempt to parse the given private key DER block. OpenSSL 0.9.8 generates // Attempt to parse the given private key DER block. OpenSSL 0.9.8 generates
// PKCS#1 private keys by default, while OpenSSL 1.0.0 generates PKCS#8 keys. // PKCS#1 private keys by default, while OpenSSL 1.0.0 generates PKCS#8 keys.
// OpenSSL ecparam generates SEC1 EC private keys for ECDSA. We try all three. // OpenSSL ecparam generates SEC1 EC private keys for ECDSA. We try all three.
@ -516,17 +532,17 @@ func validateCertificates(certChain, pkey, serverName string) tlsConfigStatus {
// is actually necessary. // is actually necessary.
func parsePrivateKey(der []byte) (key crypto.PrivateKey, typ string, err error) { func parsePrivateKey(der []byte) (key crypto.PrivateKey, typ string, err error) {
if key, err = x509.ParsePKCS1PrivateKey(der); err == nil { if key, err = x509.ParsePKCS1PrivateKey(der); err == nil {
return key, "RSA", nil return key, keyTypeRSA, nil
} }
if key, err = x509.ParsePKCS8PrivateKey(der); err == nil { if key, err = x509.ParsePKCS8PrivateKey(der); err == nil {
switch key := key.(type) { switch key := key.(type) {
case *rsa.PrivateKey: case *rsa.PrivateKey:
return key, "RSA", nil return key, keyTypeRSA, nil
case *ecdsa.PrivateKey: case *ecdsa.PrivateKey:
return key, "ECDSA", nil return key, keyTypeECDSA, nil
case ed25519.PrivateKey: case ed25519.PrivateKey:
return key, "ED25519", nil return key, keyTypeED25519, nil
default: default:
return nil, "", fmt.Errorf( return nil, "", fmt.Errorf(
"tls: found unknown private key type %T in PKCS#8 wrapping", "tls: found unknown private key type %T in PKCS#8 wrapping",
@ -536,7 +552,7 @@ func parsePrivateKey(der []byte) (key crypto.PrivateKey, typ string, err error)
} }
if key, err = x509.ParseECPrivateKey(der); err == nil { if key, err = x509.ParseECPrivateKey(der); err == nil {
return key, "ECDSA", nil return key, keyTypeECDSA, nil
} }
return nil, "", errors.Error("tls: failed to parse private key") return nil, "", errors.Error("tls: failed to parse private key")