google / certificate-transparency-go

Auditing for TLS certificates (Go code)
https://certificate.transparency.dev
Apache License 2.0
903 stars 232 forks source link

Fix failed tests on 32-bit OS #1540

Closed roger2hk closed 2 months ago

roger2hk commented 2 months ago

Fixes #1539.

Checklist

roger2hk commented 2 months ago

@daviddrysdale We would like to avoid any change in the forked x509 package. Should I change the type from int to int64 or skip the test in 32-bit OS?

roger2hk commented 2 months ago

@AlCutter PTAL. The x509/rpki_test.go will only be tested in the listed 64-bit $GOARCH.

AlCutter commented 2 months ago

It feels icky, but I guess it'll unblock the issue.

For posterity, here's the info I dug up which is related to the issue:

Seems like AS numbers are [defined to be unsigned 32bit values](https://www.arin.net/resources/guide/asn/#:~:text=This%20format%20provides%20for%2065%2C536,ASNs%20(0%20to%204294967295), but the ASN.1 is INTEGER used to stored them in X.509 is signed and of arbitrary size.

I guess this means that the rpki stuff as written is also latently semi-broken on 32bit archs - if it encounters a valid cert with ASIDRange.Max > 231-1 it'll just error out when trying to parse it since the target type isn't large enough to contain it.

Another, although potentially breaking, option might be to make ASIDRange.Min and ASIDRange.Max be int64 (See https://pkg.go.dev/encoding/asn1#Unmarshal). Perhaps we can come back and do that if we find out that using this server-side code on 32bit systems is actually a thing.

roger2hk commented 2 months ago

It feels icky, but I guess it'll unblock the issue.

For posterity, here's the info I dug up which is related to the issue:

Seems like AS numbers are [defined to be unsigned 32bit values](https://www.arin.net/resources/guide/asn/#:~:text=This%20format%20provides%20for%2065%2C536,ASNs%20(0%20to%204294967295), but the ASN.1 is INTEGER used to stored them in X.509 is signed and of arbitrary size.

I guess this means that the rpki stuff as written is also latently semi-broken on 32bit archs - if it encounters a valid cert with ASIDRange.Max > 231-1 it'll just error out when trying to parse it since the target type isn't large enough to contain it.

Another, although potentially breaking, option might be to make ASIDRange.Min and ASIDRange.Max be int64 (See https://pkg.go.dev/encoding/asn1#Unmarshal). Perhaps we can come back and do that if we find out that using this server-side code on 32bit systems is actually a thing.

@AlCutter This is a really good write-up of the issue. What if we put this into the rpki package comment?