Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Support tls-exporter channel binding type #243

Merged
merged 1 commit into from
Aug 10, 2022
Merged

Support tls-exporter channel binding type #243

merged 1 commit into from
Aug 10, 2022

Conversation

SamWhited
Copy link
Collaborator

@SamWhited SamWhited commented Jul 19, 2022

Checklist
  • make test passes
  • tests and/or benchmarks are included
  • documentation is changed or added

Affected core subsystem(s)

  • pkg/c2s
  • pkg/auth
  • pkg/transport

Description of change

This adds support for the tls-exporter channel binding type defined in the upcoming RFC 9266 (the RFC is not yet published, but at this stage only last minute editorial changes can be made and it is scheduled for publication). This enables channel binding support for TLS >= 1.3.

EDIT: The RFC has been published.

pkg/transport/socket.go Show resolved Hide resolved
pkg/auth/scram.go Show resolved Hide resolved
@ortuman
Copy link
Owner

ortuman commented Jul 27, 2022

I guess the answer is no, but are we aware of any client to test this new CB support?

If not, I'm ok to merge it anyway, but just wanted to double check.

@SamWhited
Copy link
Collaborator Author

I guess the answer is no, but are we aware of any client to test this new CB support?

I've got my own library that we could manually test against; at some point I'd like to setup some integration tests against Jackal too so that this could be automated. I am not aware of any established clients that support this yet though.

@SamWhited
Copy link
Collaborator Author

SamWhited commented Jul 28, 2022

Ahh, I removed the "SupportsChannelBinding" stuff but I remember why it can't be removed now: it's part of an interface so we need that method otherwise it's not a Transport type. I've added it back.

@SamWhited
Copy link
Collaborator Author

As a way to make testing this easier I've added a package to Alpine Linux for jackal (which is what I use in my integration tests). It's pretty straight forward, but if you have any comments or suggestions the original PR can be found here: https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/37033

Copy link
Owner

@ortuman ortuman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job @SamWhited!

🚀

@ortuman ortuman merged commit 6640d38 into ortuman:main Aug 10, 2022
@SamWhited SamWhited deleted the tls_exporter branch August 10, 2022 11:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants