Skip to content

Commit

Permalink
fix signature_algorithm_suite checks for HSM and Cloud (#47183)
Browse files Browse the repository at this point in the history 
* fix signature_algorithm_suite checks for HSM and Cloud

* fix default suite for cloud

* fix comment placement
  • Loading branch information
@nklaassen
nklaassen authored Oct 5, 2024
1 parent c8b364e commit b2aad4d
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 11 deletions.
16 changes: 13 additions & 3 deletions api/types/authentication.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,8 @@ type SignatureAlgorithmSuiteParams struct {
// UsingHSMOrKMS should be true if the auth server is configured to
// use an HSM or KMS.
UsingHSMOrKMS bool
// Cloud should be true when running in Teleport Cloud.
Cloud bool
}

// SetDefaultSignatureAlgorithmSuite sets default signature algorithm suite
Expand All @@ -600,16 +602,19 @@ func (c *AuthPreferenceV2) SetDefaultSignatureAlgorithmSuite(params SignatureAlg
switch {
case params.FIPS:
c.SetSignatureAlgorithmSuite(SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_FIPS_V1)
case params.UsingHSMOrKMS:
case params.UsingHSMOrKMS || params.Cloud:
// Cloud may eventually migrate existing CA keys to a KMS, to keep
// this option open we default to hsm-v1 suite.
c.SetSignatureAlgorithmSuite(SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_HSM_V1)
default:
c.SetSignatureAlgorithmSuite(SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_BALANCED_V1)
}
}

var (
errNonFIPSSignatureAlgorithmSuite = &trace.BadParameterError{Message: `non-FIPS compliant authentication setting: "signature_algorithm_suite" must be "fips-v1" or "legacy"`}
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"]`}
errNonFIPSSignatureAlgorithmSuite = &trace.BadParameterError{Message: `non-FIPS compliant authentication setting: "signature_algorithm_suite" must be "fips-v1" or "legacy"`}
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"]`}
errNonCloudSignatureAlgorithmSuite = &trace.BadParameterError{Message: `configured "signature_algorithm_suite" is unsupported in Teleport Cloud, supported values: ["hsm-v1", "fips-v1", "legacy"]`}
)

// CheckSignatureAlgorithmSuite returns an error if the current signature
Expand All @@ -631,6 +636,11 @@ func (c *AuthPreferenceV2) CheckSignatureAlgorithmSuite(params SignatureAlgorith
if params.UsingHSMOrKMS {
return trace.Wrap(errNonHSMSignatureAlgorithmSuite)
}
if params.Cloud {
// Cloud may eventually migrate existing CA keys to a KMS, to keep
// this option open we prevent the balanced-v1 suite.
return trace.Wrap(errNonCloudSignatureAlgorithmSuite)
}
default:
return trace.Errorf("unhandled signature_algorithm_suite %q: this is a bug", c.GetSignatureAlgorithmSuite())
}
Expand Down
1 change: 1 addition & 0 deletions lib/auth/auth_with_roles.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4670,6 +4670,7 @@ func (a *ServerWithRoles) SetAuthPreference(ctx context.Context, newAuthPref typ
if err := newAuthPref.CheckSignatureAlgorithmSuite(types.SignatureAlgorithmSuiteParams{
FIPS: a.authServer.fips,
UsingHSMOrKMS: a.authServer.keyStore.UsingHSMOrKMS(),
Cloud: modules.GetModules().Features().Cloud,
}); err != nil {
return trace.Wrap(err)
}
Expand Down
2 changes: 2 additions & 0 deletions lib/auth/grpcserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ import (
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/httplib"
"github.com/gravitational/teleport/lib/joinserver"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/observability/metrics"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/services/local"
Expand Down Expand Up @@ -5363,6 +5364,7 @@ func NewGRPCServer(cfg GRPCServerConfig) (*GRPCServer, error) {
SignatureAlgorithmSuiteParams: types.SignatureAlgorithmSuiteParams{
FIPS: cfg.AuthServer.fips,
UsingHSMOrKMS: cfg.AuthServer.keyStore.UsingHSMOrKMS(),
Cloud: modules.GetModules().Features().Cloud,
},
})
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions lib/auth/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,7 @@ func initializeAuthPreference(ctx context.Context, asrv *Server, newAuthPref typ
newAuthPref.SetDefaultSignatureAlgorithmSuite(types.SignatureAlgorithmSuiteParams{
FIPS: asrv.fips,
UsingHSMOrKMS: asrv.keyStore.UsingHSMOrKMS(),
Cloud: modules.GetModules().Features().Cloud,
})
}

Expand Down
38 changes: 35 additions & 3 deletions lib/auth/init_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ func TestSignatureAlgorithmSuite(t *testing.T) {
testCases := map[string]struct {
fips bool
hsm bool
cloud bool
expectDefaultSuite types.SignatureAlgorithmSuite
expectUnallowedSuites []types.SignatureAlgorithmSuite
}{
Expand Down Expand Up @@ -227,15 +228,31 @@ func TestSignatureAlgorithmSuite(t *testing.T) {
types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_HSM_V1,
},
},
"cloud": {
cloud: true,
expectDefaultSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_HSM_V1,
expectUnallowedSuites: []types.SignatureAlgorithmSuite{
types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_BALANCED_V1,
},
},
}

// Test the behavior of auth server init. A default signature algorithm
// suite should never overwrite a persisted signature algorithm suite for an
// existing cluster, even if that was also a default.
t.Run("init", func(t *testing.T) {
t.Parallel()
for desc, tc := range testCases {
t.Run(desc, func(t *testing.T) {
if tc.cloud {
modules.SetTestModules(t, &modules.TestModules{
TestFeatures: modules.Features{
Cloud: true,
Entitlements: map[entitlements.EntitlementKind]modules.EntitlementInfo{
entitlements.HSM: {Enabled: true},
},
},
})
}
// Assert that a fresh cluster gets expected default suite.
cfg := setupInitConfig(t, tc.fips, tc.hsm)
authServer, err := Init(ctx, cfg)
Expand Down Expand Up @@ -294,13 +311,28 @@ func TestSignatureAlgorithmSuite(t *testing.T) {
// Test that the auth preference cannot be upserted with a signature
// algorithm suite incompatible with the cluster FIPS and HSM settings.
t.Run("upsert", func(t *testing.T) {
t.Parallel()
for desc, tc := range testCases {
t.Run(desc, func(t *testing.T) {
t.Parallel()
if tc.cloud {
modules.SetTestModules(t, &modules.TestModules{
TestFeatures: modules.Features{
Cloud: true,
Entitlements: map[entitlements.EntitlementKind]modules.EntitlementInfo{
entitlements.HSM: {Enabled: true},
},
},
})
}
cfg := TestAuthServerConfig{
Dir: t.TempDir(),
FIPS: tc.fips,
AuthPreferenceSpec: &types.AuthPreferenceSpecV2{
// Cloud requires second factor enabled.
SecondFactor: constants.SecondFactorOn,
Webauthn: &types.Webauthn{
RPID: "teleport.example.com",
},
},
}
if tc.hsm {
cfg.KeystoreConfig = keystore.HSMTestConfig(t)
Expand Down
18 changes: 13 additions & 5 deletions lib/config/configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import (
"github.com/gravitational/teleport/lib/integrations/externalauditstorage/easconfig"
"github.com/gravitational/teleport/lib/integrations/samlidp/samlidpconfig"
"github.com/gravitational/teleport/lib/limiter"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/multiplexer"
"github.com/gravitational/teleport/lib/pam"
"github.com/gravitational/teleport/lib/service/servicecfg"
Expand Down Expand Up @@ -2552,11 +2553,18 @@ func Configure(clf *CommandLineFlags, cfg *servicecfg.Config, legacyAppFlags boo
}
}

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

// apply --skip-version-check flag.
Expand Down
90 changes: 90 additions & 0 deletions lib/config/configuration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import (
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/fixtures"
"github.com/gravitational/teleport/lib/limiter"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/service/servicecfg"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/tlsca"
Expand Down Expand Up @@ -5048,3 +5049,92 @@ debug_service:
})
}
}

func TestSignatureAlgorithmSuite(t *testing.T) {

for desc, tc := range map[string]struct {
fips bool
hsm bool
cloud bool
configuredSuite types.SignatureAlgorithmSuite
expectErr string
}{
"empty": {},
"hsm with no configured suite": {
hsm: true,
},
"hsm with balanced-v1": {
hsm: true,
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_BALANCED_V1,
expectErr: `configured "signature_algorithm_suite" is unsupported when "ca_key_params" configures an HSM or KMS`,
},
"hsm with hsm-v1": {
hsm: true,
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_HSM_V1,
},
"hsm with fips-v1": {
hsm: true,
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_FIPS_V1,
},
"hsm with legacy": {
hsm: true,
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_LEGACY,
},
"fips with no configured suite": {
fips: true,
},
"fips with balanced-v1": {
fips: true,
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_BALANCED_V1,
expectErr: `non-FIPS compliant authentication setting: "signature_algorithm_suite" must be "fips-v1" or "legacy"`,
},
"fips with fips-v1": {
fips: true,
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_FIPS_V1,
},
"fips with legacy": {
fips: true,
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_LEGACY,
},
"cloud with no configured suite": {
cloud: true,
},
"cloud with balanced-v1": {
cloud: true,
configuredSuite: types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_BALANCED_V1,
expectErr: `configured "signature_algorithm_suite" is unsupported in Teleport Cloud`,
},
} {
t.Run(desc, func(t *testing.T) {
modules.SetTestModules(t, &modules.TestModules{
TestFeatures: modules.Features{
Cloud: tc.cloud,
},
})
clf := &CommandLineFlags{
FIPS: tc.fips,
}
cfg := servicecfg.MakeDefaultConfig()
if tc.fips {
servicecfg.ApplyFIPSDefaults(cfg)
}
if tc.hsm {
cfg.Auth.KeyStore.AWSKMS.AWSAccount = "123456789012"
cfg.Auth.KeyStore.AWSKMS.AWSRegion = "us-west-2"
} else {
cfg.Auth.KeyStore = servicecfg.KeystoreConfig{}
}
if tc.configuredSuite != types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_UNSPECIFIED {
cfg.Auth.Preference.SetOrigin(types.OriginConfigFile)
cfg.Auth.Preference.SetSignatureAlgorithmSuite(tc.configuredSuite)
}
err := Configure(clf, cfg, false)
if tc.expectErr != "" {
require.Error(t, err)
require.ErrorContains(t, err, tc.expectErr)
return
}
require.NoError(t, err)
})
}
}

0 comments on commit b2aad4d

Please sign in to comment.