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

feat: allow go1.21 builds #1424

Merged
merged 14 commits into from
Dec 13, 2023
Merged

feat: allow go1.21 builds #1424

merged 14 commits into from
Dec 13, 2023

Conversation

bassosimone
Copy link
Contributor

@bassosimone bassosimone commented Dec 12, 2023

We can ~safely compile our net/http and crypto/tls forks with a later version of Go, because they use public packages in the standard library, which should be covered by the Go 1.x stability guarantee. Whether that is 100% safe, I don't know, but I suppose we should be fine unless there's really something subtle. (We will continue building our packages with the correct version of Go, but I also understand how distribution maintainers have other needs.)

The real price to pay for compiling with go1.21 is disabling Psiphon. There isn't much we can do here. It's not possible to have a build configuration with fixed flags that disable GQUIC for Psiphon, so I have two choices (a) either ooniprobe does not build for go1.21 because of Psiphon and I need to tell people to apply a flag; or (b) when you compile with go1.21, everything is WAI but for Psiphon. I chose the latter in this pull request.

Note that I originally tried to make oohttp and oocrypto optional. However, it turns out making oohttp optional and using net/http instead cripples ooniprobe entirely. This is why I concluded that maybe compiling go1.20 code using the go1.21 compiler and stdlib is not that bad, considering that our stdlib forks do not (obviously) depend on internals.

Part of ooni/probe#2556; closes ooni/probe#2548.

@bassosimone
Copy link
Contributor Author

bassosimone commented Dec 12, 2023

So, the code up until bd01296 is such that OONI Probe compiles using Go 1.21, however, unfortunately, the generated binary does not work. The reason why it does not work is exemplified by the following failing log message:

[      0.725700] <info> sessionresolver: lookup dkyhjv0wpi2dk.cloudfront.net using https://dns.google/dns-query... started
[      1.227024] <info> sessionresolver: lookup dkyhjv0wpi2dk.cloudfront.net using https://dns.google/dns-query... in progress
[      1.935738] <info> sessionresolver: lookup dkyhjv0wpi2dk.cloudfront.net using https://dns.google/dns-query... unknown_failure: net/http: HTTP/1.x transport connection broken: malformed HTTP response "\x00\x00\x12\x04\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00d\x00\x04\x00\x10\x00\x00\x00\x06\x00\x01\x00\x00\x00\x00\x04\b\x00\x00\x00\x00\x00\x00\x0f\x00\x01\x00\x00\x1e\a\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01http2_handshake_failed"

The TL;DR is that we're using ["h2", "http/1.1"] inside the TLS handshake. However, the Go TLS stack is not prepared to handle h2, therefore we end up failing. Now, one could argue that I can stop sending "h2" as part of the handshake, but this would actually be quite bad because it would further change the TLS handshake.

This means I've reached a bit of a stalemate. The current code cannot fundamentally work. 🤔

@bassosimone bassosimone changed the title Go1.21 feat: allow compiling using go1.21 Dec 13, 2023
@bassosimone
Copy link
Contributor Author

Okay, I've realized I can ~safely use Go 1.20 forked libraries with Go 1.21. This should mostly be fine because of the Go 1.x compatibility guarantee. This means we only need to featurize Psiphon. I'm going to update the original PR text.

@bassosimone bassosimone marked this pull request as ready for review December 13, 2023 09:38
@bassosimone bassosimone requested a review from hellais as a code owner December 13, 2023 09:38
@bassosimone bassosimone mentioned this pull request Dec 13, 2023
24 tasks
@bassosimone bassosimone changed the title feat: allow compiling using go1.21 feat: allow go1.21 builds Dec 13, 2023
@bassosimone bassosimone merged commit f8c6189 into master Dec 13, 2023
9 of 11 checks passed
@bassosimone bassosimone deleted the go1.21 branch December 13, 2023 10:00
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this pull request Feb 13, 2024
We can ~safely compile our net/http and crypto/tls forks with a later
version of Go, because they use public packages in the standard library,
which should be covered by the Go 1.x stability guarantee. Whether that
is 100% safe, I don't know, but I suppose we should be fine unless
there's really something subtle. (We will continue building our packages
with the correct version of Go, but I also understand how distribution
maintainers have other needs.)

The real price to pay for compiling with go1.21 is disabling Psiphon.
There isn't much we can do here. It's not possible to have a build
configuration with fixed flags that disable GQUIC for Psiphon, so I have
two choices (a) either ooniprobe does not build for go1.21 because of
Psiphon and I need to tell people to apply a flag; or (b) when you
compile with go1.21, everything is WAI but for Psiphon. I chose the
latter in this pull request.

Note that I originally tried to make oohttp and oocrypto optional.
However, it turns out making oohttp optional and using net/http instead
cripples ooniprobe entirely. This is why I concluded that maybe
compiling go1.20 code using the go1.21 compiler and stdlib is not that
bad, considering that our stdlib forks do not (obviously) depend on
internals.

Part of ooni/probe#2556; closes
ooni/probe#2548.
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 this pull request may close these issues.

cli, engine: use go1.21
1 participant