Skip to content

Commit b2aad4d

Browse files
nklaassennklaassen
authored
fix signature_algorithm_suite checks for HSM and Cloud (#47183)
* fix signature_algorithm_suite checks for HSM and Cloud * fix default suite for cloud * fix comment placement
1 parent c8b364e commit b2aad4d

File tree

7 files changed

+155
-11
lines changed

7 files changed

+155
-11
lines changed

‎api/types/authentication.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,8 @@ type SignatureAlgorithmSuiteParams struct {
591591
// UsingHSMOrKMS should be true if the auth server is configured to
592592
// use an HSM or KMS.
593593
UsingHSMOrKMS bool
594+
// Cloud should be true when running in Teleport Cloud.
595+
Cloud bool
594596
}
595597

596598
// SetDefaultSignatureAlgorithmSuite sets default signature algorithm suite
@@ -600,16 +602,19 @@ func (c *AuthPreferenceV2) SetDefaultSignatureAlgorithmSuite(params SignatureAlg
600602
switch {
601603
case params.FIPS:
602604
c.SetSignatureAlgorithmSuite(SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_FIPS_V1)
603-
case params.UsingHSMOrKMS:
605+
case params.UsingHSMOrKMS || params.Cloud:
606+
// Cloud may eventually migrate existing CA keys to a KMS, to keep
607+
// this option open we default to hsm-v1 suite.
604608
c.SetSignatureAlgorithmSuite(SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_HSM_V1)
605609
default:
606610
c.SetSignatureAlgorithmSuite(SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_BALANCED_V1)
607611
}
608612
}
609613

610614
var (
611-
errNonFIPSSignatureAlgorithmSuite = &trace.BadParameterError{Message: `non-FIPS compliant authentication setting: "signature_algorithm_suite" must be "fips-v1" or "legacy"`}
612-
errNonHSMSignatureAlgorithmSuite = &trace.BadParameterError{Message: `configured "signature_algorithm_suite" is unsupported when "ca_key_params" configures an HSM or KMS, supported values: ["hsm-v1", "fips-v1", "legacy"]`}
615+
errNonFIPSSignatureAlgorithmSuite = &trace.BadParameterError{Message: `non-FIPS compliant authentication setting: "signature_algorithm_suite" must be "fips-v1" or "legacy"`}
616+
errNonHSMSignatureAlgorithmSuite = &trace.BadParameterError{Message: `configured "signature_algorithm_suite" is unsupported when "ca_key_params" configures an HSM or KMS, supported values: ["hsm-v1", "fips-v1", "legacy"]`}
617+
errNonCloudSignatureAlgorithmSuite = &trace.BadParameterError{Message: `configured "signature_algorithm_suite" is unsupported in Teleport Cloud, supported values: ["hsm-v1", "fips-v1", "legacy"]`}
613618
)
614619

615620
// CheckSignatureAlgorithmSuite returns an error if the current signature
@@ -631,6 +636,11 @@ func (c *AuthPreferenceV2) CheckSignatureAlgorithmSuite(params SignatureAlgorith
631636
if params.UsingHSMOrKMS {
632637
return trace.Wrap(errNonHSMSignatureAlgorithmSuite)
633638
}
639+
if params.Cloud {
640+
// Cloud may eventually migrate existing CA keys to a KMS, to keep
641+
// this option open we prevent the balanced-v1 suite.
642+
return trace.Wrap(errNonCloudSignatureAlgorithmSuite)
643+
}
634644
default:
635645
return trace.Errorf("unhandled signature_algorithm_suite %q: this is a bug", c.GetSignatureAlgorithmSuite())
636646
}

‎lib/auth/auth_with_roles.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4670,6 +4670,7 @@ func (a *ServerWithRoles) SetAuthPreference(ctx context.Context, newAuthPref typ
46704670
if err := newAuthPref.CheckSignatureAlgorithmSuite(types.SignatureAlgorithmSuiteParams{
46714671
FIPS: a.authServer.fips,
46724672
UsingHSMOrKMS: a.authServer.keyStore.UsingHSMOrKMS(),
4673+
Cloud: modules.GetModules().Features().Cloud,
46734674
}); err != nil {
46744675
return trace.Wrap(err)
46754676
}

‎lib/auth/grpcserver.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ import (
104104
"github.com/gravitational/teleport/lib/events"
105105
"github.com/gravitational/teleport/lib/httplib"
106106
"github.com/gravitational/teleport/lib/joinserver"
107+
"github.com/gravitational/teleport/lib/modules"
107108
"github.com/gravitational/teleport/lib/observability/metrics"
108109
"github.com/gravitational/teleport/lib/services"
109110
"github.com/gravitational/teleport/lib/services/local"
@@ -5363,6 +5364,7 @@ func NewGRPCServer(cfg GRPCServerConfig) (*GRPCServer, error) {
53635364
SignatureAlgorithmSuiteParams: types.SignatureAlgorithmSuiteParams{
53645365
FIPS: cfg.AuthServer.fips,
53655366
UsingHSMOrKMS: cfg.AuthServer.keyStore.UsingHSMOrKMS(),
5367+
Cloud: modules.GetModules().Features().Cloud,
53665368
},
53675369
})
53685370
if err != nil {

‎lib/auth/init.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,7 @@ func initializeAuthPreference(ctx context.Context, asrv *Server, newAuthPref typ
790790
newAuthPref.SetDefaultSignatureAlgorithmSuite(types.SignatureAlgorithmSuiteParams{
791791
FIPS: asrv.fips,
792792
UsingHSMOrKMS: asrv.keyStore.UsingHSMOrKMS(),
793+
Cloud: modules.GetModules().Features().Cloud,
793794
})
794795
}
795796

‎lib/auth/init_test.go

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ func TestSignatureAlgorithmSuite(t *testing.T) {
197197
testCases := map[string]struct {
198198
fips bool
199199
hsm bool
200+
cloud bool
200201
expectDefaultSuite types.SignatureAlgorithmSuite
201202
expectUnallowedSuites []types.SignatureAlgorithmSuite
202203
}{
@@ -227,15 +228,31 @@ func TestSignatureAlgorithmSuite(t *testing.T) {
227228
types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_HSM_V1,
228229
},
229230
},
231+
"cloud": {
232+
cloud: true,
233+
expectDefaultSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_HSM_V1,
234+
expectUnallowedSuites: []types.SignatureAlgorithmSuite{
235+
types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_BALANCED_V1,
236+
},
237+
},
230238
}
231239

232240
// Test the behavior of auth server init. A default signature algorithm
233241
// suite should never overwrite a persisted signature algorithm suite for an
234242
// existing cluster, even if that was also a default.
235243
t.Run("init", func(t *testing.T) {
236-
t.Parallel()
237244
for desc, tc := range testCases {
238245
t.Run(desc, func(t *testing.T) {
246+
if tc.cloud {
247+
modules.SetTestModules(t, &modules.TestModules{
248+
TestFeatures: modules.Features{
249+
Cloud: true,
250+
Entitlements: map[entitlements.EntitlementKind]modules.EntitlementInfo{
251+
entitlements.HSM: {Enabled: true},
252+
},
253+
},
254+
})
255+
}
239256
// Assert that a fresh cluster gets expected default suite.
240257
cfg := setupInitConfig(t, tc.fips, tc.hsm)
241258
authServer, err := Init(ctx, cfg)
@@ -294,13 +311,28 @@ func TestSignatureAlgorithmSuite(t *testing.T) {
294311
// Test that the auth preference cannot be upserted with a signature
295312
// algorithm suite incompatible with the cluster FIPS and HSM settings.
296313
t.Run("upsert", func(t *testing.T) {
297-
t.Parallel()
298314
for desc, tc := range testCases {
299315
t.Run(desc, func(t *testing.T) {
300-
t.Parallel()
316+
if tc.cloud {
317+
modules.SetTestModules(t, &modules.TestModules{
318+
TestFeatures: modules.Features{
319+
Cloud: true,
320+
Entitlements: map[entitlements.EntitlementKind]modules.EntitlementInfo{
321+
entitlements.HSM: {Enabled: true},
322+
},
323+
},
324+
})
325+
}
301326
cfg := TestAuthServerConfig{
302327
Dir: t.TempDir(),
303328
FIPS: tc.fips,
329+
AuthPreferenceSpec: &types.AuthPreferenceSpecV2{
330+
// Cloud requires second factor enabled.
331+
SecondFactor: constants.SecondFactorOn,
332+
Webauthn: &types.Webauthn{
333+
RPID: "teleport.example.com",
334+
},
335+
},
304336
}
305337
if tc.hsm {
306338
cfg.KeystoreConfig = keystore.HSMTestConfig(t)

‎lib/config/configuration.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import (
6262
"github.com/gravitational/teleport/lib/integrations/externalauditstorage/easconfig"
6363
"github.com/gravitational/teleport/lib/integrations/samlidp/samlidpconfig"
6464
"github.com/gravitational/teleport/lib/limiter"
65+
"github.com/gravitational/teleport/lib/modules"
6566
"github.com/gravitational/teleport/lib/multiplexer"
6667
"github.com/gravitational/teleport/lib/pam"
6768
"github.com/gravitational/teleport/lib/service/servicecfg"
@@ -2552,11 +2553,18 @@ func Configure(clf *CommandLineFlags, cfg *servicecfg.Config, legacyAppFlags boo
25522553
}
25532554
}
25542555

2555-
if err := cfg.Auth.Preference.CheckSignatureAlgorithmSuite(types.SignatureAlgorithmSuiteParams{
2556-
FIPS: clf.FIPS,
2557-
UsingHSMOrKMS: cfg.Auth.KeyStore != (servicecfg.KeystoreConfig{}),
2558-
}); err != nil {
2559-
return trace.Wrap(err)
2556+
if cfg.Auth.Preference.Origin() != types.OriginDefaults {
2557+
// Only check the signature algorithm suite if the auth preference was
2558+
// actually set in the config file. If it wasn't set and the default is
2559+
// used, the algorithm suite will be overwritten in initializeAuthPreference
2560+
// based on the FIPS and HSM settings, and any already persisted auth preference.
2561+
if err := cfg.Auth.Preference.CheckSignatureAlgorithmSuite(types.SignatureAlgorithmSuiteParams{
2562+
FIPS: clf.FIPS,
2563+
UsingHSMOrKMS: cfg.Auth.KeyStore != (servicecfg.KeystoreConfig{}),
2564+
Cloud: modules.GetModules().Features().Cloud,
2565+
}); err != nil {
2566+
return trace.Wrap(err)
2567+
}
25602568
}
25612569

25622570
// apply --skip-version-check flag.

‎lib/config/configuration_test.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import (
5151
"github.com/gravitational/teleport/lib/defaults"
5252
"github.com/gravitational/teleport/lib/fixtures"
5353
"github.com/gravitational/teleport/lib/limiter"
54+
"github.com/gravitational/teleport/lib/modules"
5455
"github.com/gravitational/teleport/lib/service/servicecfg"
5556
"github.com/gravitational/teleport/lib/services"
5657
"github.com/gravitational/teleport/lib/tlsca"
@@ -5048,3 +5049,92 @@ debug_service:
50485049
})
50495050
}
50505051
}
5052+
5053+
func TestSignatureAlgorithmSuite(t *testing.T) {
5054+
5055+
for desc, tc := range map[string]struct {
5056+
fips bool
5057+
hsm bool
5058+
cloud bool
5059+
configuredSuite types.SignatureAlgorithmSuite
5060+
expectErr string
5061+
}{
5062+
"empty": {},
5063+
"hsm with no configured suite": {
5064+
hsm: true,
5065+
},
5066+
"hsm with balanced-v1": {
5067+
hsm: true,
5068+
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_BALANCED_V1,
5069+
expectErr: `configured "signature_algorithm_suite" is unsupported when "ca_key_params" configures an HSM or KMS`,
5070+
},
5071+
"hsm with hsm-v1": {
5072+
hsm: true,
5073+
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_HSM_V1,
5074+
},
5075+
"hsm with fips-v1": {
5076+
hsm: true,
5077+
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_FIPS_V1,
5078+
},
5079+
"hsm with legacy": {
5080+
hsm: true,
5081+
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_LEGACY,
5082+
},
5083+
"fips with no configured suite": {
5084+
fips: true,
5085+
},
5086+
"fips with balanced-v1": {
5087+
fips: true,
5088+
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_BALANCED_V1,
5089+
expectErr: `non-FIPS compliant authentication setting: "signature_algorithm_suite" must be "fips-v1" or "legacy"`,
5090+
},
5091+
"fips with fips-v1": {
5092+
fips: true,
5093+
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_FIPS_V1,
5094+
},
5095+
"fips with legacy": {
5096+
fips: true,
5097+
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_LEGACY,
5098+
},
5099+
"cloud with no configured suite": {
5100+
cloud: true,
5101+
},
5102+
"cloud with balanced-v1": {
5103+
cloud: true,
5104+
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_BALANCED_V1,
5105+
expectErr: `configured "signature_algorithm_suite" is unsupported in Teleport Cloud`,
5106+
},
5107+
} {
5108+
t.Run(desc, func(t *testing.T) {
5109+
modules.SetTestModules(t, &modules.TestModules{
5110+
TestFeatures: modules.Features{
5111+
Cloud: tc.cloud,
5112+
},
5113+
})
5114+
clf := &CommandLineFlags{
5115+
FIPS: tc.fips,
5116+
}
5117+
cfg := servicecfg.MakeDefaultConfig()
5118+
if tc.fips {
5119+
servicecfg.ApplyFIPSDefaults(cfg)
5120+
}
5121+
if tc.hsm {
5122+
cfg.Auth.KeyStore.AWSKMS.AWSAccount = "123456789012"
5123+
cfg.Auth.KeyStore.AWSKMS.AWSRegion = "us-west-2"
5124+
} else {
5125+
cfg.Auth.KeyStore = servicecfg.KeystoreConfig{}
5126+
}
5127+
if tc.configuredSuite != types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_UNSPECIFIED {
5128+
cfg.Auth.Preference.SetOrigin(types.OriginConfigFile)
5129+
cfg.Auth.Preference.SetSignatureAlgorithmSuite(tc.configuredSuite)
5130+
}
5131+
err := Configure(clf, cfg, false)
5132+
if tc.expectErr != "" {
5133+
require.Error(t, err)
5134+
require.ErrorContains(t, err, tc.expectErr)
5135+
return
5136+
}
5137+
require.NoError(t, err)
5138+
})
5139+
}
5140+
}

0 commit comments

Comments
 (0)