-
Notifications
You must be signed in to change notification settings - Fork 269
crypto: add TLS group/curve mapping support #2347
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -211,6 +211,39 @@ var ciphersUnsupportedByGo = map[string]string{ | |||||
| "AES256-SHA256": "TLS_RSA_WITH_AES_256_CBC_SHA256", | ||||||
| } | ||||||
|
|
||||||
| // goTLSGroups lists all TLS supported groups (key exchange mechanisms) recognized | ||||||
| // by Go's crypto/tls, keyed by Go constant name. Kept in sync with the crypto/tls | ||||||
| // package by TestConstantMaps. | ||||||
| // Ref: https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-8 | ||||||
| var goTLSGroups = map[string]tls.CurveID{ | ||||||
| "CurveP256": tls.CurveP256, // IANA 23 | ||||||
| "CurveP384": tls.CurveP384, // IANA 24 | ||||||
| "CurveP521": tls.CurveP521, // IANA 25 | ||||||
| "X25519": tls.X25519, // IANA 29 | ||||||
| "X25519MLKEM768": tls.X25519MLKEM768, // IANA 4588 | ||||||
| // TODO: add "SecP256r1MLKEM768": tls.SecP256r1MLKEM768 (IANA 4587) when Go 1.26 is the minimum version | ||||||
| // TODO: add "SecP384r1MLKEM1024": tls.SecP384r1MLKEM1024 (IANA 4590) when Go 1.26 is the minimum version | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
https://www.ietf.org/archive/id/draft-ietf-tls-ecdhe-mlkem-01.html#name-secp384r1mlkem1024 probably safer to document as 4589 - unless I'm incorrect and this is something specific to Go |
||||||
| } | ||||||
|
|
||||||
| // tlsGroupToCurveID maps OpenShift API TLSGroup values to Go's tls.CurveID. | ||||||
| // | ||||||
| // SecP256r1MLKEM768 (IANA 4587) and SecP384r1MLKEM1024 (IANA 4590) use raw | ||||||
| // numeric CurveID values because Go 1.25 does not yet define named constants | ||||||
| // for them. Go will silently ignore CurveIDs it does not implement, so these | ||||||
| // entries are no-ops on Go 1.25 and become effective on Go 1.26+. | ||||||
| // TODO: replace the raw IANA IDs with tls.SecP256r1MLKEM768 and | ||||||
| // tls.SecP384r1MLKEM1024 once Go 1.26 is the minimum version, and add | ||||||
| // the corresponding entries to goTLSGroups. | ||||||
| var tlsGroupToCurveID = map[configv1.TLSGroup]tls.CurveID{ | ||||||
| configv1.TLSGroupX25519: tls.X25519, | ||||||
| configv1.TLSGroupSecP256r1: tls.CurveP256, | ||||||
| configv1.TLSGroupSecP384r1: tls.CurveP384, | ||||||
| configv1.TLSGroupSecP521r1: tls.CurveP521, | ||||||
| configv1.TLSGroupX25519MLKEM768: tls.X25519MLKEM768, | ||||||
| configv1.TLSGroupSecP256r1MLKEM768: tls.CurveID(4587), // TODO: replace with tls.SecP256r1MLKEM768 when Go 1.26 is the minimum version | ||||||
| configv1.TLSGroupSecP384r1MLKEM1024: tls.CurveID(4590), // TODO: replace with tls.SecP384r1MLKEM1024 when Go 1.26 is the minimum version | ||||||
| } | ||||||
|
|
||||||
| // CipherSuitesToNamesOrDie given a list of cipher suites as ints, return their readable names | ||||||
| func CipherSuitesToNamesOrDie(intVals []uint16) []string { | ||||||
| ret := []string{} | ||||||
|
|
@@ -356,6 +389,43 @@ func OpenSSLToIANACipherSuites(ciphers []string) []string { | |||||
| return ianaCiphers | ||||||
| } | ||||||
|
|
||||||
| // CurveIDForTLSGroup maps an OpenShift API TLSGroup constant to Go's tls.CurveID. | ||||||
| // Returns (0, false) if the group is not supported by this Go version. | ||||||
| func CurveIDForTLSGroup(group configv1.TLSGroup) (tls.CurveID, bool) { | ||||||
| id, ok := tlsGroupToCurveID[group] | ||||||
| return id, ok | ||||||
| } | ||||||
|
damdo marked this conversation as resolved.
|
||||||
|
|
||||||
| // CurveIDsForTLSGroups converts a slice of TLSGroup values to their tls.CurveID | ||||||
| // codes. Returns the supported curve IDs and a list of unsupported group names. | ||||||
| // Unsupported groups are silently filtered — callers should log warnings. | ||||||
| func CurveIDsForTLSGroups(groups []configv1.TLSGroup) ([]tls.CurveID, []string) { | ||||||
| var curves []tls.CurveID | ||||||
| var unsupported []string | ||||||
|
|
||||||
| for _, group := range groups { | ||||||
| id, ok := CurveIDForTLSGroup(group) | ||||||
| if !ok { | ||||||
| unsupported = append(unsupported, string(group)) | ||||||
| continue | ||||||
| } | ||||||
| curves = append(curves, id) | ||||||
| } | ||||||
|
|
||||||
| return curves, unsupported | ||||||
| } | ||||||
|
|
||||||
| // ValidTLSGroups returns the TLS group names that this Go version supports, | ||||||
| // sorted alphabetically. | ||||||
| func ValidTLSGroups() []string { | ||||||
| groups := make([]string, 0, len(tlsGroupToCurveID)) | ||||||
| for g := range tlsGroupToCurveID { | ||||||
| groups = append(groups, string(g)) | ||||||
| } | ||||||
| sort.Strings(groups) | ||||||
| return groups | ||||||
| } | ||||||
|
|
||||||
| type TLSCertificateConfig struct { | ||||||
| Certs []*x509.Certificate | ||||||
| Key crypto.PrivateKey | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ package crypto | |
|
|
||
| import ( | ||
| "crypto" | ||
| "crypto/tls" | ||
| "crypto/x509" | ||
| "crypto/x509/pkix" | ||
| "fmt" | ||
|
|
@@ -36,13 +37,17 @@ func TestConstantMaps(t *testing.T) { | |
| } | ||
| discoveredVersions := map[string]bool{} | ||
| discoveredCiphers := map[string]bool{} | ||
| discoveredGroups := map[string]bool{} | ||
| for _, declName := range pkg.Scope().Names() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should have a
And add it to the respective set. |
||
| if strings.HasPrefix(declName, "VersionTLS") { | ||
| discoveredVersions[declName] = true | ||
| } | ||
| if strings.HasPrefix(declName, "TLS_RSA_") || strings.HasPrefix(declName, "TLS_ECDHE_") || strings.HasPrefix(declName, "TLS_AES_") || strings.HasPrefix(declName, "TLS_CHACHA20_") { | ||
| discoveredCiphers[declName] = true | ||
| } | ||
| if strings.HasPrefix(declName, "CurveP") || strings.HasPrefix(declName, "X25519") { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this filter need SecP and MLKEM prefixes? Or will that be updated when addressing the TODOs for go 1.26 >= ? |
||
| discoveredGroups[declName] = true | ||
| } | ||
| } | ||
|
|
||
| for k := range discoveredCiphers { | ||
|
|
@@ -73,6 +78,17 @@ func TestConstantMaps(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| for k := range discoveredGroups { | ||
| if _, ok := goTLSGroups[k]; !ok { | ||
| t.Errorf("discovered group tls.%s not in goTLSGroups map", k) | ||
| } | ||
| } | ||
| for k := range goTLSGroups { | ||
| if _, ok := discoveredGroups[k]; !ok { | ||
| t.Errorf("goTLSGroups map has %s not in tls package", k) | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| func TestCrypto(t *testing.T) { | ||
|
|
@@ -598,3 +614,113 @@ func TestCiphersUnsupportedByGoAreActuallyUnsupported(t *testing.T) { | |
| } | ||
| } | ||
| } | ||
|
|
||
| // TestTLSProfileGroupsHaveMappings verifies that all TLS groups defined in the | ||
| // OpenShift TLS security profiles have corresponding mappings in | ||
| // tlsGroupToCurveID. | ||
| func TestTLSProfileGroupsHaveMappings(t *testing.T) { | ||
| var missingMappings []string | ||
|
|
||
| for profileType, profileSpec := range configv1.TLSProfiles { | ||
| for _, group := range profileSpec.Groups { | ||
| if _, found := tlsGroupToCurveID[group]; !found { | ||
| missingMappings = append(missingMappings, fmt.Sprintf("%s (profile: %s)", group, profileType)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if len(missingMappings) > 0 { | ||
| sort.Strings(missingMappings) | ||
| t.Errorf("The following TLS groups from TLS profiles are missing mappings in tlsGroupToCurveID:\n%s", | ||
| strings.Join(missingMappings, "\n")) | ||
| } | ||
| } | ||
|
|
||
| func TestCurveIDForTLSGroup(t *testing.T) { | ||
| tests := []struct { | ||
| group configv1.TLSGroup | ||
| wantID tls.CurveID | ||
| wantOK bool | ||
| }{ | ||
| {configv1.TLSGroupX25519, tls.X25519, true}, | ||
| {configv1.TLSGroupSecP256r1, tls.CurveP256, true}, | ||
| {configv1.TLSGroupSecP384r1, tls.CurveP384, true}, | ||
| {configv1.TLSGroupSecP521r1, tls.CurveP521, true}, | ||
| {configv1.TLSGroupX25519MLKEM768, tls.X25519MLKEM768, true}, | ||
| {configv1.TLSGroupSecP256r1MLKEM768, tls.CurveID(4587), true}, // TODO: replace with tls.SecP256r1MLKEM768 when Go 1.26 is the minimum version | ||
| {configv1.TLSGroupSecP384r1MLKEM1024, tls.CurveID(4590), true}, // TODO: replace with tls.SecP384r1MLKEM1024 when Go 1.26 is the minimum version | ||
| {configv1.TLSGroup("UnknownGroup"), 0, false}, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(string(tt.group), func(t *testing.T) { | ||
| gotID, gotOK := CurveIDForTLSGroup(tt.group) | ||
| if gotID != tt.wantID { | ||
| t.Errorf("CurveIDForTLSGroup(%q) id = %d, want %d", tt.group, gotID, tt.wantID) | ||
| } | ||
| if gotOK != tt.wantOK { | ||
| t.Errorf("CurveIDForTLSGroup(%q) ok = %v, want %v", tt.group, gotOK, tt.wantOK) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestCurveIDsForTLSGroups(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| groups []configv1.TLSGroup | ||
| wantCurves []tls.CurveID | ||
| wantUnsupported []string | ||
| }{ | ||
| { | ||
| name: "all supported", | ||
| groups: []configv1.TLSGroup{configv1.TLSGroupX25519, configv1.TLSGroupSecP256r1}, | ||
| wantCurves: []tls.CurveID{tls.X25519, tls.CurveP256}, | ||
| wantUnsupported: nil, | ||
| }, | ||
| { | ||
| name: "includes post-quantum hybrids", | ||
| groups: []configv1.TLSGroup{configv1.TLSGroupX25519, configv1.TLSGroupSecP256r1MLKEM768, configv1.TLSGroupSecP384r1}, | ||
| wantCurves: []tls.CurveID{tls.X25519, tls.CurveID(4587) /* TODO: replace with tls.SecP256r1MLKEM768 when Go 1.26 is the minimum version */, tls.CurveP384}, | ||
| wantUnsupported: nil, | ||
| }, | ||
| { | ||
| name: "unknown groups filtered", | ||
| groups: []configv1.TLSGroup{configv1.TLSGroupX25519, configv1.TLSGroup("FutureGroup")}, | ||
| wantCurves: []tls.CurveID{tls.X25519}, | ||
| wantUnsupported: []string{"FutureGroup"}, | ||
| }, | ||
| { | ||
| name: "empty input", | ||
| groups: []configv1.TLSGroup{}, | ||
| wantCurves: nil, | ||
| wantUnsupported: nil, | ||
| }, | ||
| { | ||
| name: "nil input", | ||
| groups: nil, | ||
| wantCurves: nil, | ||
| wantUnsupported: nil, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| gotCurves, gotUnsupported := CurveIDsForTLSGroups(tt.groups) | ||
| require.Equal(t, tt.wantCurves, gotCurves) | ||
| require.Equal(t, tt.wantUnsupported, gotUnsupported) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestValidTLSGroups(t *testing.T) { | ||
| groups := ValidTLSGroups() | ||
| if len(groups) != len(tlsGroupToCurveID) { | ||
| t.Errorf("ValidTLSGroups() returned %d groups, want %d", len(groups), len(tlsGroupToCurveID)) | ||
| } | ||
| for i := 1; i < len(groups); i++ { | ||
| if groups[i] < groups[i-1] { | ||
| t.Errorf("ValidTLSGroups() not sorted: %q before %q", groups[i-1], groups[i]) | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be antipattern, but we could reduce all these TODOs by creating a separate file with something like:
Not sure if we normally leverage this in the project, just trying to think of ways to not have to come back to the TODOs 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a good idea to me. This would make it possible for projects using older go to continue using newer library-go.