-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[v15] feat: Allow non-FIPS endpoints on FIPS binaries #51932
Conversation
4b92a33
to
eb9a380
Compare
b06e8c1
to
b318c8b
Compare
b318c8b
to
55f88d2
Compare
55f88d2
to
6887cbe
Compare
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.
Covering specific service clients is hard because whether or not a fips endpoint exists will depend on the aws partition, region, and service.
IAM and applicationautoscaling don't support fips endpoints for us-isob-east-1 for example.
return iam.NewFromConfig(cfg), nil teleport/lib/backend/dynamo/dynamodbbk.go
Line 344 in 6887cbe
if err := SetAutoScaling(ctx, applicationautoscaling.New(b.session), GetTableID(b.TableName), AutoScalingParams{ teleport/lib/events/dynamoevents/dynamoevents.go
Lines 347 to 358 in 6887cbe
if err := dynamo.SetAutoScaling(ctx, applicationautoscaling.New(b.session), dynamo.GetTableID(b.Tablename), dynamo.AutoScalingParams{ ReadMinCapacity: b.Config.ReadMinCapacity, ReadMaxCapacity: b.Config.ReadMaxCapacity, ReadTargetValue: b.Config.ReadTargetValue, WriteMinCapacity: b.Config.WriteMinCapacity, WriteMaxCapacity: b.Config.WriteMaxCapacity, WriteTargetValue: b.Config.WriteTargetValue, }); err != nil { return nil, trace.Wrap(err) } if err := dynamo.SetAutoScaling(ctx, applicationautoscaling.New(b.session), dynamo.GetIndexID(b.Tablename, indexTimeSearchV2), dynamo.AutoScalingParams{
We'll need to do stscreds too:
teleport/lib/utils/aws/credentials.go
Line 75 in 6887cbe
return stscreds.NewCredentials(request.Provider, request.RoleARN, Line 211 in 6887cbe
stsCredentials, err := stscreds.NewCredentials(session, req.Identity.RouteToApp.AWSRoleARN, options...).Get()
lib/backend/dynamo/dynamodbbk.go
Outdated
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.
@GavinFrazar, moving #51932 (review) to a thread.
In general I would rather keep this PR largely as-is, as it's a confirmed fix, and do "larger" changes in a follow-up from master.
I do agree that the approach here is a bit crude, so I'm not opposed to follow-ups to clean it up or make it more uniform. This does have value, though, as a "map" of what changes are needed to get the builds working for the customer.
Covering specific service clients is hard because whether or not a fips endpoint exists will depend on the aws partition, region, and service.
IAM and applicationautoscaling don't support fips endpoints for us-isob-east-1 for example.
Thanks for pointing it out.
We can wrap IAM - I do set at least one of those obeying IsBoringBinary(). I would rather do it in a follow up from master, if that's OK.
I'm not sure applicationautoscaling is a problem - it follows b.session, which is created using a new-blank aws.Config that has no FIPS settings in it. Let me know if I'm reading it wrong, for example if it reads from env variables somewhere.
We'll need to do stscreds too:
Same as IAM, mind if I push this one from master?
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.
iam and stscreds changes, branched from #51924.
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.
We could also go after every IsBoringBinary() call.
PTAL reviewers - I would very much like for this to land before the embargo ends, so it gets into the next release. |
Cherry-picked 0ca6c20 from the equivalent master PR commit (#51924). As discussed with Gavin I'll follow up with a few more changes, from master, after this one (#51932 (comment)). Friendly ping @nklaassen @rosstimothy ? |
Allow FIPS binaries to use non-FIPS AWS endpoints of STS and DynamoDB. Useful for running in AWS regions that lack some of the FIPS services, but mostly ill-advised - talk to your FIPS auditors first.
The "escape hatch" is the TELEPORT_UNSTABLE_DISABLE_AWS_FIPS environment variable. Set it to yes|true|1 to enable it:
TELEPORT_UNSTABLE_DISABLE_AWS_FIPS=yes
.This is not a backport and should be fully-reviewed, as it was tested as a v15 patch for a particular version. Additionally, v15 heavily uses the AWSv1 SDK, whereas master is mostly on AWSv2. It'll be "forward-ported" to master and other active releases.
Changelog: Added an escape hatch to allow non-FIPS AWS endpoints on FIPS binaries (TELEPORT_UNSTABLE_DISABLE_AWS_FIPS=yes).