From 8fa4d15a6adc42e96704af64ed87e875ea4f764d Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 11 Aug 2025 16:12:01 -0700 Subject: [PATCH 1/2] feat: retrieve supported SSH algorithms for FIPS compliance Using the Go 1.24.5 FIPS compiler with SSH causes a panic because the elliptical curve key exchange algorithms are attempted: https://github.com/golang-fips/go/issues/316. To fix this, we need to use only FIPS-supported algorithms. This commit pulls in the changes proposed in https://go-review.googlesource.com/c/crypto/+/550515 for https://github.com/golang/go/issues/64769. --- .gitlab-ci.yml | 2 +- fips/notfips.go | 6 ++++ fips/ssh.go | 64 ++++++++++++++++++++++++++++++++++++++++ fips/ssh_notfips_test.go | 48 ++++++++++++++++++++++++++++++ fips/ssh_test.go | 50 +++++++++++++++++++++++++++++++ go.mod | 9 +++--- go.sum | 20 ++++++++----- 7 files changed, 186 insertions(+), 13 deletions(-) create mode 100644 fips/ssh.go create mode 100644 fips/ssh_notfips_test.go create mode 100644 fips/ssh_test.go diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f4d7822..8942353 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -52,7 +52,7 @@ variables: # NOTE: before changing the `GO_FIPS_VERSION` matrix, # have you reviewed # https://docs.gitlab.com/ee/development/go_guide/go_upgrade.html#testing-against-shipped-go-versions? - - GO_FIPS_VERSION: ["1.22", "1.23"] + - GO_FIPS_VERSION: ["1.22", "1.23", "1.24"] backwards_compat: image: registry.gitlab.com/gitlab-org/gitlab-build-images/debian-bookworm-golang-1.23-rust-1.73:git-2.45 diff --git a/fips/notfips.go b/fips/notfips.go index a67b7ac..d74118e 100644 --- a/fips/notfips.go +++ b/fips/notfips.go @@ -2,6 +2,8 @@ package fips +import "golang.org/x/crypto/ssh" + func Check() bool { return false } @@ -9,3 +11,7 @@ func Check() bool { func Enabled() bool { return false } + +func SupportedAlgorithms() ssh.Algorithms { + return ssh.SupportedAlgorithms() +} diff --git a/fips/ssh.go b/fips/ssh.go new file mode 100644 index 0000000..e4ddcaa --- /dev/null +++ b/fips/ssh.go @@ -0,0 +1,64 @@ +//go:build fips + +package fips + +import ( + "slices" + + "golang.org/x/crypto/ssh" +) + +// fipsKexAlgos specifies FIPS approved key-exchange algorithms implemented +// by this package. +var fipsKexAlgos = []string{ssh.KeyExchangeECDHP256, ssh.KeyExchangeECDHP384} + +// fipsCiphers specifies FIPS approved cipher algorithms implemented by this +// package. +var fipsCiphers = []string{ + ssh.CipherAES128GCM, ssh.CipherAES256GCM, + ssh.CipherAES128CTR, ssh.CipherAES192CTR, ssh.CipherAES256CTR, +} + +// fipsMACs specifies FIPS approved MAC algorithms implemented by this +// package. +var fipsMACs = []string{ + ssh.HMACSHA256ETM, ssh.HMACSHA512ETM, + ssh.HMACSHA256, ssh.HMACSHA512, +} + +// fipsHostKeyAlgos specifies FIPS approved host-key algorithms implemented +// by this package. +var fipsHostKeyAlgos = []string{ + ssh.CertAlgoRSASHA256v01, ssh.CertAlgoRSASHA512v01, + ssh.CertAlgoECDSA256v01, ssh.CertAlgoECDSA384v01, + ssh.KeyAlgoECDSA256, ssh.KeyAlgoECDSA384, + ssh.KeyAlgoRSASHA256, ssh.KeyAlgoRSASHA512, +} + +// fipsPubKeyAuthAlgos specifies FIPS approved public key authentication algorithms. +var fipsPubKeyAuthAlgos = []string{ + ssh.KeyAlgoRSASHA256, ssh.KeyAlgoRSASHA512, + ssh.KeyAlgoECDSA256, ssh.KeyAlgoECDSA384, +} + +type Algorithms struct { + Ciphers []string + MACs []string + KeyExchanges []string + HostKeys []string + PublicKeyAuths []string +} + +// SupportedAlgorithms returns algorithms currently implemented by this package, +// excluding those with security issues, which are returned by +// InsecureAlgorithms. The algorithms listed here are in preference order. +// FIPS 140-2 docs: https://csrc.nist.gov/pubs/fips/140-2/upd2/final +func SupportedAlgorithms() ssh.Algorithms { + return ssh.Algorithms{ + Ciphers: slices.Clone(fipsCiphers), + MACs: slices.Clone(fipsMACs), + KeyExchanges: slices.Clone(fipsKexAlgos), + HostKeys: slices.Clone(fipsHostKeyAlgos), + PublicKeyAuths: slices.Clone(fipsPubKeyAuthAlgos), + } +} diff --git a/fips/ssh_notfips_test.go b/fips/ssh_notfips_test.go new file mode 100644 index 0000000..538cc92 --- /dev/null +++ b/fips/ssh_notfips_test.go @@ -0,0 +1,48 @@ +//go:build !fips + +package fips + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/crypto/ssh" +) + +func TestSupportedAlgorithms_FipsDisabled(t *testing.T) { + // This test runs when FIPS build tag is NOT present + // Should always return ssh.SupportedAlgorithms() + algorithms := SupportedAlgorithms() + expected := ssh.SupportedAlgorithms() + + require.True(t, reflect.DeepEqual(expected, algorithms), + "SupportedAlgorithms() should return ssh.SupportedAlgorithms() when FIPS is not enabled") +} + +func TestSupportedAlgorithmsReturnsValidAlgorithms_NonFips(t *testing.T) { + algorithms := SupportedAlgorithms() + + // Test that all returned algorithms are valid (non-empty strings) + require.NotEmpty(t, algorithms.Ciphers, "Should have cipher algorithms") + require.NotEmpty(t, algorithms.MACs, "Should have MAC algorithms") + require.NotEmpty(t, algorithms.KeyExchanges, "Should have key exchange algorithms") + require.NotEmpty(t, algorithms.HostKeys, "Should have host key algorithms") + require.NotEmpty(t, algorithms.PublicKeyAuths, "Should have public key auth algorithms") + + for _, cipher := range algorithms.Ciphers { + require.NotEmpty(t, cipher, "Cipher algorithm should not be empty") + } + for _, mac := range algorithms.MACs { + require.NotEmpty(t, mac, "MAC algorithm should not be empty") + } + for _, kex := range algorithms.KeyExchanges { + require.NotEmpty(t, kex, "KeyExchange algorithm should not be empty") + } + for _, hostKey := range algorithms.HostKeys { + require.NotEmpty(t, hostKey, "HostKey algorithm should not be empty") + } + for _, pubKey := range algorithms.PublicKeyAuths { + require.NotEmpty(t, pubKey, "PublicKeyAuth algorithm should not be empty") + } +} diff --git a/fips/ssh_test.go b/fips/ssh_test.go new file mode 100644 index 0000000..7722715 --- /dev/null +++ b/fips/ssh_test.go @@ -0,0 +1,50 @@ +//go:build fips + +package fips + +import ( + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/crypto/ssh" +) + +func TestSupportedAlgorithms_FipsEnabled(t *testing.T) { + algorithms := SupportedAlgorithms() + + // When FIPS is enabled, should return FIPS-compliant algorithms + t.Run("FipsEnabled", func(t *testing.T) { + // Test that we get FIPS algorithms + require.Equal(t, fipsCiphers, algorithms.Ciphers, "Ciphers should match FIPS-approved list") + require.Equal(t, fipsMACs, algorithms.MACs, "MACs should match FIPS-approved list") + require.Equal(t, fipsKexAlgos, algorithms.KeyExchanges, "KeyExchanges should match FIPS-approved list") + require.Equal(t, fipsHostKeyAlgos, algorithms.HostKeys, "HostKeys should match FIPS-approved list") + require.Equal(t, fipsPubKeyAuthAlgos, algorithms.PublicKeyAuths, "PublicKeyAuths should match FIPS-approved list") + + // Verify these are clones, not the original slices + require.False(t, &algorithms.Ciphers == &fipsCiphers, "Ciphers should be cloned") + require.False(t, &algorithms.MACs == &fipsMACs, "MACs should be cloned") + require.False(t, &algorithms.KeyExchanges == &fipsKexAlgos, "KeyExchanges should be cloned") + require.False(t, &algorithms.HostKeys == &fipsHostKeyAlgos, "HostKeys should be cloned") + require.False(t, &algorithms.PublicKeyAuths == &fipsPubKeyAuthAlgos, "PublicKeyAuths should be cloned") + + // Verify modifying returned algorithms doesn't affect originals + originalCiphersLen := len(fipsCiphers) + algorithms.Ciphers = append(algorithms.Ciphers, "test-cipher") + require.Equal(t, originalCiphersLen, len(fipsCiphers), "Original fipsCiphers should be unchanged") + }) + + t.Run("FipsAlgorithmsContent", func(t *testing.T) { + // Test that FIPS algorithms contain expected values + require.Contains(t, algorithms.Ciphers, ssh.CipherAES128GCM, "Should contain AES-128-GCM") + require.Contains(t, algorithms.Ciphers, ssh.CipherAES256GCM, "Should contain AES-256-GCM") + require.Contains(t, algorithms.KeyExchanges, ssh.KeyExchangeECDHP256, "Should contain ECDH P-256") + require.Contains(t, algorithms.KeyExchanges, ssh.KeyExchangeECDHP384, "Should contain ECDH P-384") + require.Contains(t, algorithms.MACs, ssh.HMACSHA256, "Should contain HMAC-SHA256") + require.Contains(t, algorithms.MACs, ssh.HMACSHA512, "Should contain HMAC-SHA512") + + // Verify no non-FIPS algorithms are included + require.NotContains(t, algorithms.Ciphers, "chacha20-poly1305@openssh.com", "Should not contain ChaCha20") + require.NotContains(t, algorithms.KeyExchanges, "curve25519-sha256", "Should not contain Curve25519") + }) +} diff --git a/go.mod b/go.mod index a061b98..eb88f80 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,7 @@ require ( github.com/uber/jaeger-client-go v2.29.1+incompatible gitlab.com/gitlab-org/go/reopen v1.0.0 go.opencensus.io v0.23.0 + golang.org/x/crypto v0.33.0 google.golang.org/api v0.54.0 google.golang.org/grpc v1.40.0 gopkg.in/DataDog/dd-trace-go.v1 v1.32.0 @@ -64,11 +65,11 @@ require ( github.com/tklauser/numcpus v0.2.1 // indirect github.com/uber/jaeger-lib v2.4.1+incompatible // indirect go.uber.org/atomic v1.4.0 // indirect - golang.org/x/net v0.29.0 // indirect + golang.org/x/net v0.35.0 // indirect golang.org/x/oauth2 v0.23.0 // indirect - golang.org/x/sync v0.8.0 // indirect - golang.org/x/sys v0.25.0 // indirect - golang.org/x/text v0.18.0 // indirect + golang.org/x/sync v0.16.0 // indirect + golang.org/x/sys v0.35.0 // indirect + golang.org/x/text v0.28.0 // indirect golang.org/x/time v0.0.0-20201208040808-7e3f01d25324 // indirect golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect google.golang.org/appengine v1.6.7 // indirect diff --git a/go.sum b/go.sum index d028362..8c93356 100644 --- a/go.sum +++ b/go.sum @@ -329,6 +329,8 @@ golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.41.0 h1:WKYxWedPGCTVVl5+WHSSrOBT0O8lx32+zxmHxijgXp4= +golang.org/x/crypto v0.41.0/go.mod h1:pO5AFd7FA68rFak7rOAGVuygIISepHftHnr8dr6+sUc= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -405,8 +407,8 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLdyRGr576XBO4/greRjx4P4O3yc= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210503060351-7fd8e65b6420/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/net v0.29.0 h1:5ORfpBpCs4HzDYoodCDBbwHzdR5UrLBZ3sOnUJmFoHo= -golang.org/x/net v0.29.0/go.mod h1:gLkgy8jTGERgjzMic6DS9+SP0ajcu6Xu3Orq/SpETg0= +golang.org/x/net v0.42.0 h1:jzkYrhi3YQWD6MLBJcsklgQsoAcw89EcZbJw8Z614hs= +golang.org/x/net v0.42.0/go.mod h1:FF1RA5d3u7nAYA4z2TkclSCKh68eSXtiFwcWQpPXdt8= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -435,8 +437,8 @@ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ= -golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw= +golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -486,9 +488,11 @@ golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210806184541-e5e7981a1069/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.25.0 h1:r+8e+loiHxRqhXVl6ML1nO3l1+oFoWbnlu2Ehimmi34= -golang.org/x/sys v0.25.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.35.0 h1:vz1N37gP5bs89s7He8XuIYXpyY0+QlsKmzipCbUtyxI= +golang.org/x/sys v0.35.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.34.0 h1:O/2T7POpk0ZZ7MAzMeWFSg6S5IpWd/RXDlM9hgM3DR4= +golang.org/x/term v0.34.0/go.mod h1:5jC53AEywhIVebHgPVeg0mj8OD3VO9OzclacVrqpaAw= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -497,8 +501,8 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.18.0 h1:XvMDiNzPAl0jr17s6W9lcaIhGUfUORdGCNsuLmPG224= -golang.org/x/text v0.18.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= +golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng= +golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= -- GitLab From 72d67bcbadd2f94849c896837f45feff3680c4dc Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 11 Aug 2025 16:23:05 -0700 Subject: [PATCH 2/2] chore: drop Go 1.22 support x/crypto/ssh requires Go 1.23 (https://github.com/golang/crypto/commit/89ff08d67c4d79f9ac619aaf1f7388888798651f), so we need to drop support for Go 1.22. --- .gitlab-ci.yml | 4 ++-- go.mod | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8942353..e687b77 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -44,7 +44,7 @@ variables: # have you reviewed # https://docs.gitlab.com/ee/development/go_guide/go_upgrade.html#testing-against-shipped-go-versions? # Generally, we should always support the latest 3 minor Go versions - - GO_VERSION: ["1.22", "1.23", "1.24"] + - GO_VERSION: ["1.23", "1.24"] .go-fips-version-matrix: parallel: @@ -52,7 +52,7 @@ variables: # NOTE: before changing the `GO_FIPS_VERSION` matrix, # have you reviewed # https://docs.gitlab.com/ee/development/go_guide/go_upgrade.html#testing-against-shipped-go-versions? - - GO_FIPS_VERSION: ["1.22", "1.23", "1.24"] + - GO_FIPS_VERSION: ["1.23", "1.24"] backwards_compat: image: registry.gitlab.com/gitlab-org/gitlab-build-images/debian-bookworm-golang-1.23-rust-1.73:git-2.45 diff --git a/go.mod b/go.mod index eb88f80..080630c 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module gitlab.com/gitlab-org/labkit -go 1.22 +go 1.23.0 require ( cloud.google.com/go/profiler v0.1.0 @@ -19,7 +19,7 @@ require ( github.com/uber/jaeger-client-go v2.29.1+incompatible gitlab.com/gitlab-org/go/reopen v1.0.0 go.opencensus.io v0.23.0 - golang.org/x/crypto v0.33.0 + golang.org/x/crypto v0.41.0 google.golang.org/api v0.54.0 google.golang.org/grpc v1.40.0 gopkg.in/DataDog/dd-trace-go.v1 v1.32.0 @@ -65,7 +65,7 @@ require ( github.com/tklauser/numcpus v0.2.1 // indirect github.com/uber/jaeger-lib v2.4.1+incompatible // indirect go.uber.org/atomic v1.4.0 // indirect - golang.org/x/net v0.35.0 // indirect + golang.org/x/net v0.42.0 // indirect golang.org/x/oauth2 v0.23.0 // indirect golang.org/x/sync v0.16.0 // indirect golang.org/x/sys v0.35.0 // indirect -- GitLab