From 2fc108486dc7029cde487dee7b987531396db95f Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Mon, 1 Nov 2021 13:33:12 +0300 Subject: [PATCH] Pull request: tls: better err msg for ed25519 Updates #3737. Squashed commit of the following: commit 4dedd4690c49d7cbfd8c8e5d5b4c34d1a90705f1 Author: Ainar Garipov Date: Mon Nov 1 13:23:18 2021 +0300 tls: better err msg for ed25519 --- CHANGELOG.md | 3 ++- internal/home/tls.go | 30 +++++++++++++++++++++++------- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa504b32..fd40945e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,8 @@ and this project adheres to ### Changed +- Better error message for ED25519 private keys, which are not widely supported + ([#3737]). - Cache now follows RFC more closely for negative answers ([#3707]). - `$dnsrewrite` rules and other DNS rewrites will now be applied even when the protection is disabled ([#1558]). @@ -119,7 +121,6 @@ In this release, the schema version has changed from 10 to 12. ### Fixed -- ED25519 private key validation ([#3737]). - Incorrect assignment of explicitly configured DHCP options ([#3744]). - Occasional panic during shutdown ([#3655]). - Addition of IPs into only one as opposed to all matching ipsets on Linux diff --git a/internal/home/tls.go b/internal/home/tls.go index fc842929..c334e2e1 100644 --- a/internal/home/tls.go +++ b/internal/home/tls.go @@ -450,27 +450,36 @@ func validatePkey(data *tlsConfigStatus, pkey string) error { if decoded == nil { break } + if decoded.Type == "PRIVATE KEY" || strings.HasSuffix(decoded.Type, " PRIVATE KEY") { key = decoded + break } } if key == nil { data.WarningValidation = "No valid keys were found" + return errors.Error(data.WarningValidation) } // parse the decoded key - _, keytype, err := parsePrivateKey(key.Bytes) + _, keyType, err := parsePrivateKey(key.Bytes) if err != nil { 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) } data.ValidKey = true - data.KeyType = keytype + data.KeyType = keyType + return nil } @@ -508,6 +517,13 @@ func validateCertificates(certChain, pkey, serverName string) tlsConfigStatus { 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 // 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. @@ -516,17 +532,17 @@ func validateCertificates(certChain, pkey, serverName string) tlsConfigStatus { // is actually necessary. func parsePrivateKey(der []byte) (key crypto.PrivateKey, typ string, err error) { if key, err = x509.ParsePKCS1PrivateKey(der); err == nil { - return key, "RSA", nil + return key, keyTypeRSA, nil } if key, err = x509.ParsePKCS8PrivateKey(der); err == nil { switch key := key.(type) { case *rsa.PrivateKey: - return key, "RSA", nil + return key, keyTypeRSA, nil case *ecdsa.PrivateKey: - return key, "ECDSA", nil + return key, keyTypeECDSA, nil case ed25519.PrivateKey: - return key, "ED25519", nil + return key, keyTypeED25519, nil default: return nil, "", fmt.Errorf( "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 { - return key, "ECDSA", nil + return key, keyTypeECDSA, nil } return nil, "", errors.Error("tls: failed to parse private key")