Implement channel binding support for SCRAM-PLUS - #12
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive channel binding support for SCRAM-PLUS authentication, enabling cryptographic binding of SCRAM authentication to underlying TLS connections as specified in RFC 5802, RFC 5929, and RFC 9266.
Key changes:
- Added
ChannelBindingtype system with support for three binding types:tls-unique,tls-server-end-point, andtls-exporter - Extended client and server with
WithChannelBinding()configuration methods - Updated authentication protocol parsers and conversation logic to handle channel binding negotiation and validation
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| common.go | Introduces ChannelBindingType enum and ChannelBinding struct with helper methods |
| client.go | Adds WithChannelBinding() method and passes binding to conversations |
| client_conv.go | Updates client conversation to include channel binding data in authentication messages |
| server.go | Adds WithChannelBinding() method and passes binding to conversations |
| server_conv.go | Implements server-side channel binding validation logic |
| parse.go | Extends parser to handle p=<cb-type> format in GS2 header |
| doc.go | Updates package documentation with channel binding usage examples |
| README.md | Adds channel binding overview and client example |
| CHANGELOG.md | Documents new channel binding feature |
| channel_binding_test.go | Comprehensive test suite for channel binding functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var expectedCBind []byte | ||
| if sc.clientCBFlag == "p" { | ||
| // Client used channel binding - expect gs2 header + channel binding data | ||
| expectedCBind = append([]byte(sc.gs2Header), sc.channelBinding.Data...) |
There was a problem hiding this comment.
The append operation on line 145 can cause aliasing issues. If []byte(sc.gs2Header) has unused capacity, appending may modify the underlying array that could be shared. Use an explicit slice creation or copy to ensure independence.
| expectedCBind = append([]byte(sc.gs2Header), sc.channelBinding.Data...) | |
| b := make([]byte, len(sc.gs2Header)) | |
| copy(b, sc.gs2Header) | |
| expectedCBind = append(b, sc.channelBinding.Data...) |
Add typed constants for all server error values defined in RFC-5802 and RFC-7677, replacing inline string literals with named constants. This improves code maintainability, prevents typos, and provides better documentation for all valid server error responses. Changes: - Add complete set of server error constants to common.go - Replace all "e=..." literals in server_conv.go with typed constants - All tests pass with no behavioral changes
35f9742 to
619086e
Compare
This commit adds comprehensive channel binding support to enable SCRAM-PLUS authentication variants, addressing issue #10. Core Changes: - Added ChannelBindingType enumeration (tls-unique, tls-server-end-point, tls-exporter) - Added ChannelBinding struct to hold binding type and data - Implemented WithChannelBinding() methods for Client and Server - Updated parsers to handle gs2-cbind-flag "p=<cb-type>" format - Client conversations now include channel binding data in authentication - Server conversations validate channel binding during authentication RFCs Implemented: - RFC 5802: SCRAM protocol with channel binding - RFC 5929: Channel Bindings for TLS (tls-unique, tls-server-end-point) - RFC 9266: Channel Bindings for TLS 1.3 (tls-exporter)
|
Taking this down while I rework the API. |
|
@xdg: Excellent, good job, please inform when it will be perfect for you! |
|
@xdg: PS: When it will be done, please update the "About" description, and add topic with all SCRAM supported versions: In more scram topic: Note: I-Drafts: |
This commit adds comprehensive channel binding support to enable SCRAM-PLUS authentication variants.
Core Changes:
RFCs Implemented: