Skip to content

Commit

Permalink
Manual backport from Beats
Browse files Browse the repository at this point in the history
* libbeat: fix IP and hostname validation on TLS certificates
* tlscommon: fix race condition on TLS hostname validation

This commit is a combined backport from:
* elastic/beats#30305
* elastic/beats@b4314ad
  • Loading branch information
belimawr authored Aug 25, 2023
1 parent d94596f commit 270b03e
Show file tree
Hide file tree
Showing 2 changed files with 426 additions and 1 deletion.
42 changes: 41 additions & 1 deletion transport/tlscommon/tls_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ type TLSConfig struct {
// this certificate will be added to the list of trusted CAs (RootCAs) during the handshake.
CATrustedFingerprint string

// ServerName is the remote server we're connecting to. It can be a hostname or IP address.
ServerName string

// time returns the current time as the number of seconds since the epoch.
// If time is nil, TLS uses time.Now.
time func() time.Time
Expand Down Expand Up @@ -129,12 +132,25 @@ func (c *TLSConfig) BuildModuleClientConfig(host string) *tls.Config {
InsecureSkipVerify: true, //nolint: gosec // we are using our own verification for now
VerifyConnection: makeVerifyConnection(&TLSConfig{
Verification: VerifyFull,
ServerName: host,
}),
}
}

config := c.ToConfig()
// Make a copy of c, because we're gonna mutate it after
// calling ToConfig. ToConfig calls a function that creates
// a closure that needs to access cc. A shallow copy is enough
// because all slice/pointer fields won't be modified.
cc := *c

// Keep a copy of the host (wheather an IP or hostname)
// for later validation. It is used by makeVerifyConnection
cc.ServerName = host
config := cc.ToConfig()

// config.ServerName does not verify IP addresses
config.ServerName = host

return config
}

Expand All @@ -147,6 +163,7 @@ func (c *TLSConfig) BuildServerConfig(host string) *tls.Config {
InsecureSkipVerify: true, //nolint: gosec // we are using our own verification for now
VerifyConnection: makeVerifyServerConnection(&TLSConfig{
Verification: VerifyFull,
ServerName: host,
}),
}
}
Expand Down Expand Up @@ -195,6 +212,9 @@ func trustRootCA(cfg *TLSConfig, peerCerts []*x509.Certificate) error {
func makeVerifyConnection(cfg *TLSConfig) func(tls.ConnectionState) error {
switch cfg.Verification {
case VerifyFull:
// Cert is trusted by CA
// Hostname or IP matches the certificate
// tls.Config.InsecureSkipVerify is set to true
return func(cs tls.ConnectionState) error {
if cfg.CATrustedFingerprint != "" {
if err := trustRootCA(cfg, cs.PeerCertificates); err != nil {
Expand All @@ -217,6 +237,9 @@ func makeVerifyConnection(cfg *TLSConfig) func(tls.ConnectionState) error {
return verifyHostname(cs.PeerCertificates[0], cs.ServerName)
}
case VerifyCertificate:
// Cert is trusted by CA
// Does NOT validate hostname or IP addresses
// tls.Config.InsecureSkipVerify is set to true
return func(cs tls.ConnectionState) error {
if cfg.CATrustedFingerprint != "" {
if err := trustRootCA(cfg, cs.PeerCertificates); err != nil {
Expand All @@ -235,6 +258,12 @@ func makeVerifyConnection(cfg *TLSConfig) func(tls.ConnectionState) error {
return verifyCertsWithOpts(cs.PeerCertificates, cfg.CASha256, opts)
}
case VerifyStrict:
// Cert is trusted by CA
// Hostname or IP matches the certificate
// Returns error if SNA is empty
// The whole validation is done by Go's standard library default
// SSL/TLS verification (tls.Config.InsecureSkipVerify is set to false)
// so we only need to check the pin
if len(cfg.CASha256) > 0 {
return func(cs tls.ConnectionState) error {
if cfg.CATrustedFingerprint != "" {
Expand Down Expand Up @@ -317,6 +346,9 @@ func verifyCertsWithOpts(certs []*x509.Certificate, casha256 []string, opts x509
return nil
}

// verifyHostname verifies if the provided hostnmae matches
// cert.DNSNames, cert.IPAddress (SNA)
// For hostnames, if SNA is empty, validate the hostname against cert.Subject.CommonName
func verifyHostname(cert *x509.Certificate, hostname string) error {
if hostname == "" {
return nil
Expand All @@ -333,6 +365,14 @@ func verifyHostname(cert *x509.Certificate, hostname string) error {
return nil
}
}

parsedCNIP := net.ParseIP(cert.Subject.CommonName)
if parsedCNIP != nil {
if parsedIP.Equal(parsedCNIP) {
return nil
}
}

return x509.HostnameError{Certificate: cert, Host: hostname}
}

Expand Down
Loading

0 comments on commit 270b03e

Please sign in to comment.