Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: crypto/tls: Config adds an option to get RootCAs #70946

Closed
shaj13 opened this issue Dec 21, 2024 · 4 comments
Closed

proposal: crypto/tls: Config adds an option to get RootCAs #70946

shaj13 opened this issue Dec 21, 2024 · 4 comments
Labels
Milestone

Comments

@shaj13
Copy link

shaj13 commented Dec 21, 2024

Proposal Details

The tls.config provides several callback functions such as GetCertificate, GetClientCertificate, and GetConfigForClient, which enable dynamic behavior during the TLS handshake. These callbacks are particularly useful for scenarios involving certificate rotation, both on the client and server sides.

However, tls.Config currently lacks an option for dynamically managing the Root CAs used to verify server certificates on the client side. Since x509.CertPool methods cannot be invoked concurrently by multiple goroutines, it becomes challenging to update or reload the Root CA pool at runtime without risking data races or inconsistent state.

Motivation:

  • Root CA Certificate rotation
  • Dynamic trust policies

This proposal seeks to introduce a new callback in tls.Config that allows for dynamically retrieving the latest CertPool, similar to the existing callback functions.

// GetRootCAs returns  the set of root certificate authorities
// that clients use when verifying server certificates.
// 
// If GetRootCAs is nil, then the RootCAs is used. 
GetRootCAs func() (*x509.CertPool)

The client handshake will work with the proposed changes as follows:

func (c *Config) rootCAs() *x509.CertPool{
	if c.GetRootCAs == nil {
		return c.RootCAs
	}
        
        return c.GetRootCAs()
}
func (c *Conn) verifyServerCertificate(certificates [][]byte) error {
	} else if !c.config.InsecureSkipVerify {
		opts := x509.VerifyOptions{
			Roots:         c.config.rootCAs(),
			CurrentTime:   c.config.time(),
			DNSName:       c.config.ServerName,
			Intermediates: x509.NewCertPool(),
		}

		for _, cert := range certs[1:] {
			opts.Intermediates.AddCert(cert)
		}
		chains, err := certs[0].Verify(opts)
		if err != nil {
			c.sendAlert(alertBadCertificate)
			return &CertificateVerificationError{UnverifiedCertificates: certs, Err: err}
		}

		c.verifiedChains, err = fipsAllowedChains(chains)
		if err != nil {
			c.sendAlert(alertBadCertificate)
			return &CertificateVerificationError{UnverifiedCertificates: certs, Err: err}
		}
	}
}
@gopherbot gopherbot added this to the Proposal milestone Dec 21, 2024
@Jorropo
Copy link
Member

Jorropo commented Dec 21, 2024

Thx to https://go-review.googlesource.com/c/go/+/28773#related-content I think that dup of #16066
can be reopened if this is different.

@Jorropo Jorropo closed this as not planned Won't fix, can't repro, duplicate, stale Dec 21, 2024
@shaj13
Copy link
Author

shaj13 commented Dec 21, 2024

Thx to https://go-review.googlesource.com/c/go/+/28773#related-content I think that dup of #16066 can be reopened if this is different.

Its a dup of #64796, #16066 is for server-side where GetConfigForClient is introduced later.

@shaj13 shaj13 changed the title proposal: crypto/tls: Config adds an option to get proposal: crypto/tls: Config adds an option to get RootCAs Dec 21, 2024
@Jorropo
Copy link
Member

Jorropo commented Dec 21, 2024

I see thx, this could be reopened but I see you posted there #64796 (comment) I guess both works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants