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 it possible to use multiple qtls versions at the same time, add support for Go 1.15 #2720

Merged
merged 4 commits into from
Aug 20, 2020

Conversation

marten-seemann
Copy link
Member

Fixes #2614. Obviously, I need to cut a release of qtls-go115 (and rename the repository, since I accidentally pushed old tags there, and probably polluted the Go module cache with those...) before we can merge this PR. Other than that, this PR should be done.

Due to the super-tight coupling between qtls and tls we need to be able to use multiple versions of qtls, depending on the Go version we're using. This is necessary because we need to do what would be called a reinterpret-cast between structs defined in qtls and in crypto/tls, e.g.

var tlsCert *tls.Certificate
var qtlsCert *qtls.Certificate = (*qtls.Certificate)(unsafe.Pointer(tlsCert))

This is obviously unsafe, but should be fine as long as the struct defined in qtls exactly matches the struct defined in crypto/tls.
When Go updates the struct in crypto/tls (as has happened on multiple occasions both for the Go 1.14 and Go 1.15 release), this violates this assumption. We'd need to include multiple versions of qtls. Unfortunately, Go Modules doesn't allow us to do this. We have to decide for one version.
The workaround around this restriction is as straightforward as it is ugly: use separate repositories for the qtls code for Go 1.14 and Go 1.15. Then all we need is a thin shim layer around qtls, which imports the respective version for Go 1.14 and Go 1.15.

qtls-go115 goes a few steps further than the Go 1.14 version, in that it defines even more type aliases, especially for the tls.Config. This means that as a tls.Config is now essentially as tls.Config. This allows us to get rid of a lot of conversion code.

@codecov
Copy link

codecov bot commented Aug 18, 2020

Codecov Report

Merging #2720 into master will decrease coverage by 0.27%.
The diff coverage is 74.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2720      +/-   ##
==========================================
- Coverage   86.75%   86.49%   -0.27%     
==========================================
  Files         124      126       +2     
  Lines        9895     9952      +57     
==========================================
+ Hits         8584     8607      +23     
- Misses        979     1013      +34     
  Partials      332      332              
Impacted Files Coverage Δ
internal/handshake/aead.go 96.36% <ø> (ø)
internal/handshake/initial_aead.go 100.00% <ø> (ø)
internal/handshake/tls_extension_handler.go 100.00% <ø> (ø)
internal/handshake/updatable_aead.go 91.34% <ø> (ø)
internal/qerr/error_codes.go 100.00% <ø> (ø)
internal/qtls/go114.go 36.36% <36.36%> (ø)
internal/qtls/go115.go 36.36% <36.36%> (ø)
internal/handshake/crypto_setup.go 67.20% <77.42%> (+0.25%) ⬆️
internal/qtls/qtls.go 85.96% <85.96%> (ø)
internal/handshake/header_protector.go 83.05% <100.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 705f17d...125318d. Read the comment docs.

@marten-seemann marten-seemann force-pushed the qtls-multiple-versions branch 3 times, most recently from e6e7054 to 837061c Compare August 20, 2020 06:20
@marten-seemann marten-seemann merged commit 2f736d3 into master Aug 20, 2020
@marten-seemann marten-seemann deleted the qtls-multiple-versions branch August 20, 2020 07:28
@aschmahmann aschmahmann mentioned this pull request Sep 22, 2020
72 tasks
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.

support Go 1.15
2 participants