From d218e047a3de91895b873d627de557722c84814c Mon Sep 17 00:00:00 2001 From: Simon Zolin Date: Wed, 27 Feb 2019 13:07:29 +0300 Subject: [PATCH 1/9] + add tests for validateCertificates() --- control_test.go | 81 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 control_test.go diff --git a/control_test.go b/control_test.go new file mode 100644 index 00000000..725457b9 --- /dev/null +++ b/control_test.go @@ -0,0 +1,81 @@ +package main + +import ( + "testing" + "time" +) + +/* Tests performed: +. Bad certificate +. Bad private key +. Valid certificate & private key */ +func TestValidateCertificates(t *testing.T) { + var data tlsConfig + + // bad cert + data.CertificateChain = "bad cert" + data.PrivateKey = "" + data = validateCertificates(data) + if !(data.WarningValidation != "" && + !data.ValidCert && + !data.ValidChain) { + t.Fatalf("bad cert: validateCertificates(): %v", data) + } + + // bad priv key + data.CertificateChain = "" + data.PrivateKey = "bad priv key" + data = validateCertificates(data) + if !(data.WarningValidation != "" && + !data.ValidKey) { + t.Fatalf("bad priv key: validateCertificates(): %v", data) + } + + // valid cert & priv key + data.CertificateChain = `-----BEGIN CERTIFICATE----- +MIICKzCCAZSgAwIBAgIJAMT9kPVJdM7LMA0GCSqGSIb3DQEBCwUAMC0xFDASBgNV +BAoMC0FkR3VhcmQgTHRkMRUwEwYDVQQDDAxBZEd1YXJkIEhvbWUwHhcNMTkwMjI3 +MDkyNDIzWhcNNDYwNzE0MDkyNDIzWjAtMRQwEgYDVQQKDAtBZEd1YXJkIEx0ZDEV +MBMGA1UEAwwMQWRHdWFyZCBIb21lMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKB +gQCwvwUnPJiOvLcOaWmGu6Y68ksFr13nrXBcsDlhxlXy8PaohVi3XxEmt2OrVjKW +QFw/bdV4fZ9tdWFAVRRkgeGbIZzP7YBD1Ore/O5SQ+DbCCEafvjJCcXQIrTeKFE6 +i9G3aSMHs0Pwq2LgV8U5mYotLrvyFiE8QPInJbDDMpaFYwIDAQABo1MwUTAdBgNV +HQ4EFgQUdLUmQpEqrhn4eKO029jYd2AAZEQwHwYDVR0jBBgwFoAUdLUmQpEqrhn4 +eKO029jYd2AAZEQwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOBgQB8 +LwlXfbakf7qkVTlCNXgoY7RaJ8rJdPgOZPoCTVToEhT6u/cb1c2qp8QB0dNExDna +b0Z+dnODTZqQOJo6z/wIXlcUrnR4cQVvytXt8lFn+26l6Y6EMI26twC/xWr+1swq +Muj4FeWHVDerquH4yMr1jsYLD3ci+kc5sbIX6TfVxQ== +-----END CERTIFICATE-----` + data.PrivateKey = `-----BEGIN PRIVATE KEY----- +MIICeAIBADANBgkqhkiG9w0BAQEFAASCAmIwggJeAgEAAoGBALC/BSc8mI68tw5p +aYa7pjrySwWvXeetcFywOWHGVfLw9qiFWLdfESa3Y6tWMpZAXD9t1Xh9n211YUBV +FGSB4ZshnM/tgEPU6t787lJD4NsIIRp++MkJxdAitN4oUTqL0bdpIwezQ/CrYuBX +xTmZii0uu/IWITxA8iclsMMyloVjAgMBAAECgYEAmjzoG1h27UDkIlB9BVWl95TP +QVPLB81D267xNFDnWk1Lgr5zL/pnNjkdYjyjgpkBp1yKyE4gHV4skv5sAFWTcOCU +QCgfPfUn/rDFcxVzAdJVWAa/CpJNaZgjTPR8NTGU+Ztod+wfBESNCP5tbnuw0GbL +MuwdLQJGbzeJYpsNysECQQDfFHYoRNfgxHwMbX24GCoNZIgk12uDmGTA9CS5E+72 +9t3V1y4CfXxSkfhqNbd5RWrUBRLEw9BKofBS7L9NMDKDAkEAytQoIueE1vqEAaRg +a3A1YDUekKesU5wKfKfKlXvNgB7Hwh4HuvoQS9RCvVhf/60Dvq8KSu6hSjkFRquj +FQ5roQJBAMwKwyiCD5MfJPeZDmzcbVpiocRQ5Z4wPbffl9dRTDnIA5AciZDthlFg +An/jMjZSMCxNl6UyFcqt5Et1EGVhuFECQQCZLXxaT+qcyHjlHJTMzuMgkz1QFbEp +O5EX70gpeGQMPDK0QSWpaazg956njJSDbNCFM4BccrdQbJu1cW4qOsfBAkAMgZuG +O88slmgTRHX4JGFmy3rrLiHNI2BbJSuJ++Yllz8beVzh6NfvuY+HKRCmPqoBPATU +kXS9jgARhhiWXJrk +-----END PRIVATE KEY-----` + data = validateCertificates(data) + notBefore, _ := time.Parse(time.RFC3339, "2019-02-27T09:24:23Z") + notAfter, _ := time.Parse(time.RFC3339, "2046-07-14T09:24:23Z") + if !(data.WarningValidation != "" /* self signed */ && + data.ValidCert && + !data.ValidChain && + data.ValidKey && + data.KeyType == "RSA" && + data.Subject == "CN=AdGuard Home,O=AdGuard Ltd" && + data.Issuer == "CN=AdGuard Home,O=AdGuard Ltd" && + data.NotBefore == notBefore && + data.NotAfter == notAfter && + // data.DNSNames[0] == && + data.usable) { + t.Fatalf("valid cert & priv key: validateCertificates(): %v", data) + } +} From 766fbab0718f2b02fb3864e521820033ab015e36 Mon Sep 17 00:00:00 2001 From: Simon Zolin Date: Wed, 27 Feb 2019 14:11:41 +0300 Subject: [PATCH 2/9] * validateCertificates(): change input parameters; added short description --- app.go | 4 ++-- control.go | 31 +++++++++++++++++-------------- control_test.go | 16 ++++++---------- 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/app.go b/app.go index 0b55b6fa..07c67916 100644 --- a/app.go +++ b/app.go @@ -178,13 +178,13 @@ func run(args options) { } address := net.JoinHostPort(config.BindHost, strconv.Itoa(config.TLS.PortHTTPS)) // validate current TLS config and update warnings (it could have been loaded from file) - data := validateCertificates(config.TLS) + data := validateCertificates(config.TLS.CertificateChain, config.TLS.PrivateKey, config.TLS.ServerName) if !data.usable { log.Fatal(data.WarningValidation) os.Exit(1) } config.Lock() - config.TLS = data // update warnings + config.TLS.tlsConfigStatus = data // update warnings config.Unlock() // prepare certs for HTTPS server diff --git a/control.go b/control.go index fd1759e3..f6b4bab1 100644 --- a/control.go +++ b/control.go @@ -1025,7 +1025,7 @@ func handleTLSValidate(w http.ResponseWriter, r *http.Request) { } } - data = validateCertificates(data) + data.tlsConfigStatus = validateCertificates(data.CertificateChain, data.PrivateKey, data.ServerName) marshalTLS(w, data) } @@ -1051,7 +1051,7 @@ func handleTLSConfigure(w http.ResponseWriter, r *http.Request) { } restartHTTPS := false - data = validateCertificates(data) + data.tlsConfigStatus = validateCertificates(data.CertificateChain, data.PrivateKey, data.ServerName) if !reflect.DeepEqual(config.TLS.tlsConfigSettings, data.tlsConfigSettings) { log.Printf("tls config settings have changed, will restart HTTPS server") restartHTTPS = true @@ -1078,21 +1078,24 @@ func handleTLSConfigure(w http.ResponseWriter, r *http.Request) { } } -func validateCertificates(data tlsConfig) tlsConfig { +/* Process certificate data and its private key. +CertificateChain, PrivateKey parameters are optional. +On error, return partially set object + with 'WarningValidation' field containing error description. +*/ +func validateCertificates(CertificateChain, PrivateKey, ServerName string) tlsConfigStatus { var err error - - // clear out status for certificates - data.tlsConfigStatus = tlsConfigStatus{} + var data tlsConfigStatus // check only public certificate separately from the key - if data.CertificateChain != "" { - log.Tracef("got certificate: %s", data.CertificateChain) + if CertificateChain != "" { + log.Tracef("got certificate: %s", CertificateChain) // now do a more extended validation var certs []*pem.Block // PEM-encoded certificates var skippedBytes []string // skipped bytes - pemblock := []byte(data.CertificateChain) + pemblock := []byte(CertificateChain) for { var decoded *pem.Block decoded, pemblock = pem.Decode(pemblock) @@ -1127,7 +1130,7 @@ func validateCertificates(data tlsConfig) tlsConfig { // spew.Dump(parsedCerts) opts := x509.VerifyOptions{ - DNSName: data.ServerName, + DNSName: ServerName, } log.Printf("number of certs - %d", len(parsedCerts)) @@ -1164,13 +1167,13 @@ func validateCertificates(data tlsConfig) tlsConfig { } // validate private key (right now the only validation possible is just parsing it) - if data.PrivateKey != "" { + if PrivateKey != "" { // now do a more extended validation var key *pem.Block // PEM-encoded certificates var skippedBytes []string // skipped bytes // go through all pem blocks, but take first valid pem block and drop the rest - pemblock := []byte(data.PrivateKey) + pemblock := []byte(PrivateKey) for { var decoded *pem.Block decoded, pemblock = pem.Decode(pemblock) @@ -1202,8 +1205,8 @@ func validateCertificates(data tlsConfig) tlsConfig { } // if both are set, validate both in unison - if data.PrivateKey != "" && data.CertificateChain != "" { - _, err = tls.X509KeyPair([]byte(data.CertificateChain), []byte(data.PrivateKey)) + if PrivateKey != "" && CertificateChain != "" { + _, err = tls.X509KeyPair([]byte(CertificateChain), []byte(PrivateKey)) if err != nil { data.WarningValidation = fmt.Sprintf("Invalid certificate or key: %s", err) return data diff --git a/control_test.go b/control_test.go index 725457b9..c5df3b45 100644 --- a/control_test.go +++ b/control_test.go @@ -10,12 +10,10 @@ import ( . Bad private key . Valid certificate & private key */ func TestValidateCertificates(t *testing.T) { - var data tlsConfig + var data tlsConfigStatus // bad cert - data.CertificateChain = "bad cert" - data.PrivateKey = "" - data = validateCertificates(data) + data = validateCertificates("bad cert", "", "") if !(data.WarningValidation != "" && !data.ValidCert && !data.ValidChain) { @@ -23,16 +21,14 @@ func TestValidateCertificates(t *testing.T) { } // bad priv key - data.CertificateChain = "" - data.PrivateKey = "bad priv key" - data = validateCertificates(data) + data = validateCertificates("", "bad priv key", "") if !(data.WarningValidation != "" && !data.ValidKey) { t.Fatalf("bad priv key: validateCertificates(): %v", data) } // valid cert & priv key - data.CertificateChain = `-----BEGIN CERTIFICATE----- + CertificateChain := `-----BEGIN CERTIFICATE----- MIICKzCCAZSgAwIBAgIJAMT9kPVJdM7LMA0GCSqGSIb3DQEBCwUAMC0xFDASBgNV BAoMC0FkR3VhcmQgTHRkMRUwEwYDVQQDDAxBZEd1YXJkIEhvbWUwHhcNMTkwMjI3 MDkyNDIzWhcNNDYwNzE0MDkyNDIzWjAtMRQwEgYDVQQKDAtBZEd1YXJkIEx0ZDEV @@ -46,7 +42,7 @@ LwlXfbakf7qkVTlCNXgoY7RaJ8rJdPgOZPoCTVToEhT6u/cb1c2qp8QB0dNExDna b0Z+dnODTZqQOJo6z/wIXlcUrnR4cQVvytXt8lFn+26l6Y6EMI26twC/xWr+1swq Muj4FeWHVDerquH4yMr1jsYLD3ci+kc5sbIX6TfVxQ== -----END CERTIFICATE-----` - data.PrivateKey = `-----BEGIN PRIVATE KEY----- + PrivateKey := `-----BEGIN PRIVATE KEY----- MIICeAIBADANBgkqhkiG9w0BAQEFAASCAmIwggJeAgEAAoGBALC/BSc8mI68tw5p aYa7pjrySwWvXeetcFywOWHGVfLw9qiFWLdfESa3Y6tWMpZAXD9t1Xh9n211YUBV FGSB4ZshnM/tgEPU6t787lJD4NsIIRp++MkJxdAitN4oUTqL0bdpIwezQ/CrYuBX @@ -62,7 +58,7 @@ O5EX70gpeGQMPDK0QSWpaazg956njJSDbNCFM4BccrdQbJu1cW4qOsfBAkAMgZuG O88slmgTRHX4JGFmy3rrLiHNI2BbJSuJ++Yllz8beVzh6NfvuY+HKRCmPqoBPATU kXS9jgARhhiWXJrk -----END PRIVATE KEY-----` - data = validateCertificates(data) + data = validateCertificates(CertificateChain, PrivateKey, "") notBefore, _ := time.Parse(time.RFC3339, "2019-02-27T09:24:23Z") notAfter, _ := time.Parse(time.RFC3339, "2046-07-14T09:24:23Z") if !(data.WarningValidation != "" /* self signed */ && From f4a6ca726c56febfac49ff72cb05533f404ae4c9 Mon Sep 17 00:00:00 2001 From: Simon Zolin Date: Wed, 27 Feb 2019 14:31:53 +0300 Subject: [PATCH 3/9] * validateCertificates(): split the function's code --- control.go | 240 ++++++++++++++++++++++++++++------------------------- 1 file changed, 128 insertions(+), 112 deletions(-) diff --git a/control.go b/control.go index f6b4bab1..57e614ca 100644 --- a/control.go +++ b/control.go @@ -1078,135 +1078,151 @@ func handleTLSConfigure(w http.ResponseWriter, r *http.Request) { } } +// Return 0 on success +func verifyCertChain(data *tlsConfigStatus, certChain string, serverName string) int { + log.Tracef("got certificate: %s", certChain) + + // now do a more extended validation + var certs []*pem.Block // PEM-encoded certificates + var skippedBytes []string // skipped bytes + + pemblock := []byte(certChain) + for { + var decoded *pem.Block + decoded, pemblock = pem.Decode(pemblock) + if decoded == nil { + break + } + if decoded.Type == "CERTIFICATE" { + certs = append(certs, decoded) + } else { + skippedBytes = append(skippedBytes, decoded.Type) + } + } + + var parsedCerts []*x509.Certificate + + for _, cert := range certs { + parsed, err := x509.ParseCertificate(cert.Bytes) + if err != nil { + data.WarningValidation = fmt.Sprintf("Failed to parse certificate: %s", err) + return 1 + } + parsedCerts = append(parsedCerts, parsed) + } + + if len(parsedCerts) == 0 { + data.WarningValidation = fmt.Sprintf("You have specified an empty certificate") + return 1 + } + + data.ValidCert = true + + // spew.Dump(parsedCerts) + + opts := x509.VerifyOptions{ + DNSName: serverName, + } + + log.Printf("number of certs - %d", len(parsedCerts)) + if len(parsedCerts) > 1 { + // set up an intermediate + pool := x509.NewCertPool() + for _, cert := range parsedCerts[1:] { + log.Printf("got an intermediate cert") + pool.AddCert(cert) + } + opts.Intermediates = pool + } + + // TODO: save it as a warning rather than error it out -- shouldn't be a big problem + mainCert := parsedCerts[0] + _, err := mainCert.Verify(opts) + if err != nil { + // let self-signed certs through + data.WarningValidation = fmt.Sprintf("Your certificate does not verify: %s", err) + } else { + data.ValidChain = true + } + // spew.Dump(chains) + + // update status + if mainCert != nil { + notAfter := mainCert.NotAfter + data.Subject = mainCert.Subject.String() + data.Issuer = mainCert.Issuer.String() + data.NotAfter = notAfter + data.NotBefore = mainCert.NotBefore + data.DNSNames = mainCert.DNSNames + } + + return 0 +} + +// Return 0 on success +func validatePkey(data *tlsConfigStatus, pkey string) int { + // now do a more extended validation + var key *pem.Block // PEM-encoded certificates + var skippedBytes []string // skipped bytes + + // go through all pem blocks, but take first valid pem block and drop the rest + pemblock := []byte(pkey) + for { + var decoded *pem.Block + decoded, pemblock = pem.Decode(pemblock) + if decoded == nil { + break + } + if decoded.Type == "PRIVATE KEY" || strings.HasSuffix(decoded.Type, " PRIVATE KEY") { + key = decoded + break + } else { + skippedBytes = append(skippedBytes, decoded.Type) + } + } + + if key == nil { + data.WarningValidation = "No valid keys were found" + return 1 + } + + // parse the decoded key + _, keytype, err := parsePrivateKey(key.Bytes) + if err != nil { + data.WarningValidation = fmt.Sprintf("Failed to parse private key: %s", err) + return 1 + } + + data.ValidKey = true + data.KeyType = keytype + return 0 +} + /* Process certificate data and its private key. -CertificateChain, PrivateKey parameters are optional. +All parameters are optional. On error, return partially set object with 'WarningValidation' field containing error description. */ -func validateCertificates(CertificateChain, PrivateKey, ServerName string) tlsConfigStatus { - var err error +func validateCertificates(certChain, pkey, serverName string) tlsConfigStatus { var data tlsConfigStatus // check only public certificate separately from the key - if CertificateChain != "" { - log.Tracef("got certificate: %s", CertificateChain) - - // now do a more extended validation - var certs []*pem.Block // PEM-encoded certificates - var skippedBytes []string // skipped bytes - - pemblock := []byte(CertificateChain) - for { - var decoded *pem.Block - decoded, pemblock = pem.Decode(pemblock) - if decoded == nil { - break - } - if decoded.Type == "CERTIFICATE" { - certs = append(certs, decoded) - } else { - skippedBytes = append(skippedBytes, decoded.Type) - } - } - - var parsedCerts []*x509.Certificate - - for _, cert := range certs { - parsed, err := x509.ParseCertificate(cert.Bytes) - if err != nil { - data.WarningValidation = fmt.Sprintf("Failed to parse certificate: %s", err) - return data - } - parsedCerts = append(parsedCerts, parsed) - } - - if len(parsedCerts) == 0 { - data.WarningValidation = fmt.Sprintf("You have specified an empty certificate") + if certChain != "" { + if verifyCertChain(&data, certChain, serverName) != 0 { return data } - - data.ValidCert = true - - // spew.Dump(parsedCerts) - - opts := x509.VerifyOptions{ - DNSName: ServerName, - } - - log.Printf("number of certs - %d", len(parsedCerts)) - if len(parsedCerts) > 1 { - // set up an intermediate - pool := x509.NewCertPool() - for _, cert := range parsedCerts[1:] { - log.Printf("got an intermediate cert") - pool.AddCert(cert) - } - opts.Intermediates = pool - } - - // TODO: save it as a warning rather than error it out -- shouldn't be a big problem - mainCert := parsedCerts[0] - _, err := mainCert.Verify(opts) - if err != nil { - // let self-signed certs through - data.WarningValidation = fmt.Sprintf("Your certificate does not verify: %s", err) - } else { - data.ValidChain = true - } - // spew.Dump(chains) - - // update status - if mainCert != nil { - notAfter := mainCert.NotAfter - data.Subject = mainCert.Subject.String() - data.Issuer = mainCert.Issuer.String() - data.NotAfter = notAfter - data.NotBefore = mainCert.NotBefore - data.DNSNames = mainCert.DNSNames - } } // validate private key (right now the only validation possible is just parsing it) - if PrivateKey != "" { - // now do a more extended validation - var key *pem.Block // PEM-encoded certificates - var skippedBytes []string // skipped bytes - - // go through all pem blocks, but take first valid pem block and drop the rest - pemblock := []byte(PrivateKey) - for { - var decoded *pem.Block - decoded, pemblock = pem.Decode(pemblock) - if decoded == nil { - break - } - if decoded.Type == "PRIVATE KEY" || strings.HasSuffix(decoded.Type, " PRIVATE KEY") { - key = decoded - break - } else { - skippedBytes = append(skippedBytes, decoded.Type) - } - } - - if key == nil { - data.WarningValidation = "No valid keys were found" + if pkey != "" { + if validatePkey(&data, pkey) != 0 { return data } - - // parse the decoded key - _, keytype, err := parsePrivateKey(key.Bytes) - if err != nil { - data.WarningValidation = fmt.Sprintf("Failed to parse private key: %s", err) - return data - } - - data.ValidKey = true - data.KeyType = keytype } // if both are set, validate both in unison - if PrivateKey != "" && CertificateChain != "" { - _, err = tls.X509KeyPair([]byte(CertificateChain), []byte(PrivateKey)) + if pkey != "" && certChain != "" { + _, err := tls.X509KeyPair([]byte(certChain), []byte(pkey)) if err != nil { data.WarningValidation = fmt.Sprintf("Invalid certificate or key: %s", err) return data From 5ad9f8ead26b85c4475363df94bf13f2600c84d8 Mon Sep 17 00:00:00 2001 From: Simon Zolin Date: Wed, 27 Feb 2019 17:36:02 +0300 Subject: [PATCH 4/9] * tlsConfigStatus.usable is public, renamed ("ValidPair") and is exported to json ("valid_pair") --- app.go | 2 +- config.go | 2 +- control.go | 2 +- control_test.go | 2 +- openapi/openapi.yaml | 4 ++++ 5 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app.go b/app.go index 07c67916..90575090 100644 --- a/app.go +++ b/app.go @@ -179,7 +179,7 @@ func run(args options) { address := net.JoinHostPort(config.BindHost, strconv.Itoa(config.TLS.PortHTTPS)) // validate current TLS config and update warnings (it could have been loaded from file) data := validateCertificates(config.TLS.CertificateChain, config.TLS.PrivateKey, config.TLS.ServerName) - if !data.usable { + if !data.ValidPair { log.Fatal(data.WarningValidation) os.Exit(1) } diff --git a/config.go b/config.go index 87e5c6a8..1afc8f17 100644 --- a/config.go +++ b/config.go @@ -87,7 +87,7 @@ type tlsConfigStatus struct { KeyType string `yaml:"-" json:"key_type,omitempty"` // KeyType is one of RSA or ECDSA // is usable? set by validator - usable bool + ValidPair bool `yaml:"-" json:"valid_pair"` // ValidPair is true if both certificate and private key are correct // warnings WarningValidation string `yaml:"-" json:"warning_validation,omitempty"` // WarningValidation is a validation warning message with the issue description diff --git a/control.go b/control.go index 57e614ca..eba3fd6e 100644 --- a/control.go +++ b/control.go @@ -1227,7 +1227,7 @@ func validateCertificates(certChain, pkey, serverName string) tlsConfigStatus { data.WarningValidation = fmt.Sprintf("Invalid certificate or key: %s", err) return data } - data.usable = true + data.ValidPair = true } return data diff --git a/control_test.go b/control_test.go index c5df3b45..b823b252 100644 --- a/control_test.go +++ b/control_test.go @@ -71,7 +71,7 @@ kXS9jgARhhiWXJrk data.NotBefore == notBefore && data.NotAfter == notAfter && // data.DNSNames[0] == && - data.usable) { + data.ValidPair) { t.Fatalf("valid cert & priv key: validateCertificates(): %v", data) } } diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index f1e23f86..4cf406e0 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -1247,6 +1247,10 @@ definitions: type: "string" example: "You have specified an empty certificate" description: "warning_validation is a validation warning message with the issue description" + valid_pair: + type: "boolean" + example: "true" + description: "valid_pair is true if both certificate and private key are correct" NetInterface: type: "object" description: "Network interface info" From 24edf7eeb66668893b9add03003309d755295088 Mon Sep 17 00:00:00 2001 From: Simon Zolin Date: Wed, 27 Feb 2019 18:46:04 +0300 Subject: [PATCH 5/9] * helper functions return 'error', not 'int' --- control.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/control.go b/control.go index 540dee3f..0a074592 100644 --- a/control.go +++ b/control.go @@ -1114,8 +1114,7 @@ func handleTLSConfigure(w http.ResponseWriter, r *http.Request) { } } -// Return 0 on success -func verifyCertChain(data *tlsConfigStatus, certChain string, serverName string) int { +func verifyCertChain(data *tlsConfigStatus, certChain string, serverName string) error { log.Tracef("got certificate: %s", certChain) // now do a more extended validation @@ -1142,14 +1141,14 @@ func verifyCertChain(data *tlsConfigStatus, certChain string, serverName string) parsed, err := x509.ParseCertificate(cert.Bytes) if err != nil { data.WarningValidation = fmt.Sprintf("Failed to parse certificate: %s", err) - return 1 + return errors.New("") } parsedCerts = append(parsedCerts, parsed) } if len(parsedCerts) == 0 { data.WarningValidation = fmt.Sprintf("You have specified an empty certificate") - return 1 + return errors.New("") } data.ValidCert = true @@ -1192,11 +1191,10 @@ func verifyCertChain(data *tlsConfigStatus, certChain string, serverName string) data.DNSNames = mainCert.DNSNames } - return 0 + return nil } -// Return 0 on success -func validatePkey(data *tlsConfigStatus, pkey string) int { +func validatePkey(data *tlsConfigStatus, pkey string) error { // now do a more extended validation var key *pem.Block // PEM-encoded certificates var skippedBytes []string // skipped bytes @@ -1219,19 +1217,19 @@ func validatePkey(data *tlsConfigStatus, pkey string) int { if key == nil { data.WarningValidation = "No valid keys were found" - return 1 + return errors.New("") } // parse the decoded key _, keytype, err := parsePrivateKey(key.Bytes) if err != nil { data.WarningValidation = fmt.Sprintf("Failed to parse private key: %s", err) - return 1 + return errors.New("") } data.ValidKey = true data.KeyType = keytype - return 0 + return nil } /* Process certificate data and its private key. @@ -1244,14 +1242,14 @@ func validateCertificates(certChain, pkey, serverName string) tlsConfigStatus { // check only public certificate separately from the key if certChain != "" { - if verifyCertChain(&data, certChain, serverName) != 0 { + if verifyCertChain(&data, certChain, serverName) != nil { return data } } // validate private key (right now the only validation possible is just parsing it) if pkey != "" { - if validatePkey(&data, pkey) != 0 { + if validatePkey(&data, pkey) != nil { return data } } From 241e7ca20c6ee3a53cd59922c44dfe9fe35698d7 Mon Sep 17 00:00:00 2001 From: Simon Zolin Date: Wed, 27 Feb 2019 18:53:16 +0300 Subject: [PATCH 6/9] * control: move TLS handlers to a separate file --- control.go | 320 +--------------------------------------------- control_tls.go | 338 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 339 insertions(+), 319 deletions(-) create mode 100644 control_tls.go diff --git a/control.go b/control.go index 0a074592..9342aefd 100644 --- a/control.go +++ b/control.go @@ -3,21 +3,12 @@ package main import ( "bytes" "context" - "crypto" - "crypto/ecdsa" - "crypto/rsa" - "crypto/tls" - "crypto/x509" - "encoding/base64" "encoding/json" - "encoding/pem" - "errors" "fmt" "io/ioutil" "net" "net/http" "os" - "reflect" "sort" "strconv" "strings" @@ -26,7 +17,6 @@ import ( "github.com/AdguardTeam/AdGuardHome/dnsforward" "github.com/AdguardTeam/dnsproxy/upstream" "github.com/AdguardTeam/golibs/log" - "github.com/joomcode/errorx" "github.com/miekg/dns" govalidator "gopkg.in/asaskevich/govalidator.v4" ) @@ -1030,312 +1020,6 @@ func handleInstallConfigure(w http.ResponseWriter, r *http.Request) { } } -// --- -// TLS -// --- -func handleTLSStatus(w http.ResponseWriter, r *http.Request) { - log.Tracef("%s %v", r.Method, r.URL) - marshalTLS(w, config.TLS) -} - -func handleTLSValidate(w http.ResponseWriter, r *http.Request) { - log.Tracef("%s %v", r.Method, r.URL) - data, err := unmarshalTLS(r) - if err != nil { - httpError(w, http.StatusBadRequest, "Failed to unmarshal TLS config: %s", err) - return - } - - // check if port is available - // BUT: if we are already using this port, no need - alreadyRunning := false - if httpsServer.server != nil { - alreadyRunning = true - } - if !alreadyRunning { - err = checkPortAvailable(config.BindHost, data.PortHTTPS) - if err != nil { - httpError(w, http.StatusBadRequest, "port %d is not available, cannot enable HTTPS on it", data.PortHTTPS) - return - } - } - - data.tlsConfigStatus = validateCertificates(data.CertificateChain, data.PrivateKey, data.ServerName) - marshalTLS(w, data) -} - -func handleTLSConfigure(w http.ResponseWriter, r *http.Request) { - log.Tracef("%s %v", r.Method, r.URL) - data, err := unmarshalTLS(r) - if err != nil { - httpError(w, http.StatusBadRequest, "Failed to unmarshal TLS config: %s", err) - return - } - - // check if port is available - // BUT: if we are already using this port, no need - alreadyRunning := false - if httpsServer.server != nil { - alreadyRunning = true - } - if !alreadyRunning { - err = checkPortAvailable(config.BindHost, data.PortHTTPS) - if err != nil { - httpError(w, http.StatusBadRequest, "port %d is not available, cannot enable HTTPS on it", data.PortHTTPS) - return - } - } - - restartHTTPS := false - data.tlsConfigStatus = validateCertificates(data.CertificateChain, data.PrivateKey, data.ServerName) - if !reflect.DeepEqual(config.TLS.tlsConfigSettings, data.tlsConfigSettings) { - log.Printf("tls config settings have changed, will restart HTTPS server") - restartHTTPS = true - } - config.TLS = data - err = writeAllConfigsAndReloadDNS() - if err != nil { - httpError(w, http.StatusInternalServerError, "Couldn't write config file: %s", err) - return - } - marshalTLS(w, data) - // this needs to be done in a goroutine because Shutdown() is a blocking call, and it will block - // until all requests are finished, and _we_ are inside a request right now, so it will block indefinitely - if restartHTTPS { - go func() { - time.Sleep(time.Second) // TODO: could not find a way to reliably know that data was fully sent to client by https server, so we wait a bit to let response through before closing the server - httpsServer.cond.L.Lock() - httpsServer.cond.Broadcast() - if httpsServer.server != nil { - httpsServer.server.Shutdown(context.TODO()) - } - httpsServer.cond.L.Unlock() - }() - } -} - -func verifyCertChain(data *tlsConfigStatus, certChain string, serverName string) error { - log.Tracef("got certificate: %s", certChain) - - // now do a more extended validation - var certs []*pem.Block // PEM-encoded certificates - var skippedBytes []string // skipped bytes - - pemblock := []byte(certChain) - for { - var decoded *pem.Block - decoded, pemblock = pem.Decode(pemblock) - if decoded == nil { - break - } - if decoded.Type == "CERTIFICATE" { - certs = append(certs, decoded) - } else { - skippedBytes = append(skippedBytes, decoded.Type) - } - } - - var parsedCerts []*x509.Certificate - - for _, cert := range certs { - parsed, err := x509.ParseCertificate(cert.Bytes) - if err != nil { - data.WarningValidation = fmt.Sprintf("Failed to parse certificate: %s", err) - return errors.New("") - } - parsedCerts = append(parsedCerts, parsed) - } - - if len(parsedCerts) == 0 { - data.WarningValidation = fmt.Sprintf("You have specified an empty certificate") - return errors.New("") - } - - data.ValidCert = true - - // spew.Dump(parsedCerts) - - opts := x509.VerifyOptions{ - DNSName: serverName, - } - - log.Printf("number of certs - %d", len(parsedCerts)) - if len(parsedCerts) > 1 { - // set up an intermediate - pool := x509.NewCertPool() - for _, cert := range parsedCerts[1:] { - log.Printf("got an intermediate cert") - pool.AddCert(cert) - } - opts.Intermediates = pool - } - - // TODO: save it as a warning rather than error it out -- shouldn't be a big problem - mainCert := parsedCerts[0] - _, err := mainCert.Verify(opts) - if err != nil { - // let self-signed certs through - data.WarningValidation = fmt.Sprintf("Your certificate does not verify: %s", err) - } else { - data.ValidChain = true - } - // spew.Dump(chains) - - // update status - if mainCert != nil { - notAfter := mainCert.NotAfter - data.Subject = mainCert.Subject.String() - data.Issuer = mainCert.Issuer.String() - data.NotAfter = notAfter - data.NotBefore = mainCert.NotBefore - data.DNSNames = mainCert.DNSNames - } - - return nil -} - -func validatePkey(data *tlsConfigStatus, pkey string) error { - // now do a more extended validation - var key *pem.Block // PEM-encoded certificates - var skippedBytes []string // skipped bytes - - // go through all pem blocks, but take first valid pem block and drop the rest - pemblock := []byte(pkey) - for { - var decoded *pem.Block - decoded, pemblock = pem.Decode(pemblock) - if decoded == nil { - break - } - if decoded.Type == "PRIVATE KEY" || strings.HasSuffix(decoded.Type, " PRIVATE KEY") { - key = decoded - break - } else { - skippedBytes = append(skippedBytes, decoded.Type) - } - } - - if key == nil { - data.WarningValidation = "No valid keys were found" - return errors.New("") - } - - // parse the decoded key - _, keytype, err := parsePrivateKey(key.Bytes) - if err != nil { - data.WarningValidation = fmt.Sprintf("Failed to parse private key: %s", err) - return errors.New("") - } - - data.ValidKey = true - data.KeyType = keytype - return nil -} - -/* Process certificate data and its private key. -All parameters are optional. -On error, return partially set object - with 'WarningValidation' field containing error description. -*/ -func validateCertificates(certChain, pkey, serverName string) tlsConfigStatus { - var data tlsConfigStatus - - // check only public certificate separately from the key - if certChain != "" { - if verifyCertChain(&data, certChain, serverName) != nil { - return data - } - } - - // validate private key (right now the only validation possible is just parsing it) - if pkey != "" { - if validatePkey(&data, pkey) != nil { - return data - } - } - - // if both are set, validate both in unison - if pkey != "" && certChain != "" { - _, err := tls.X509KeyPair([]byte(certChain), []byte(pkey)) - if err != nil { - data.WarningValidation = fmt.Sprintf("Invalid certificate or key: %s", err) - return data - } - data.ValidPair = true - } - - return data -} - -// 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. -func parsePrivateKey(der []byte) (crypto.PrivateKey, string, error) { - if key, err := x509.ParsePKCS1PrivateKey(der); err == nil { - return key, "RSA", nil - } - if key, err := x509.ParsePKCS8PrivateKey(der); err == nil { - switch key := key.(type) { - case *rsa.PrivateKey: - return key, "RSA", nil - case *ecdsa.PrivateKey: - return key, "ECDSA", nil - default: - return nil, "", errors.New("tls: found unknown private key type in PKCS#8 wrapping") - } - } - if key, err := x509.ParseECPrivateKey(der); err == nil { - return key, "ECDSA", nil - } - - return nil, "", errors.New("tls: failed to parse private key") -} - -// unmarshalTLS handles base64-encoded certificates transparently -func unmarshalTLS(r *http.Request) (tlsConfig, error) { - data := tlsConfig{} - err := json.NewDecoder(r.Body).Decode(&data) - if err != nil { - return data, errorx.Decorate(err, "Failed to parse new TLS config json") - } - - if data.CertificateChain != "" { - certPEM, err := base64.StdEncoding.DecodeString(data.CertificateChain) - if err != nil { - return data, errorx.Decorate(err, "Failed to base64-decode certificate chain") - } - data.CertificateChain = string(certPEM) - } - - if data.PrivateKey != "" { - keyPEM, err := base64.StdEncoding.DecodeString(data.PrivateKey) - if err != nil { - return data, errorx.Decorate(err, "Failed to base64-decode private key") - } - - data.PrivateKey = string(keyPEM) - } - - return data, nil -} - -func marshalTLS(w http.ResponseWriter, data tlsConfig) { - w.Header().Set("Content-Type", "application/json") - if data.CertificateChain != "" { - encoded := base64.StdEncoding.EncodeToString([]byte(data.CertificateChain)) - data.CertificateChain = encoded - } - if data.PrivateKey != "" { - encoded := base64.StdEncoding.EncodeToString([]byte(data.PrivateKey)) - data.PrivateKey = encoded - } - err := json.NewEncoder(w).Encode(data) - if err != nil { - httpError(w, http.StatusInternalServerError, "Failed to marshal json with TLS status: %s", err) - return - } -} - // -------------- // DNS-over-HTTPS // -------------- @@ -1401,9 +1085,7 @@ func registerControlHandlers() { http.HandleFunc("/control/dhcp/set_config", postInstall(optionalAuth(ensurePOST(handleDHCPSetConfig)))) http.HandleFunc("/control/dhcp/find_active_dhcp", postInstall(optionalAuth(ensurePOST(handleDHCPFindActiveServer)))) - http.HandleFunc("/control/tls/status", postInstall(optionalAuth(ensureGET(handleTLSStatus)))) - http.HandleFunc("/control/tls/configure", postInstall(optionalAuth(ensurePOST(handleTLSConfigure)))) - http.HandleFunc("/control/tls/validate", postInstall(optionalAuth(ensurePOST(handleTLSValidate)))) + RegisterTLSHandlers() http.HandleFunc("/dns-query", postInstall(handleDOH)) } diff --git a/control_tls.go b/control_tls.go new file mode 100644 index 00000000..d444fe07 --- /dev/null +++ b/control_tls.go @@ -0,0 +1,338 @@ +// Control: TLS configuring handlers + +package main + +import ( + "context" + "crypto" + "crypto/ecdsa" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "encoding/base64" + "encoding/json" + "encoding/pem" + "errors" + "fmt" + "net/http" + "reflect" + "strings" + "time" + + "github.com/AdguardTeam/golibs/log" + "github.com/joomcode/errorx" +) + +// RegisterTLSHandlers registers HTTP handlers for TLS configuration +func RegisterTLSHandlers() { + http.HandleFunc("/control/tls/status", postInstall(optionalAuth(ensureGET(handleTLSStatus)))) + http.HandleFunc("/control/tls/configure", postInstall(optionalAuth(ensurePOST(handleTLSConfigure)))) + http.HandleFunc("/control/tls/validate", postInstall(optionalAuth(ensurePOST(handleTLSValidate)))) +} + +func handleTLSStatus(w http.ResponseWriter, r *http.Request) { + log.Tracef("%s %v", r.Method, r.URL) + marshalTLS(w, config.TLS) +} + +func handleTLSValidate(w http.ResponseWriter, r *http.Request) { + log.Tracef("%s %v", r.Method, r.URL) + data, err := unmarshalTLS(r) + if err != nil { + httpError(w, http.StatusBadRequest, "Failed to unmarshal TLS config: %s", err) + return + } + + // check if port is available + // BUT: if we are already using this port, no need + alreadyRunning := false + if httpsServer.server != nil { + alreadyRunning = true + } + if !alreadyRunning { + err = checkPortAvailable(config.BindHost, data.PortHTTPS) + if err != nil { + httpError(w, http.StatusBadRequest, "port %d is not available, cannot enable HTTPS on it", data.PortHTTPS) + return + } + } + + data.tlsConfigStatus = validateCertificates(data.CertificateChain, data.PrivateKey, data.ServerName) + marshalTLS(w, data) +} + +func handleTLSConfigure(w http.ResponseWriter, r *http.Request) { + log.Tracef("%s %v", r.Method, r.URL) + data, err := unmarshalTLS(r) + if err != nil { + httpError(w, http.StatusBadRequest, "Failed to unmarshal TLS config: %s", err) + return + } + + // check if port is available + // BUT: if we are already using this port, no need + alreadyRunning := false + if httpsServer.server != nil { + alreadyRunning = true + } + if !alreadyRunning { + err = checkPortAvailable(config.BindHost, data.PortHTTPS) + if err != nil { + httpError(w, http.StatusBadRequest, "port %d is not available, cannot enable HTTPS on it", data.PortHTTPS) + return + } + } + + restartHTTPS := false + data.tlsConfigStatus = validateCertificates(data.CertificateChain, data.PrivateKey, data.ServerName) + if !reflect.DeepEqual(config.TLS.tlsConfigSettings, data.tlsConfigSettings) { + log.Printf("tls config settings have changed, will restart HTTPS server") + restartHTTPS = true + } + config.TLS = data + err = writeAllConfigsAndReloadDNS() + if err != nil { + httpError(w, http.StatusInternalServerError, "Couldn't write config file: %s", err) + return + } + marshalTLS(w, data) + // this needs to be done in a goroutine because Shutdown() is a blocking call, and it will block + // until all requests are finished, and _we_ are inside a request right now, so it will block indefinitely + if restartHTTPS { + go func() { + time.Sleep(time.Second) // TODO: could not find a way to reliably know that data was fully sent to client by https server, so we wait a bit to let response through before closing the server + httpsServer.cond.L.Lock() + httpsServer.cond.Broadcast() + if httpsServer.server != nil { + httpsServer.server.Shutdown(context.TODO()) + } + httpsServer.cond.L.Unlock() + }() + } +} + +func verifyCertChain(data *tlsConfigStatus, certChain string, serverName string) error { + log.Tracef("got certificate: %s", certChain) + + // now do a more extended validation + var certs []*pem.Block // PEM-encoded certificates + var skippedBytes []string // skipped bytes + + pemblock := []byte(certChain) + for { + var decoded *pem.Block + decoded, pemblock = pem.Decode(pemblock) + if decoded == nil { + break + } + if decoded.Type == "CERTIFICATE" { + certs = append(certs, decoded) + } else { + skippedBytes = append(skippedBytes, decoded.Type) + } + } + + var parsedCerts []*x509.Certificate + + for _, cert := range certs { + parsed, err := x509.ParseCertificate(cert.Bytes) + if err != nil { + data.WarningValidation = fmt.Sprintf("Failed to parse certificate: %s", err) + return errors.New("") + } + parsedCerts = append(parsedCerts, parsed) + } + + if len(parsedCerts) == 0 { + data.WarningValidation = fmt.Sprintf("You have specified an empty certificate") + return errors.New("") + } + + data.ValidCert = true + + // spew.Dump(parsedCerts) + + opts := x509.VerifyOptions{ + DNSName: serverName, + } + + log.Printf("number of certs - %d", len(parsedCerts)) + if len(parsedCerts) > 1 { + // set up an intermediate + pool := x509.NewCertPool() + for _, cert := range parsedCerts[1:] { + log.Printf("got an intermediate cert") + pool.AddCert(cert) + } + opts.Intermediates = pool + } + + // TODO: save it as a warning rather than error it out -- shouldn't be a big problem + mainCert := parsedCerts[0] + _, err := mainCert.Verify(opts) + if err != nil { + // let self-signed certs through + data.WarningValidation = fmt.Sprintf("Your certificate does not verify: %s", err) + } else { + data.ValidChain = true + } + // spew.Dump(chains) + + // update status + if mainCert != nil { + notAfter := mainCert.NotAfter + data.Subject = mainCert.Subject.String() + data.Issuer = mainCert.Issuer.String() + data.NotAfter = notAfter + data.NotBefore = mainCert.NotBefore + data.DNSNames = mainCert.DNSNames + } + + return nil +} + +func validatePkey(data *tlsConfigStatus, pkey string) error { + // now do a more extended validation + var key *pem.Block // PEM-encoded certificates + var skippedBytes []string // skipped bytes + + // go through all pem blocks, but take first valid pem block and drop the rest + pemblock := []byte(pkey) + for { + var decoded *pem.Block + decoded, pemblock = pem.Decode(pemblock) + if decoded == nil { + break + } + if decoded.Type == "PRIVATE KEY" || strings.HasSuffix(decoded.Type, " PRIVATE KEY") { + key = decoded + break + } else { + skippedBytes = append(skippedBytes, decoded.Type) + } + } + + if key == nil { + data.WarningValidation = "No valid keys were found" + return errors.New("") + } + + // parse the decoded key + _, keytype, err := parsePrivateKey(key.Bytes) + if err != nil { + data.WarningValidation = fmt.Sprintf("Failed to parse private key: %s", err) + return errors.New("") + } + + data.ValidKey = true + data.KeyType = keytype + return nil +} + +/* Process certificate data and its private key. +All parameters are optional. +On error, return partially set object + with 'WarningValidation' field containing error description. */ +func validateCertificates(certChain, pkey, serverName string) tlsConfigStatus { + var data tlsConfigStatus + + // check only public certificate separately from the key + if certChain != "" { + if verifyCertChain(&data, certChain, serverName) != nil { + return data + } + } + + // validate private key (right now the only validation possible is just parsing it) + if pkey != "" { + if validatePkey(&data, pkey) != nil { + return data + } + } + + // if both are set, validate both in unison + if pkey != "" && certChain != "" { + _, err := tls.X509KeyPair([]byte(certChain), []byte(pkey)) + if err != nil { + data.WarningValidation = fmt.Sprintf("Invalid certificate or key: %s", err) + return data + } + data.ValidPair = true + } + + return data +} + +// 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. +func parsePrivateKey(der []byte) (crypto.PrivateKey, string, error) { + if key, err := x509.ParsePKCS1PrivateKey(der); err == nil { + return key, "RSA", nil + } + + if key, err := x509.ParsePKCS8PrivateKey(der); err == nil { + switch key := key.(type) { + case *rsa.PrivateKey: + return key, "RSA", nil + case *ecdsa.PrivateKey: + return key, "ECDSA", nil + default: + return nil, "", errors.New("tls: found unknown private key type in PKCS#8 wrapping") + } + } + + if key, err := x509.ParseECPrivateKey(der); err == nil { + return key, "ECDSA", nil + } + + return nil, "", errors.New("tls: failed to parse private key") +} + +// unmarshalTLS handles base64-encoded certificates transparently +func unmarshalTLS(r *http.Request) (tlsConfig, error) { + data := tlsConfig{} + err := json.NewDecoder(r.Body).Decode(&data) + if err != nil { + return data, errorx.Decorate(err, "Failed to parse new TLS config json") + } + + if data.CertificateChain != "" { + certPEM, err := base64.StdEncoding.DecodeString(data.CertificateChain) + if err != nil { + return data, errorx.Decorate(err, "Failed to base64-decode certificate chain") + } + data.CertificateChain = string(certPEM) + } + + if data.PrivateKey != "" { + keyPEM, err := base64.StdEncoding.DecodeString(data.PrivateKey) + if err != nil { + return data, errorx.Decorate(err, "Failed to base64-decode private key") + } + + data.PrivateKey = string(keyPEM) + } + + return data, nil +} + +func marshalTLS(w http.ResponseWriter, data tlsConfig) { + w.Header().Set("Content-Type", "application/json") + + if data.CertificateChain != "" { + encoded := base64.StdEncoding.EncodeToString([]byte(data.CertificateChain)) + data.CertificateChain = encoded + } + + if data.PrivateKey != "" { + encoded := base64.StdEncoding.EncodeToString([]byte(data.PrivateKey)) + data.PrivateKey = encoded + } + + err := json.NewEncoder(w).Encode(data) + if err != nil { + httpError(w, http.StatusInternalServerError, "Failed to marshal json with TLS status: %s", err) + return + } +} From 224c2a891dbb83332a1d582bab32a94dc3201215 Mon Sep 17 00:00:00 2001 From: Simon Zolin Date: Thu, 28 Feb 2019 11:55:16 +0300 Subject: [PATCH 7/9] * control: TLS: don't return empty error messages --- control_tls.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/control_tls.go b/control_tls.go index d444fe07..25be44d6 100644 --- a/control_tls.go +++ b/control_tls.go @@ -138,14 +138,14 @@ func verifyCertChain(data *tlsConfigStatus, certChain string, serverName string) parsed, err := x509.ParseCertificate(cert.Bytes) if err != nil { data.WarningValidation = fmt.Sprintf("Failed to parse certificate: %s", err) - return errors.New("") + return errors.New(data.WarningValidation) } parsedCerts = append(parsedCerts, parsed) } if len(parsedCerts) == 0 { data.WarningValidation = fmt.Sprintf("You have specified an empty certificate") - return errors.New("") + return errors.New(data.WarningValidation) } data.ValidCert = true @@ -214,14 +214,14 @@ func validatePkey(data *tlsConfigStatus, pkey string) error { if key == nil { data.WarningValidation = "No valid keys were found" - return errors.New("") + return errors.New(data.WarningValidation) } // parse the decoded key _, keytype, err := parsePrivateKey(key.Bytes) if err != nil { data.WarningValidation = fmt.Sprintf("Failed to parse private key: %s", err) - return errors.New("") + return errors.New(data.WarningValidation) } data.ValidKey = true From b8595b87c4c8705eddc8eed711b4760da9e894fa Mon Sep 17 00:00:00 2001 From: Ildar Kamalov Date: Thu, 28 Feb 2019 14:59:25 +0300 Subject: [PATCH 8/9] * disable config saving if invalid pair --- .../components/Settings/Encryption/Form.js | 21 +++++++++++-------- client/src/reducers/encryption.js | 1 + 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/client/src/components/Settings/Encryption/Form.js b/client/src/components/Settings/Encryption/Form.js index f4b01560..fd43f933 100644 --- a/client/src/components/Settings/Encryption/Form.js +++ b/client/src/components/Settings/Encryption/Form.js @@ -57,6 +57,7 @@ let Form = (props) => { valid_chain, valid_key, valid_cert, + valid_pair, dns_names, key_type, issuer, @@ -65,6 +66,15 @@ let Form = (props) => { setTlsConfig, } = props; + const isSavingDisabled = invalid + || submitting + || processingConfig + || processingValidate + || (isEnabled && (!privateKey || !certificateChain)) + || (privateKey && !valid_key) + || (certificateChain && !valid_cert) + || (privateKey && certificateChain && !valid_pair); + return (
@@ -291,15 +301,7 @@ let Form = (props) => { @@ -334,6 +336,7 @@ Form.propTypes = { valid_chain: PropTypes.bool, valid_key: PropTypes.bool, valid_cert: PropTypes.bool, + valid_pair: PropTypes.bool, dns_names: PropTypes.string, key_type: PropTypes.string, issuer: PropTypes.string, diff --git a/client/src/reducers/encryption.js b/client/src/reducers/encryption.js index 3f51b217..f861701b 100644 --- a/client/src/reducers/encryption.js +++ b/client/src/reducers/encryption.js @@ -70,6 +70,7 @@ const encryption = handleActions({ valid_chain: false, valid_key: false, valid_cert: false, + valid_pair: false, status_cert: '', status_key: '', certificate_chain: '', From 6b7a8078b0155ba81678e71fe53f447f444dce8b Mon Sep 17 00:00:00 2001 From: Simon Zolin Date: Thu, 28 Feb 2019 15:28:38 +0300 Subject: [PATCH 9/9] * control: use single line comment --- control_tls.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/control_tls.go b/control_tls.go index 25be44d6..0546c5f1 100644 --- a/control_tls.go +++ b/control_tls.go @@ -229,10 +229,10 @@ func validatePkey(data *tlsConfigStatus, pkey string) error { return nil } -/* Process certificate data and its private key. -All parameters are optional. -On error, return partially set object - with 'WarningValidation' field containing error description. */ +// Process certificate data and its private key. +// All parameters are optional. +// On error, return partially set object +// with 'WarningValidation' field containing error description. func validateCertificates(certChain, pkey, serverName string) tlsConfigStatus { var data tlsConfigStatus