Skip to content

Minio Client AWS Auth only supports Access Keys; Add Options to Credential Chain #32271

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

Closed
ghost opened this issue Oct 15, 2024 · 5 comments
Closed
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Milestone

Comments

@ghost
Copy link

ghost commented Oct 15, 2024

Feature Description

Objective

We propose the following features added to the Gitea release cycle (release/v1.22):

  • Merge the credential chain reference for the Minio client credentials from the main branch into the current release cycle
  • Stretch goal: Add the Minio.Credential.IAM_AWS function getEKSPodIdentityCredentials to this credential chain at the end, this way native Kubernetes authentication is handled.

Details

We have deployed Gitea using the Helm Chart on our AWS EKS cluster. We’ve been able to stand it up with supporting infrastructure and figure out configurations, but we’ve discovered that the implementation of the Minio client under the Gitea release branch is only looking at AWS Access Keys (long term credentials) for authentication with AWS. The recommended best practice from AWS prefers relying on temporary credentials instead of long-term credentials, such as access keys where possible.

We hope to see implementation of short-lived tokens to authenticate the Minio client with AWS APIs, as this would provide better security and simplified implementation.

Expanding on the credential chain, this currently does not import other supported features from the minio-go SDK. Notably, as we are on AWS EKS, the ideal solution we were hoping to implement is using IRSA roles which would allow direct usage of the token file generated by the Service Account.

References

Screenshots

No response

@ghost ghost added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Oct 15, 2024
@yp05327

This comment was marked as off-topic.

@techknowlogick
Copy link
Member

Thank you for your issue. We don't backport enhancements/features such as this, but we hope to release 1.23 this quarter, which would allow you to use this in a non-nightly version.

@ghost
Copy link
Author

ghost commented Oct 18, 2024

Hello, thank you for the quick response!

We have given the nightly-rootless image a try, and this helped us figure out some follow up issues as it wasn’t working right away, so we have one working solution via workaround but also another fix recommended to allow flexibility with AWS Auth using Minio for storage.


The first discovery is that the credential process workaround was not working immediately, but found it was due to this line the Minio SDK that is imported - https://github.com/minio/minio-go/blob/master/pkg/credentials/file_aws_credentials.go#L113

Since it’s checking for two arguments, we just added a dummy string afterwards and it worked perfectly to authenticate without the use of Access Keys. Working example:

credential_process = /data/gitea/git/gen-token dummy-text

Ideally it should be checking with < not <=, but that isn’t due to the implementation by Gitea and was easily worked around for us. So, this will work when it makes it into the 1.23 release for short term credentials, but this is ultimately only a workaround for native authentication, leading into the next section ->


Secondly, while reviewing the code again I realized I have misinterpreted what this line actually does, I now see this is a reference to the entire file from the Minio SDK, not just that one “EKSPodIdentity” function I mentioned originally.

However, this does not work out of the box when trying this on the nightly image (explicitly not passing the shared credential environment variable and access keys.) We discovered this is due to the EC2 Metadata endpoint being hard coded at this line which is passed to the IAM package here. Once the code proceeds to this function, at L150 of this switch case since the endpoint is provided it does not attempt to utilize the actual STS endpoint resulting in an error, and ultimately a "permission denied."

To summarize the issue here, it is trying to receive credentials using the normal EC2 Metadata endpoint with the identity file rather than the STS endpoint.

Proposed Solution

To work around the hard coded EC2 Metadata endpoint, we have the following idea:

  1. Remove the credentials.DefaultIAMRoleEndpoint value here, as this is what determines usage of EC2 Metadata endpoint only - https://github.com/go-gitea/gitea/blob/main/modules/storage/minio.go#L100
  2. Add a new config value for the storage block, which is accessed here - https://github.com/go-gitea/gitea/blob/main/modules/storage/minio.go#L187 (which calls the Minio SDK here https://github.com/minio/minio-go/blob/master/pkg/credentials/iam_aws.go)
    1. Possibly with the other storage configs here, it may make sense to call it something like MINIO_IAM_ENDPOINT to keep with consistency - https://docs.gitea.com/administration/config-cheat-sheet#storage-storage
  3. This allows the EC2 endpoint to be passed for tests and still pass, but allows it to be empty by default for the SDK to decide for itself what endpoint it should use - https://github.com/go-gitea/gitea/pull/31051/files#diff-97ef7d5c55f1086fb6eb6bff28747793dd6dabfceadcda573db8c5ba0a0aa8e5R120

We’ll try and work on a PR that accomplishes our proposal, but wanted to get this update out first.

@techknowlogick
Copy link
Member

Thanks for that detailed followup @vkice-at-ocp :)
I've re-opened this ticket to keep track of it.

lunny pushed a commit that referenced this issue Nov 22, 2024
…32581)

Targeting issue #32271

This modification allows native Kubernetes + AWS (EKS) authentication
with the Minio client, to Amazon S3 using the IRSA role assigned to a
Service account by replacing the hard coded reference to the
`DefaultIAMRoleEndpoint` with an optional configurable endpoint.

Internally, Minio's `credentials.IAM` provider implements a discovery
flow for IAM Endpoints if it is not set.

For backwards compatibility: 
- We have added a configuration mechanism for an `IamEndpoint` to retain
the unit test safety in `minio_test.go`.
- We believe existing clients will continue to function the same without
needing to provide a new config property since the internals of Minio
client also often resolve to the `http://169.254.169.254` default
endpoint that was being hard coded before

To test, we were able to build a docker image from source and, observe
it choosing the expected IAM endpoint, and see files uploaded via the
client.
@ghost
Copy link
Author

ghost commented Nov 22, 2024

Our PR was merged in to resolve our issue, closing!

@ghost ghost closed this as completed Nov 22, 2024
@lunny lunny added this to the 1.23.0 milestone Nov 22, 2024
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Feb 21, 2025
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

3 participants