-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: Authority Information Access support #31773
Comments
AIA is arguably a misfeature of X.509: it causes unexpected network requests during chain validation, which should be a self-contained process, introduces latency, system calls, and potentially intermittent failures in a process that should be deterministic, and multiplies the attack surface of a security critical operation. Note how Go would have been safe from the catastrophic Windows chain verification vulnerability because it rejects custom curves, if it weren't that Windows implements AIA so was willing to fetch such a poisoned certificate behind Go's back. crypto/x509 manages the complexity of X.509 by focusing on a strict useful subset, and I don't think AIA fits. Note that applications have access to the extensions, so you could write a thrid-party module that parses AIA with golang.org/x/crypto/cryptobyte, fetches the intermediate, and repeats a failed validation with it in the pool. This would provide you with all the control you need over these requests without growing the crypto/tls API. |
Thanks for the detailed reply. I understand the principle of reducing the attack surface here. It's certainly not very common to see AIA on the internet (as evidenced by the fact that most images load just fine), but it's common enough that we've had multiple independent reports of this not working. Do you know how common it would have to be for this to ship with go? (I'm assuming that some things are included (despite being bad ideas) because they're necessary for interoperability, and can probably get some numbers on the % of domains we see with this feature enabled (if Google doesn't already have that in an internal dashboard somewhere :D)). Also, I'd love your help with a little more direction on "write a thrid-party module that parses AIA with golang.org/x/crypto/cryptobyte, fetches the intermediate, and repeats a failed validation with it in the pool." Would this require a replacement to |
I don't have an explicit bar to commit to, also because it moves depending on the complexity and attack surface cost, as well as an ecosystem benefit rationale. For AIA, the bar would be very high, also because I believe it's good for the ecosystem to leave AIA behind, rather than adopt it further.
This would be logic in your custom If the verification fails, you'd look at |
Thank you for the extra context. (I love the idea of pushing the ecosystem forward — easier to do as a large popular project, but harder when you're just getting started :D). I'll play around and see if I can get something working — and if so put it on Github for others with this problem. Feel free to close this if you think it's not something that go will implement ever. |
Hey @ConradIrwin, playing around with this a little I've come up with the following POC, I haven't done much testing (so use at your own RISK 👀) but the brunt of it is there. Tomorrow when I have a chance I'll verify the code, add some test cases and role it into a little library on my Github. I will update you here when I do so. package main
import (
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"io/ioutil"
"log"
"net"
"net/http"
"time"
)
func main() {
rootCAs, err := x509.SystemCertPool()
if err != nil {
log.Fatal(err)
}
client := http.Client{
Transport: &http.Transport{
DialTLS: func(network, addr string) (net.Conn, error) {
conn, err := tls.Dial(network, addr, &tls.Config{
InsecureSkipVerify: true,
ServerName: addr,
RootCAs: rootCAs,
VerifyPeerCertificate: func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
host, _, err := net.SplitHostPort(addr)
if err != nil {
return err
}
return verifyPeerCerts(rootCAs, host, rawCerts)
},
})
if err != nil {
return conn, err
}
return conn, nil
},
},
}
res, err := client.Get("https://incomplete-chain.badssl.com/")
if err != nil {
log.Fatal(err)
}
fmt.Println(res.Status)
}
func verifyPeerCerts(rootCAs *x509.CertPool, serverName string, rawCerts [][]byte) error {
certs := make([]*x509.Certificate, len(rawCerts))
for i, asn1Data := range rawCerts {
cert, err := x509.ParseCertificate(asn1Data)
if err != nil {
return errors.New("failed to parse certificate from server: " + err.Error())
}
certs[i] = cert
}
opts := x509.VerifyOptions{
Roots: rootCAs,
CurrentTime: time.Now(),
DNSName: serverName,
Intermediates: x509.NewCertPool(),
}
for _, cert := range certs[1:] {
opts.Intermediates.AddCert(cert)
}
_, err := certs[0].Verify(opts)
if err != nil {
if _, ok := err.(x509.UnknownAuthorityError); ok {
lastCert := certs[len(certs)-1]
if len(lastCert.IssuingCertificateURL) >= 1 && lastCert.IssuingCertificateURL[0] != "" {
resp, err := http.Get(lastCert.IssuingCertificateURL[0])
if resp != nil {
defer resp.Body.Close()
}
if err != nil {
return err
}
data, err := ioutil.ReadAll(resp.Body)
if err != nil {
return err
}
rawCerts = append(rawCerts, data)
return verifyPeerCerts(rootCAs, serverName, rawCerts)
}
}
return err
}
return nil
} |
@fcjr this is awesome! |
(Hah, I didn't know we already parse the AIA extension into |
Adding to proposal minutes but this seems headed for a likely decline. |
Thanks @FiloSottile, I've updated my comment to reflect that. I've also set up a small library which provides a preconfigured transport which supports AIA for anyone who is interested https://github.com/fcjr/aia-transport-go. |
Based on the discussion above (including the fact that it's so easily implemented outside the standard library), this seems like a likely decline. |
No change in consensus, so declined. |
I am using Go to proxy image requests from an email client (to mask the IP addresses of our users), but there are some images that load in Gmail, but fail to load in our email client.
Investigating this, it seems likely that the problem is that go does not support the "Authority Information Access" extension, which allows a certificate like the one below (in use by AT&T) to validate even though the server does not pass back the entire certificate chain.
I have verified that the intermediate does not appear in my Trust Store and that the image can be loaded in Chrome, Firefox and Safari. I can also see that the image does not load using curl (issue), or using the go standard library.
I would like to propose that go's tls stack support AIA. In particular the subset of AIA that enables loading id-ad-caIssuers over HTTP as defined in https://tools.ietf.org/html/rfc5280#section-4.2.2.2.
It could either be enabled by default, which would allow
http.Get
to work more like a web-browser; or if we're worried about unexpected extra requests, as an option totls.Config
that enables this behaviour.The text was updated successfully, but these errors were encountered: