Skip to content
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

credential_process requires an available shell #2879

Open
3 tasks done
anuraaga opened this issue Dec 1, 2021 · 10 comments
Open
3 tasks done

credential_process requires an available shell #2879

anuraaga opened this issue Dec 1, 2021 · 10 comments
Assignees
Labels
Cross-SDK Requires cross-sdk coordination. Implementation under consideration feature-request A feature should be added or improved. p2 This is a standard priority issue

Comments

@anuraaga
Copy link

anuraaga commented Dec 1, 2021

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug
credential_process always executes commands in a shell

https://github.com/aws/aws-sdk-go/blob/main/aws/credentials/processcreds/provider.go#L301

This prevents it from being usable in Docker images that don't have a shell such as scratch or distroless. There doesn't seem to be any reason that it shouldn't be possible to invoke a binary directly through credential_process without a shell in such environments, for statically compiled Go binaries, those base images are fairly popular now.

Omitting remaining sections since issue is obvious from the code.

@anuraaga
Copy link
Author

anuraaga commented Dec 1, 2021

/cc @xibz

@KaibaLopez
Copy link
Contributor

Hi @anuraaga ,
Yeah, I see the use case.. though I might argue this is a feature request to support shell-less environments.
Can you also just let me know if you've tried any workarounds?

@anuraaga
Copy link
Author

@KaibaLopez I'm not aware of any workaround other than installing a shell into the docker image but many projects may not be able to do this, so we lose the ability to use credential process.

@nicolasff
Copy link

I'm not aware of any workaround other than installing a shell into the docker image

Hello! I work with @xibz (who opened the issue in aws-otel-collector) and one workaround we've found was to mount a copy of /bin/sh and its single dependency /lib/ld-musl-x86_64.so.1 from an Alpine image into the container. So yes, installing a shell into the docker image.

Something like:

mkdir -p ./alpine-shell/bin ./alpine-shell/lib
docker run --rm -ti alpine:3.15.0 base64 /bin/sh | base64 -D > ./alpine-shell/bin/sh
docker run --rm -ti alpine:3.15.0 base64 /lib/ld-musl-x86_64.so.1 | base64 -D > ./alpine-shell/lib/ld-musl-x86_64.so.1

and then run the container that uses the AWS SDK with -v ./alpine-shell/bin:/bin -v ./alpine-shell/lib:/lib

Needless to say we're unhappy with this (gross) workaround, but it does work for now as an intermediary between the Go program using the AWS SDK and our credential_process binary. Of course this isn't an option if the container needs any of the files it has under /bin or /lib.

Alternatively you could also write a statically linked program – in Go, C, or Rust maybe – that recognizes -c <command> and runs that command, or strips the initial -c and passes its parameters to a hardcoded credential provider.

Both approaches feel super hacky, more like "a workaround is possible if you really want it" rather than "here's a great way to resolve the issue". Just running the credentials provider directly seems like the better long-term fix.

@rittneje
Copy link

@KaibaLopez I would also point out even other SDKs disagree on this point.

Ruby and Python (and thus by extension the CLI) do not use the shell.
https://github.com/aws/aws-sdk-ruby/blob/a82c8981c95a8296ffb6269c3c06a4f551d87f7d/gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb#L36-L41
https://github.com/boto/botocore/blob/04d1fae43b657952e49b21d16daa86378ddb4253/botocore/credentials.py#L979-L984

However, C# does.
https://github.com/aws/aws-sdk-net/blob/475822dec5e87954b7a47ac65995714ae1f1b115/sdk/src/Core/Amazon.Runtime/Credentials/ProcessAWSCredentials.cs#L58-L78

One interesting implication is that Go will ultimately perform environment variable expansion, while other languages will not.

package main

import (
	"fmt"
	"os"

	"github.com/aws/aws-sdk-go/aws/credentials/processcreds"
)

func main() {
	creds := processcreds.NewCredentials(`echo '{"Version":1,"AccessKeyId":"'"${FOO}"'","SecretAccessKey":"'"${BAR}"'"}'`)

	os.Setenv("FOO", "foo")
	os.Setenv("BAR", "bar")

	got, err := creds.Get()
	if err != nil {
		panic(err)
	}

	// {AccessKeyID:foo SecretAccessKey:bar SessionToken: ProviderName:ProcessProvider}
	fmt.Printf("%+v\n", got)
}

The actual documentation for credential_process makes no clear mention of a shell one way or the other. https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-sourcing-external.html

I would say that:

  1. The documentation should be clarified that no implicit shell will be spawned, and thus no environment variables will be expanded by default.
  2. The Go and C# SDKs should be fixed to align with the other SDKs. (For backwards compatibility this may need to be opt-in.)
  3. Users who actually want a shell should explicitly invoke /bin/sh (or similar) as the credential process.

@skmcgrail
Copy link
Member

skmcgrail commented Jan 3, 2022

I don't believe this issue can be easily resolved, as arguably there is not limited to just Go and C#, Java, Javascript, PHP, Ruby, and C++ also rely on the shell for argument parsing and the respective environment. Any change of behavior would need to be done across all the SDKs to standardize on one set of behavior, and provide a mechanism which would designate or opt-in to the desired behavior, whatever that is determined to be. This issue is would likely be better driven by being sent to the aws-sdk tracking repository of issues as this is a cross-cutting behavior that we likely can't change in a vacuum.

  • Java SDK will invoke either /bin/sh or cmd.exe for the respective platform.
  • Javascript SDK uses the nodejs child_process module which will invoke the command in a shell by default /bin/sh or cmd.exe for the respective platform.
  • PHP - Uses shell_exec
  • Ruby does actually invoke the shell, but it is not immediately obvious. Per the Ruby documentation on the underlying Kernel#exec that the backticks utilize, a single argument call to exec is evaluated in the the environments standard shell. Per the Ruby documentation:

In the first form, the string is taken as a command line that is subject to shell expansion before being executed. The standard shell always means "/bin/sh" on Unix-like systems, same as ENV["RUBYSHELL"] (or ENV["COMSPEC"] on Windows NT series), and similar.

  • C++: Uses popen which is documented as passing the command to /bin/sh

Edit: Updated with more findings.

@anuraaga
Copy link
Author

anuraaga commented Jan 3, 2022

@skmcgrail Would you be able to transfer the issue to aws-sdk and help drive it there? One thing to keep in mind though is that it's far more common to not have a shell, e.g. in docker, for a go binary than any of those other languages thanks to static compilation so it's a higher priority for this language than the others possibly.

@skmcgrail skmcgrail transferred this issue from aws/aws-sdk-go Jan 3, 2022
@skmcgrail skmcgrail added the feature-request A feature should be added or improved. label Jan 4, 2022
@vudh1 vudh1 self-assigned this Jun 9, 2022
@vudh1 vudh1 assigned RanVaknin and unassigned vudh1 Sep 9, 2022
@hans-d
Copy link

hans-d commented Jan 23, 2023

see also: aws/aws-sdk-net#1845
In the case of a locked down windows environment (no cmd.exe allowed for normal users)

@jgrigg
Copy link

jgrigg commented Apr 12, 2024

Crazy hacky idea. Would publishing some public images where /bin/sh is actually not really a proper shell but a binary that runs the credential process (maybe written in go so we can target all platforms) and possibly takes some parameters. These would just be the base distroless images with this one extra file copied in and avoid an RCE attack.

Pretty gross but if it gets traction it might incentivise AWS to sort out the underlying issue.

Another option we're looking into is dropping a sidecar in our pods (we're in eks land) and asking for creds over http over the pod localhost. https://docs.aws.amazon.com/sdkref/latest/guide/feature-container-credentials.html

Would love to hear if anyone has had any luck with this. The pod identities docs don't mention this specifically but this guide mentions pod identities 🤔

I expect that pod identities being relatively new as opposed to irsa means their docs are in need of some tlc

@pschulten
Copy link

@jgrigg I had the exact same hack idea several years ago as well. It works flawlessly ever since.
The golang repo produces two binaries.

  1. A shell "wrapper"
func main() {
  fmt.Fprintf(os.Stderr, "DEBUG: sh wrapper args %s\n", os.Args)

  if os.Args[0] != "sh" || os.Args[1] != "-c" {
  	panic("unexpected invocation. Expected sh -c *program*")
  }

  // credential_process code calls "sh -c (custom command)", we are looking for os.Args[2]
  fmt.Fprintf(os.Stderr, "DEBUG: execing %s\n", os.Args[2])
  command := strings.Split(os.Args[2], " ")
  binary, lookErr := exec.LookPath(command[0])
  if lookErr != nil {
  	panic(lookErr)
  }

  env := os.Environ()

  execErr := syscall.Exec(binary, command, env)
  if execErr != nil {
  	panic(execErr)
  }
}
  1. the credential_process (In our case) using hashicorp vault call vark

it runs a sidecar in our clusters using this image:

FROM golang:1.22 as builder
ENV CGO_ENABLED=0
WORKDIR /workspace
COPY go.mod .
COPY go.sum .
RUN go mod download
COPY . .
RUN go build -ldflags "-s -w" ./cmd/sh-wrapper && \
  go build -ldflags "-s -w" ./cmd/vark

FROM gcr.io/distroless/static:nonroot
ENV HOME=/home/nonroot
ENV AWS_SDK_LOAD_CONFIG=1
ENV AWS_SHARED_CREDENTIALS_FILE=/home/nonroot/.aws/credentials
ENV PATH="/"

COPY credentials /home/nonroot/.aws/credentials
COPY --from=builder /workspace/vark /vark
COPY --from=builder /workspace/sh-wrapper /sh

ENTRYPOINT ["/vark"]

@ashishdhingra ashishdhingra added the p2 This is a standard priority issue label Aug 29, 2024
@tim-finnigan tim-finnigan transferred this issue from aws/aws-sdk Oct 30, 2024
@RanVaknin RanVaknin added the Cross-SDK Requires cross-sdk coordination. Implementation under consideration label Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cross-SDK Requires cross-sdk coordination. Implementation under consideration feature-request A feature should be added or improved. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests