source: add ecr-credential-helper-shim#836
source: add ecr-credential-helper-shim#836sky1122 wants to merge 1 commit intobottlerocket-os:developfrom
Conversation
62fb013 to
a15c282
Compare
Introduce ecr-credential-helper-shim, a Rust binary that downgrades GODEBUG from fips140=only to fips140=on before exec'ing the Go-based ecr-credential-helper, since the Go ECR credential helper does not support fips140=only mode. Signed-off-by: Jingwei Wang <jweiw@amazon.com>
a15c282 to
3c37ffd
Compare
| let status = Command::new("/usr/libexec/docker-credential-ecr-login") | ||
| .args(env::args().skip(1)) | ||
| .env("GODEBUG", &godebug) | ||
| .status(); |
There was a problem hiding this comment.
Rather than executing the call of the helper in a separate process (fork), you should exec the call instead:
https://www.man7.org/linux/man-pages/man3/exec.3.html
So that you don't have to capture the status, and you don't have to worry about handling STDOUT/STDERR in case they require special handling by whatever is calling your shim.
| // Bottlerocket does not have a $HOME set by default and notation expect to find | ||
| // credentials from the ecr-credential-helper here. | ||
| os.Setenv("HOME", "/root") | ||
| os.Setenv("GODEBUG", "fips140=on") |
There was a problem hiding this comment.
Will notation break if fips140=only? Or did you do this to guarantee that the notation process that calls your shim also enforces fips140=on? If the only thing that breaks is the ECR helper, this shouldn't be needed.
| @@ -0,0 +1,15 @@ | |||
| [package] | |||
There was a problem hiding this comment.
Can you clarify why a new package rather than using the OS package?
|
Thanks for the review!!! That answer a lot of my questions and pros and cons. |
Issue number:
Closes #
Description of changes:
This is part of the changes to enable
notationinfips140=onlymode.Introduce ecr-credential-helper-shim, a Rust binary that downgrades GODEBUG from fips140=only to fips140=on before exec'ing the Go-based ecr-credential-helper, since the Go ECR credential helper does not support fips140=only mode.
Testing done:
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.