Configure FIPS AWS service endpoints #218
Conversation
ginglis13
force-pushed
the
fips-aws-endpoints
branch
from
8c2892b to
724d354
Compare
|
^ force push adds unit test for the |
ginglis13
force-pushed
the
fips-aws-endpoints
branch
2 times, most recently
from
acc7c84 to
17b40b9
Compare
|
^ force pushes stores the AWS config as a tempfile, removes the |
ginglis13
force-pushed
the
fips-aws-endpoints
branch
from
17b40b9 to
7ab6eb1
Compare
|
^ rebase |
| let aws_profile = get_param(helper, 1)?; | ||
| let aws_profile = aws_profile |
There was a problem hiding this comment.
We should not assume aws_profile is defined; it could be null if the downstream build doesn't define a default value for it.
There was a problem hiding this comment.
Does that also then apply to some of the other helpers we have, such as ecr-prefix (e.g. if a downstream has no default region):
bottlerocket-core-kit/sources/api/schnauzer/src/helpers/mod.rs
Lines 467 to 473 in a10182f
My understanding is that if this isn't defined, in both this new helper and in ecr-prefix, the helper will fail. Would it be preffered to just silently return and do nothing?
There was a problem hiding this comment.
Does that also then apply to some of the other helpers we have, such as ecr-prefix ...?
Likely yes - this is an artifact of the in-tree world, where we "knew" that all variants would have a default value because we could review or update the defaults at the same time we added these helpers.
There was a problem hiding this comment.
Sounds good. I can backlog an issue for that. As for this PR, I've removed the dependency on settings.aws.profile for this helper given our other comment thread discussing only setting this config for the default profile
| /// Constructs the base64 encoded AWS config for a variant, setting | ||
| /// `use_fips_endpoint`. | ||
| fn build_aws_config<S: AsRef<str>>(profile: S, fips_enabled: bool) -> String { | ||
| let aws_config_str = format!( | ||
| r#"[{}] |
There was a problem hiding this comment.
Assuming that the AWS SDKs for Rust and Go expect the same syntax as the AWS CLI, we have to render this differently for default and non-default profiles:
[default]
use_fips_endpoint = true
[profile other]
use-fips-endpoint = true
I'm not sure if [profile default] is proper syntax, though it's mentioned in the Bottlerocket docs. I would guess it means the profile that's used if you specify --profile default rather than the configuration that's used if you do not specify a profile. I would avoid writing a config file that uses this syntax since it's confusing even if it's not incorrect.
However, I also don't know how functional a named profile could be without a corresponding config file being supplied also. So we might want to do this instead:
- if profile is null, assume "default"
- if profile is "default", and config is null, render this config
- if profile is anything else, skip it - assume the admin knows what they're doing
There was a problem hiding this comment.
So we might want to do this instead:
if profile is null, assume "default"
if profile is "default", and config is null, render this config
if profile is anything else, skip it - assume the admin knows what they're doing
This sounds reasonable to me; the current implementation accounts for a variant with settings.aws.profile set to something non-default, while settings.aws.config is unset. But it wouldn't make much sense to specify a non-default profile without a corresponding config/credential file.
There was a problem hiding this comment.
I'm not seeing the kebab case use-fips-endpoint = true in the docs you linked. My read leads me to believe the setting is use_fips_endpoint regardless of profile
There was a problem hiding this comment.
I'm not seeing the kebab case
use-fips-endpoint = truein the docs you linked. My read leads me to believe the setting isuse_fips_endpointregardless of profile
Oh sorry, that was an error in my example; I'm used to kebab case for Bottlerocket. All I meant to call attention to was the use of [default] for default settings, and [profile x] for profile-specific settings, and not [profile default] for anything.
ginglis13
force-pushed
the
fips-aws-endpoints
branch
from
7ab6eb1 to
4e77410
Compare
|
^ force push includes
|
Write the contents of settings.aws.config to a file and point the AWS_CONFIG_FILE env variable at this file, if settings.aws.config is set. This ensures that configuration set by the user is applied to the AWS service interactions that pluto performs.
Add a new helper, aws-config, which will be used to create and set a node's settings.aws.config. The contents of the config will set use_fips_endpoint=<true|false> depending on if the variant is detected go be FIPS enabled. Signed-off-by: Gavin Inglis <[email protected]>
ginglis13
force-pushed
the
fips-aws-endpoints
branch
from
4e77410 to
708d599
Compare
|
^ latest force push serializes the two pluto unit tests such that |
| // the AWS config. | ||
| let aws_profile = match get_param(helper, 1)? { | ||
| Value::Null => "default", | ||
| Value::String(s) => s, |
There was a problem hiding this comment.
nit: you could return early here if you check s == "default"
Issue number:
Related: bottlerocket-os/bottlerocket#1667
Description of changes:
This PR ensures that Bottlerocket FIPS variants use FIPS AWS service endpoints by default. It adds a setting generator,
aws_config, which will be used to setsettings.aws.configby default if this setting is null. The value of the setting will be base64 encodedChanges are made to
plutoto ensure thatsettings.aws.configis used for its AWS service calls, given thatplutoruns before settings are applied. This is done by reading the inflight value ofsettings.aws.config, storing it to a file,/root/.aws/config.pluto, and temporarily settingAWS_CONFIG_FILEto this config file.Testing done:
I built FIPS
aws-k8s-1.29variants in us-west-2, us-gov-west-1, ca-central-1, and eu-west-1 for testing. These AMIs include changes forfips = truein variant definition.settings.aws.configrendered by schnauzer, if not set, to includeuse_fips_endpoint=<true|false>depending on if the AMI is FIPS enabled (aka, using theaws_confighelper added in this PR)Ran manual tests for the following scenarios and outcomes with a dev
aws-k8s-1.29FIPS variant:The reason for the slight difference in behavior for ca-central-1 and eu-west-2 is that ca-central-1 does have EC2 FIPS endpoints: https://aws.amazon.com/compliance/fips/#FIPS_Endpoints_by_Service
I added debug logging to
plutoto ensure thatuse_fipsfor the AWS SDK it uses returnstrueorfalsedepending on the/root/.aws/config.plutothat is used.Details for the happy path, with debug logging in
pluto, are below (I can include the details for all others in the above table, I just don't want to clutter this description):FIPS enabled AMI, FIPS enabled region - settings.aws.config is set with use_fips_endpoint=true, boots successfully and joins cluster ✅
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.