home: imp code

internal/home/tls.go

@@ -160,7 +160,7 @@ func loadTLSConf(tlsConf *tlsConfigSettings, status *tlsConfigStatus) (err error
 	defer func() {
 		if err != nil {
 			status.WarningValidation = err.Error()
-			if errors.As(err, new(warningError)) {
+			if status.ValidCert && status.ValidKey && status.ValidPair {
 				// Do not return warnings since those aren't critical.
 				err = nil
 			}
@@ -180,6 +180,8 @@ func loadTLSConf(tlsConf *tlsConfigSettings, status *tlsConfigStatus) (err error
 			return fmt.Errorf("reading cert file: %w", err)
 		}
 
+		// Set status.ValidCert to true to signal the frontend that the
+		// certificate opens successfully while the private key can't be opened.
 		status.ValidCert = true
 	}
 
@@ -203,15 +205,10 @@ func loadTLSConf(tlsConf *tlsConfigSettings, status *tlsConfigStatus) (err error
 		tlsConf.ServerName,
 	)
 	if err != nil {
-		if warning := (warningError{}); errors.As(err, &warning) {
-			warning.error = fmt.Errorf("validating certificate pair: %w", warning.error)
-			err = warning
-		} else {
-			err = fmt.Errorf("validating certificate pair: %w", err)
-		}
+		return fmt.Errorf("validating certificate pair: %w", err)
 	}
 
-	return err
+	return nil
 }
 
 // tlsConfigStatus contains the status of a certificate chain and key pair.
@@ -492,9 +489,9 @@ func validatePorts(
 }
 
 // validateCertChain validates the certificate chain.  It returns the first
-// certificate within the chain.  mainCert is guaranteed to be non-nil if the
-// returned error is nil or a [warningError].
-func validateCertChain(chain []byte, serverName string) (mainCert *x509.Certificate, err error) {
+// certificate within the chain.  ok is true if the main certificate is valid.
+// main is guaranteed to be non-nil when ok is true.
+func validateCertChain(chain []byte, srvName string) (main *x509.Certificate, ok bool, err error) {
 	log.Debug("tls: got certificate chain: %d bytes", len(chain))
 
 	var certs []*pem.Block
@@ -508,32 +505,29 @@ func validateCertChain(chain []byte, serverName string) (mainCert *x509.Certific
 
 	parsedCerts, err := parsePEMCerts(certs)
 	if err != nil {
-		return nil, err
-	}
-
-	opts := x509.VerifyOptions{
-		DNSName: serverName,
-		Roots:   Context.tlsRoots,
+		return nil, false, err
 	}
 
 	log.Info("tls: number of certs: %d", len(parsedCerts))
 
+	var others []*x509.Certificate
+	main, others = parsedCerts[0], parsedCerts[1:]
+
 	pool := x509.NewCertPool()
-	for _, cert := range parsedCerts[1:] {
+	for _, cert := range others {
 		log.Info("tls: got an intermediate cert")
 		pool.AddCert(cert)
 	}
 
-	opts.Intermediates = pool
-
-	mainCert = parsedCerts[0]
-	_, err = mainCert.Verify(opts)
-	if err != nil {
-		// Let self-signed certs through and don't return this error.
-		err = warningError{fmt.Errorf("certificate does not verify: %s", err)}
+	opts := x509.VerifyOptions{
+		DNSName:       srvName,
+		Roots:         Context.tlsRoots,
+		Intermediates: pool,
 	}
+	_, err = main.Verify(opts)
 
-	return mainCert, err
+	// Let self-signed certs through and don't return this error.
+	return main, true, errors.Annotate(err, "certificate does not verify: %w")
 }
 
 // parsePEMCerts parses multiple PEM-encoded certificates.
@@ -590,30 +584,6 @@ func validatePKey(pkey []byte) (keyType string, err error) {
 	return keyType, nil
 }
 
-// warningError is a non-critical error to be reported.  It capitalizes self
-// string representation, assuming that a wrapped error's message is ASCII-only.
-type warningError struct{ error }
-
-// type check
-var _ error = warningError{}
-
-// Error implements the [error] interface for warningError.  It returns the
-// capitalized string representation of werr.
-func (werr warningError) Error() (msg string) {
-	msg = werr.error.Error()
-	if werr.error != nil {
-		msg = strings.ToUpper(msg[:1]) + msg[1:]
-	}
-
-	return msg
-}
-
-// type check
-var _ errors.Wrapper = warningError{}
-
-// Unwrap implements the [errors.Wrapper] interface for warningError.
-func (werr warningError) Unwrap() (err error) { return werr.error }
-
 // validateCertificates processes certificate data and its private key.  All
 // parameters are optional.  status must not be nil.  The returned error is also
 // set in status.WarningValidation.
@@ -625,18 +595,17 @@ func validateCertificates(
 ) (err error) {
 	// Check only the public certificate separately from the key.
 	if len(certChain) > 0 {
-		mainCert, verr := validateCertChain(certChain, serverName)
-		if verr != nil {
-			if !errors.As(verr, new(warningError)) {
-				return verr
-			} else {
-				err = verr
+		var mainCert *x509.Certificate
+		mainCert, status.ValidCert, err = validateCertChain(certChain, serverName)
+		if err != nil {
+			if !status.ValidCert {
+				// Don't wrap the error, since it's informative enough as is.
+				return err
 			}
 		} else {
 			status.ValidChain = true
 		}
 
-		status.ValidCert = true
 		status.Subject = mainCert.Subject.String()
 		status.Issuer = mainCert.Issuer.String()
 		status.NotAfter = mainCert.NotAfter
@@ -644,22 +613,21 @@ func validateCertificates(
 		status.DNSNames = mainCert.DNSNames
 
 		if err == nil && len(mainCert.IPAddresses) == 0 {
-			const errMsg errors.Error = `certificate has no IP addresses` +
-				`, this may cause issues with DNS-over-TLS clients.`
-
-			err = warningError{errMsg}
+			err = errors.Error(`certificate has no IP addresses` +
+				`, this may cause issues with DNS-over-TLS clients.`)
 		}
 	}
 
 	// Validate the private key by parsing it.
 	if len(pkey) > 0 {
-		keyType, verr := validatePKey(pkey)
+		var verr error
+		status.KeyType, verr = validatePKey(pkey)
 		if verr != nil {
+			// Don't wrap the error, since it's informative enough as is.
 			return verr
 		}
 
 		status.ValidKey = true
-		status.KeyType = keyType
 	}
 
 	// If both are set, validate together.

internal/home/tls_internal_test.go

@@ -57,9 +57,12 @@ func TestValidateCertificates(t *testing.T) {
 	})
 
 	t.Run("valid", func(t *testing.T) {
+		const wantErrMsg = `certificate does not verify: x509: ` +
+			`“AdGuard Home” certificate is using a broken key size`
+
 		status := &tlsConfigStatus{}
 		err := validateCertificates(status, testCertChainData, testPrivateKeyData, "")
-		assert.ErrorAs(t, err, new(warningError))
+		testutil.AssertErrorMsg(t, wantErrMsg, err)
 
 		notBefore := time.Date(2019, 2, 27, 9, 24, 23, 0, time.UTC)
 		notAfter := time.Date(2046, 7, 14, 9, 24, 23, 0, time.UTC)