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: Authority Information Access support #31773

Closed
ConradIrwin opened this issue May 1, 2019 · 11 comments
Closed

proposal: crypto/tls: Authority Information Access support #31773

ConradIrwin opened this issue May 1, 2019 · 11 comments
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@ConradIrwin
Copy link
Contributor

ConradIrwin commented May 1, 2019

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 to tls.Config that enables this behaviour.

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            0a:e4:90:72:1c:85:e6:10:78:9a:2c:68:4f:0f:6c:b9
    Signature Algorithm: sha256WithRSAEncryption
        Issuer: C=US, O=DigiCert Inc, CN=DigiCert SHA2 Secure Server CA
        Validity
            Not Before: Feb 19 00:00:00 2019 GMT
            Not After : May  2 12:00:00 2020 GMT
        Subject: C=US, ST=Texas, L=Dallas, O=AT&T Services, Inc., OU=AT&T IoT, CN=simcontrolcenter.wireless.att.com
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)
                Modulus:
                    00:ee:11:b8:b8:64:e8:4f:e9:27:92:5f:a1:24:27:
                    fe:e3:8f:56:25:df:c1:13:9d:ac:ff:74:a1:aa:05:
                    2a:1e:58:55:45:bc:72:de:08:21:bd:94:62:f0:fa:
                    91:6e:f4:01:f7:a8:73:67:b8:91:70:dc:2a:1a:5a:
                    9e:99:cd:dd:bd:b8:39:f0:53:2d:22:de:60:3e:32:
                    4f:e6:67:47:83:f3:d2:53:5e:37:0a:a1:ef:c9:15:
                    77:fd:65:a4:70:88:34:5a:a7:e1:1d:eb:43:48:28:
                    48:01:ea:97:37:ad:02:d9:21:ef:de:2c:8b:9c:dc:
                    35:84:c8:50:50:74:32:2f:9c:15:cb:aa:65:31:57:
                    c4:58:59:76:a0:bb:a1:d9:ce:ba:26:9f:5a:45:f5:
                    e8:ad:e6:ca:25:65:c6:8f:35:82:08:c1:ca:c0:ea:
                    7d:43:89:a6:7e:a0:29:11:5b:49:88:32:c2:5c:ad:
                    8a:26:76:25:61:6f:44:fd:0a:46:46:33:0d:06:e6:
                    17:10:34:27:92:a1:d6:86:ee:bb:33:28:ab:cb:55:
                    61:2e:ba:f8:d0:19:90:2e:b4:90:eb:b4:64:80:df:
                    a5:f9:91:6c:cd:8b:83:3f:ae:e9:fa:e2:c7:11:4e:
                    fa:63:3c:65:05:3f:90:a8:ae:aa:92:df:98:21:63:
                    47:b3
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Authority Key Identifier:
                keyid:0F:80:61:1C:82:31:61:D5:2F:28:E7:8D:46:38:B4:2C:E1:C6:D9:E2

            X509v3 Subject Key Identifier:
                BF:6B:51:FD:71:80:72:7E:64:3F:C1:1F:AA:BE:D3:AB:66:9E:08:29
            X509v3 Subject Alternative Name:
                DNS:simcontrolcenter.wireless.att.com
            X509v3 Key Usage: critical
                Digital Signature, Key Encipherment
            X509v3 Extended Key Usage:
                TLS Web Server Authentication, TLS Web Client Authentication
            X509v3 CRL Distribution Points:

                Full Name:
                  URI:http://crl3.digicert.com/ssca-sha2-g6.crl

                Full Name:
                  URI:http://crl4.digicert.com/ssca-sha2-g6.crl

            X509v3 Certificate Policies:
                Policy: 2.16.840.1.114412.1.1
                  CPS: https://www.digicert.com/CPS
                Policy: 2.23.140.1.2.2

            Authority Information Access:
                OCSP - URI:http://ocsp.digicert.com
                CA Issuers - URI:http://cacerts.digicert.com/DigiCertSHA2SecureServerCA.crt

            X509v3 Basic Constraints:
                CA:FALSE
            1.3.6.1.4.1.11129.2.4.2:
                ......v.......q...#...{G8W.
.R....d6.......i.7W~.....G0E. >..H..A.Z>..S..Y...d.,...F..X.*..!...a.G............=..xNw....cU..N.w..u..Y|..C._..n.V.GV6.J.`....^......i.7X......H0F.!..z..H..C.w..9s?m. ...i........r..!...61^<..(Y!0T.D......+K.C+..jWpp
    Signature Algorithm: sha256WithRSAEncryption
         79:d8:2a:1c:6f:d1:17:77:94:e3:b0:aa:c2:33:35:d1:18:ff:
         0b:06:0d:69:c7:d0:d9:16:c4:77:2c:8b:fa:03:57:c2:fc:39:
         d9:f2:5b:8e:62:e5:57:a0:69:d1:c8:19:93:a0:ed:ae:3e:d4:
         92:16:8f:30:82:91:4a:51:6b:f4:ab:c2:45:c2:7d:36:17:0f:
         1f:90:8b:3c:9f:a3:fd:f0:e3:59:f5:15:d9:9a:89:7d:89:24:
         5a:7f:4f:6a:b8:0d:5d:34:11:32:1d:19:f0:4e:83:19:8e:08:
         34:ac:af:7e:6c:44:77:15:f4:28:5e:d9:59:d6:87:ae:09:dd:
         80:a5:d3:57:2e:d1:98:a2:67:78:fa:95:9f:7d:5a:cf:7f:97:
         be:8d:7d:bd:0c:7c:f3:c8:ae:3e:21:fd:02:0c:42:80:cb:77:
         a4:de:a3:bf:1c:c3:03:76:90:2c:89:cf:c0:1a:2a:78:b1:93:
         fc:7a:d6:b2:bb:ff:f6:3b:e5:50:eb:dd:3c:21:ae:aa:e0:1b:
         d8:83:48:07:0f:74:52:72:cb:f4:cf:df:fe:75:5e:97:f7:44:
         d5:a3:c5:b5:09:71:03:fc:af:a7:c9:c7:7c:51:50:a3:23:6d:
         82:bf:07:72:ea:88:74:50:1e:64:16:b9:fc:58:79:cc:37:bf:
         d0:1f:da:7c
-----BEGIN CERTIFICATE-----
MIIGUzCCBTugAwIBAgIQCuSQchyF5hB4mixoTw9suTANBgkqhkiG9w0BAQsFADBN
MQswCQYDVQQGEwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMScwJQYDVQQDEx5E
aWdpQ2VydCBTSEEyIFNlY3VyZSBTZXJ2ZXIgQ0EwHhcNMTkwMjE5MDAwMDAwWhcN
MjAwNTAyMTIwMDAwWjCBizELMAkGA1UEBhMCVVMxDjAMBgNVBAgTBVRleGFzMQ8w
DQYDVQQHEwZEYWxsYXMxHDAaBgNVBAoME0FUJlQgU2VydmljZXMsIEluYy4xETAP
BgNVBAsMCEFUJlQgSW9UMSowKAYDVQQDEyFzaW1jb250cm9sY2VudGVyLndpcmVs
ZXNzLmF0dC5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDuEbi4
ZOhP6SeSX6EkJ/7jj1Yl38ETnaz/dKGqBSoeWFVFvHLeCCG9lGLw+pFu9AH3qHNn
uJFw3CoaWp6Zzd29uDnwUy0i3mA+Mk/mZ0eD89JTXjcKoe/JFXf9ZaRwiDRap+Ed
60NIKEgB6pc3rQLZIe/eLIuc3DWEyFBQdDIvnBXLqmUxV8RYWXagu6HZzromn1pF
9eit5solZcaPNYIIwcrA6n1DiaZ+oCkRW0mIMsJcrYomdiVhb0T9CkZGMw0G5hcQ
NCeSodaG7rszKKvLVWEuuvjQGZAutJDrtGSA36X5kWzNi4M/run64scRTvpjPGUF
P5CorqqS35ghY0ezAgMBAAGjggLuMIIC6jAfBgNVHSMEGDAWgBQPgGEcgjFh1S8o
541GOLQs4cbZ4jAdBgNVHQ4EFgQUv2tR/XGAcn5kP8Efqr7Tq2aeCCkwLAYDVR0R
BCUwI4Ihc2ltY29udHJvbGNlbnRlci53aXJlbGVzcy5hdHQuY29tMA4GA1UdDwEB
/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwawYDVR0fBGQw
YjAvoC2gK4YpaHR0cDovL2NybDMuZGlnaWNlcnQuY29tL3NzY2Etc2hhMi1nNi5j
cmwwL6AtoCuGKWh0dHA6Ly9jcmw0LmRpZ2ljZXJ0LmNvbS9zc2NhLXNoYTItZzYu
Y3JsMEwGA1UdIARFMEMwNwYJYIZIAYb9bAEBMCowKAYIKwYBBQUHAgEWHGh0dHBz
Oi8vd3d3LmRpZ2ljZXJ0LmNvbS9DUFMwCAYGZ4EMAQICMHwGCCsGAQUFBwEBBHAw
bjAkBggrBgEFBQcwAYYYaHR0cDovL29jc3AuZGlnaWNlcnQuY29tMEYGCCsGAQUF
BzAChjpodHRwOi8vY2FjZXJ0cy5kaWdpY2VydC5jb20vRGlnaUNlcnRTSEEyU2Vj
dXJlU2VydmVyQ0EuY3J0MAkGA1UdEwQCMAAwggEFBgorBgEEAdZ5AgQCBIH2BIHz
APEAdgC72d+8H4pxtZOUI5eqkntHOFeVCqtS6BqQlmQ2jh7RhQAAAWkHN1d+AAAE
AwBHMEUCID7wgEivAUHXWj7R1lOC2Vmh6/9kEizFpo9Gle9YAyr8AiEA9QRhqUfj
j+XWHoPL8QmnEgQ93wl4Tncu3r6QY1UVB04AdwCHdb/nWXz4jEOZX73zbv9WjUdW
Nv9KtWDBtOr/XqCDDwAAAWkHN1i1AAAEAwBIMEYCIQCXepnrSIqlQx53vsw5cz9t
GSCpqxJp9eKkDsqwggdy9gIhAO2zNjFePLCIKFkhMFSlRI3hlfv/0CtL9EMrobpq
V3BwMA0GCSqGSIb3DQEBCwUAA4IBAQB52Cocb9EXd5TjsKrCMzXRGP8LBg1px9DZ
FsR3LIv6A1fC/DnZ8luOYuVXoGnRyBmToO2uPtSSFo8wgpFKUWv0q8JFwn02Fw8f
kIs8n6P98ONZ9RXZmol9iSRaf09quA1dNBEyHRnwToMZjgg0rK9+bER3FfQoXtlZ
1oeuCd2ApdNXLtGYomd4+pWffVrPf5e+jX29DHzzyK4+If0CDEKAy3ek3qO/HMMD
dpAsic/AGip4sZP8etayu//2O+VQ6908Ia6q4BvYg0gHD3RScsv0z9/+dV6X90TV
o8W1CXED/K+nycd8UVCjI22Cvwdy6oh0UB5kFrn8WHnMN7/QH9p8
-----END CERTIFICATE-----
@FiloSottile
Copy link
Contributor

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.

@ConradIrwin
Copy link
Contributor Author

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 crypto/x509 and implementing an http.RoundTripper that uses that, or are there hooks I could use to add this functionality to the existing network stack?

@FiloSottile
Copy link
Contributor

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)).

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.

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 crypto/x509 and implementing an http.RoundTripper that uses that, or are there hooks I could use to add this functionality to the existing network stack?

This would be logic in your custom tls.Config.VerifyPeerCertificate, which you can set in http.Transport.TLSClientConfig.

If the verification fails, you'd look at certs[0].Extensions, find the one with the AIA OID, parse its value with golang.org/x/crypto/cryptobyte, add the CA to the x509.VerifyOptions.Intermediates pool, and try Verify again.

@ConradIrwin
Copy link
Contributor Author

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.

@fcjr
Copy link

fcjr commented Feb 5, 2020

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
}

@ConradIrwin
Copy link
Contributor Author

@fcjr this is awesome!

@FiloSottile
Copy link
Contributor

(Hah, I didn't know we already parse the AIA extension into Certificate.IssuingCertificateURL. Much easier then! By the way I'd recommend using net.SplitHostPort in of tlsHost.)

@rsc
Copy link
Contributor

rsc commented Feb 5, 2020

Adding to proposal minutes but this seems headed for a likely decline.

@fcjr
Copy link

fcjr commented Feb 8, 2020

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.

@rsc
Copy link
Contributor

rsc commented Feb 12, 2020

Based on the discussion above (including the fact that it's so easily implemented outside the standard library), this seems like a likely decline.

@rsc rsc moved this from Active to Likely Decline in Proposals (old) Feb 12, 2020
@rsc
Copy link
Contributor

rsc commented Feb 26, 2020

No change in consensus, so declined.

@rsc rsc moved this from Likely Decline to Declined in Proposals (old) Feb 26, 2020
@rsc rsc closed this as completed Feb 26, 2020
@golang golang locked and limited conversation to collaborators Feb 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
No open projects
Development

No branches or pull requests

5 participants