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

Make tests robust to non-AES ciphersuites #267

Closed
jdtw opened this issue Jun 1, 2022 · 3 comments · Fixed by #268
Closed

Make tests robust to non-AES ciphersuites #267

jdtw opened this issue Jun 1, 2022 · 3 comments · Fixed by #268

Comments

@jdtw
Copy link
Contributor

jdtw commented Jun 1, 2022

I was a bit late to chime in with a review before this got merged.
One other source of test failures was the assumption of AES_128_GCM_SHA256 being selected by both the httptest spawned TLS server and the unit test TLS client.

Go alters the default ciphersuite to prefer ChaPoly over AES-GCM suites if the underlying hardware doesn't have AES extensions. (https://cs.opensource.google/go/go/+/master:src/crypto/tls/handshake_server_tls13.go;l=158?q=defaultCipherSuitesTLS13NoAES%20&ss=go%2Fgo). So CI/CD systems running under virtualization may not always reflect the underlying CPUs extensions. One good example—and how I ended up debugging this issue—was NixOS CI system for Darwin x86-64 wasn't picking AES. Temporarily I ended up disabling tests on darwin-x86_64.

So the lines 110 and 138 for expectedConnect string in TestConnect trying to match Cipher Suite: AES_128_GCM_SHA256 cipher will continue causing test failures where Go cannot deduce presence of hardware acceleration for AES.

Originally posted by @bdd in #265 (comment)

@jdtw
Copy link
Contributor Author

jdtw commented Jun 1, 2022

Hardcoding TLS 1.3 is also fragile.

@bdd
Copy link

bdd commented Jun 1, 2022

Hardcoding TLS 1.3 is also fragile.

Certigo requires at least Go 1.13.
Go added TLS 1.3 support in 1.12 as opt-in and promoted it to on-by-default in 1.13.
Go always chooses MaxVersion. Due to the nature of the unit test, the server and the client are instantiated in the same process. So as long as there's no TLS 1.4 in Go, you're good. That's gonna take long years to land :)

@jdtw
Copy link
Contributor Author

jdtw commented Jun 1, 2022

Yeah, I was imaging TLS 1.4 :). But those 20 year certs in the test will probably expire before then...

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

Successfully merging a pull request may close this issue.

2 participants