Skip to content

Commit

Permalink
Pull request: 4898-redirect-https
Browse files Browse the repository at this point in the history 
Merge in DNS/adguard-home from 4898-redirect-https to master

Updates AdguardTeam#4898.
Updates AdguardTeam#4927.

Squashed commit of the following:

commit bc41b6c
Merge: 815e299 ac7634d
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Nov 1 13:02:23 2022 +0300

    Merge branch 'master' into 4898-redirect-https

commit 815e299
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Nov 1 12:58:28 2022 +0300

    home: imp ip addr detection

commit 9d4ecd9
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Oct 31 17:23:41 2022 +0300

    home: imp cyclo

commit 86c47b6
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Oct 31 15:06:05 2022 +0300

    all: imp text

commit bcc2569
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Oct 31 11:47:57 2022 +0300

    home: fix test

commit bb51a74
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Sun Oct 30 23:23:40 2022 +0300

    home: imp code

commit 3852233
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Fri Oct 28 17:00:50 2022 +0300

    home: imp code

commit 7284f72
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Oct 27 19:42:57 2022 +0300

    all: log changes

commit 540efcb
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Oct 27 19:24:21 2022 +0300

    home: imp tls
  • Loading branch information
@EugeneOne1 @heyxkhoa
EugeneOne1 authored and heyxkhoa committed Mar 17, 2023
1 parent 8e94f74 commit 05439e1
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 105 deletions.
200 changes: 101 additions & 99 deletions internal/home/tls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"encoding/pem"
"fmt"
"net/http"
"net/netip"
"os"
"strings"
"sync"
Expand Down Expand Up @@ -160,6 +161,10 @@ func loadTLSConf(tlsConf *tlsConfigSettings, status *tlsConfigStatus) (err error
defer func() {
if err != nil {
status.WarningValidation = err.Error()
if status.ValidCert && status.ValidKey && status.ValidPair {
// Do not return warnings since those aren't critical.
err = nil
}
}
}()

Expand All @@ -176,6 +181,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
}

Expand Down Expand Up @@ -482,71 +489,73 @@ func validatePorts(
return nil
}

// validateCertChain validates the certificate chain and sets data in status.
// The returned error is also set in status.WarningValidation.
func validateCertChain(status *tlsConfigStatus, certChain []byte, serverName string) (err error) {
defer func() {
if err != nil {
status.WarningValidation = err.Error()
}
}()

log.Debug("tls: got certificate chain: %d bytes", len(certChain))

var certs []*pem.Block
pemblock := certChain
for {
var decoded *pem.Block
decoded, pemblock = pem.Decode(pemblock)
if decoded == nil {
break
}
// validateCertChain verifies certs using the first as the main one and others
// as intermediate. srvName stands for the expected DNS name.
func validateCertChain(certs []*x509.Certificate, srvName string) (err error) {
main, others := certs[0], certs[1:]

if decoded.Type == "CERTIFICATE" {
certs = append(certs, decoded)
}
pool := x509.NewCertPool()
for _, cert := range others {
log.Info("tls: got an intermediate cert")
pool.AddCert(cert)
}

parsedCerts, err := parsePEMCerts(certs)
opts := x509.VerifyOptions{
DNSName: srvName,
Roots: Context.tlsRoots,
Intermediates: pool,
}
_, err = main.Verify(opts)
if err != nil {
return err
return fmt.Errorf("certificate does not verify: %w", err)
}

status.ValidCert = true
return nil
}

// certHasIP returns true if cert has at least a single IP address either in its
// DNS names or in the IP addresses section.
func certHasIP(cert *x509.Certificate) (ok bool) {
if len(cert.IPAddresses) > 0 {
return true
}

opts := x509.VerifyOptions{
DNSName: serverName,
Roots: Context.tlsRoots,
for _, name := range cert.DNSNames {
if _, err := netip.ParseAddr(name); err == nil {
return true
}
}

log.Info("tls: number of certs: %d", len(parsedCerts))
return false
}

pool := x509.NewCertPool()
for _, cert := range parsedCerts[1:] {
log.Info("tls: got an intermediate cert")
pool.AddCert(cert)
}
// parseCertChain parses the certificate chain from raw data, and returns it.
// If ok is true, the returned error, if any, is not critical.
func parseCertChain(chain []byte) (parsedCerts []*x509.Certificate, ok bool, err error) {
log.Debug("tls: got certificate chain: %d bytes", len(chain))

opts.Intermediates = pool
var certs []*pem.Block
for decoded, pemblock := pem.Decode(chain); decoded != nil; {
if decoded.Type == "CERTIFICATE" {
certs = append(certs, decoded)
}

decoded, pemblock = pem.Decode(pemblock)
}

mainCert := parsedCerts[0]
_, err = mainCert.Verify(opts)
parsedCerts, err = parsePEMCerts(certs)
if err != nil {
// Let self-signed certs through and don't return this error.
status.WarningValidation = fmt.Sprintf("certificate does not verify: %s", err)
} else {
status.ValidChain = true
return nil, false, err
}

if mainCert != nil {
status.Subject = mainCert.Subject.String()
status.Issuer = mainCert.Issuer.String()
status.NotAfter = mainCert.NotAfter
status.NotBefore = mainCert.NotBefore
status.DNSNames = mainCert.DNSNames
log.Info("tls: number of certs: %d", len(parsedCerts))

if !certHasIP(parsedCerts[0]) {
err = errors.Error(`certificate has no IP addresses` +
`, this may cause issues with DNS-over-TLS clients`)
}

return nil
return parsedCerts, true, err
}

// parsePEMCerts parses multiple PEM-encoded certificates.
Expand All @@ -568,106 +577,99 @@ func parsePEMCerts(certs []*pem.Block) (parsedCerts []*x509.Certificate, err err
return parsedCerts, nil
}

// validatePKey validates the private key and sets data in status. The returned
// error is also set in status.WarningValidation.
func validatePKey(status *tlsConfigStatus, pkey []byte) (err error) {
defer func() {
if err != nil {
status.WarningValidation = err.Error()
}
}()

// validatePKey validates the private key, returning its type. It returns an
// empty string if error occurs.
func validatePKey(pkey []byte) (keyType string, err error) {
var key *pem.Block

// 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
}

for decoded, pemblock := pem.Decode([]byte(pkey)); decoded != nil; {
if decoded.Type == "PRIVATE KEY" || strings.HasSuffix(decoded.Type, " PRIVATE KEY") {
key = decoded

break
}

decoded, pemblock = pem.Decode(pemblock)
}

if key == nil {
return errors.Error("no valid keys were found")
return "", errors.Error("no valid keys were found")
}

_, keyType, err := parsePrivateKey(key.Bytes)
_, keyType, err = parsePrivateKey(key.Bytes)
if err != nil {
return fmt.Errorf("parsing private key: %w", err)
return "", fmt.Errorf("parsing private key: %w", err)
}

if keyType == keyTypeED25519 {
return errors.Error(
return "", errors.Error(
"ED25519 keys are not supported by browsers; " +
"did you mean to use X25519 for key exchange?",
)
}

status.ValidKey = true
status.KeyType = keyType

return nil
return keyType, nil
}

// 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.
// validateCertificates processes certificate data and its private key. status
// must not be nil, since it's used to accumulate the validation results. Other
// parameters are optional.
func validateCertificates(
status *tlsConfigStatus,
certChain []byte,
pkey []byte,
serverName string,
) (err error) {
defer func() {
// Capitalize the warning for the UI. Assume that warnings are all
// ASCII-only.
//
// TODO(a.garipov): Figure out a better way to do this. Perhaps a
// custom string or error type.
if w := status.WarningValidation; w != "" {
status.WarningValidation = strings.ToUpper(w[:1]) + w[1:]
}
}()

// Check only the public certificate separately from the key.
if len(certChain) > 0 {
err = validateCertChain(status, certChain, serverName)
if err != nil {
var certs []*x509.Certificate
certs, status.ValidCert, err = parseCertChain(certChain)
if !status.ValidCert {
// Don't wrap the error, since it's informative enough as is.
return err
}

mainCert := certs[0]
status.Subject = mainCert.Subject.String()
status.Issuer = mainCert.Issuer.String()
status.NotAfter = mainCert.NotAfter
status.NotBefore = mainCert.NotBefore
status.DNSNames = mainCert.DNSNames

if chainErr := validateCertChain(certs, serverName); chainErr != nil {
// Let self-signed certs through and don't return this error to set
// its message into the status.WarningValidation afterwards.
err = chainErr
} else {
status.ValidChain = true
}
}

// Validate the private key by parsing it.
if len(pkey) > 0 {
err = validatePKey(status, pkey)
if err != nil {
return err
var keyErr error
status.KeyType, keyErr = validatePKey(pkey)
if keyErr != nil {
// Don't wrap the error, since it's informative enough as is.
return keyErr
}

status.ValidKey = true
}

// If both are set, validate together.
if len(certChain) > 0 && len(pkey) > 0 {
_, err = tls.X509KeyPair(certChain, pkey)
if err != nil {
err = fmt.Errorf("certificate-key pair: %w", err)
status.WarningValidation = err.Error()

return err
_, pairErr := tls.X509KeyPair(certChain, pkey)
if pairErr != nil {
return fmt.Errorf("certificate-key pair: %w", pairErr)
}

status.ValidPair = true
}

return nil
return err
}

// Key types.
Expand Down
10 changes: 4 additions & 6 deletions internal/home/tls_internal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"
"time"

"github.com/AdguardTeam/golibs/testutil"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -43,29 +44,26 @@ func TestValidateCertificates(t *testing.T) {
t.Run("bad_certificate", func(t *testing.T) {
status := &tlsConfigStatus{}
err := validateCertificates(status, []byte("bad cert"), nil, "")
assert.Error(t, err)
assert.NotEmpty(t, status.WarningValidation)
testutil.AssertErrorMsg(t, "empty certificate", err)
assert.False(t, status.ValidCert)
assert.False(t, status.ValidChain)
})

t.Run("bad_private_key", func(t *testing.T) {
status := &tlsConfigStatus{}
err := validateCertificates(status, nil, []byte("bad priv key"), "")
assert.Error(t, err)
assert.NotEmpty(t, status.WarningValidation)
testutil.AssertErrorMsg(t, "no valid keys were found", err)
assert.False(t, status.ValidKey)
})

t.Run("valid", func(t *testing.T) {
status := &tlsConfigStatus{}
err := validateCertificates(status, testCertChainData, testPrivateKeyData, "")
assert.NoError(t, err)
assert.Error(t, 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)

assert.NotEmpty(t, status.WarningValidation)
assert.True(t, status.ValidCert)
assert.False(t, status.ValidChain)
assert.True(t, status.ValidKey)
Expand Down

0 comments on commit 05439e1

Please sign in to comment.