From d971153c9f0dd05fa3475ec9a71d58bf4a59c581 Mon Sep 17 00:00:00 2001 From: Setu Shah Date: Tue, 20 Aug 2024 02:06:39 -0700 Subject: [PATCH 1/7] Support IMDSv2 credential fetch with AWS IAM provider --- minio/credentials/providers.py | 27 ++++++++++++++++++--------- tests/unit/credentials_test.py | 11 +++++++++-- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/minio/credentials/providers.py b/minio/credentials/providers.py index 51dca1f1..73366eaf 100644 --- a/minio/credentials/providers.py +++ b/minio/credentials/providers.py @@ -402,10 +402,9 @@ def __init__( self._full_uri = os.environ.get("AWS_CONTAINER_CREDENTIALS_FULL_URI") self._credentials: Credentials | None = None - def fetch(self, url: str) -> Credentials: - """Fetch credentials from EC2/ECS. """ - - res = _urlopen(self._http_client, "GET", url) + def fetch(self, url: str, headers: dict) -> Credentials: + """Fetch credentials from EC2/ECS.""" + res = _urlopen(self._http_client, "GET", url, headers=headers) data = json.loads(res.data) if data.get("Code", "Success") != "Success": raise ValueError( @@ -457,17 +456,27 @@ def retrieve(self) -> Credentials: "http://169.254.169.254" + "/latest/meta-data/iam/security-credentials/" ) - - res = _urlopen(self._http_client, "GET", url) + # Step 1 of the IMDSv2 protocol: get a token from the metadata + # service with a 6-hour TTL. + response = _urlopen( + self._http_client, "PUT", + "http://169.254.169.254/latest/api/token", + headers={"X-aws-ec2-metadata-token-ttl-seconds": "21600"} + ) + # Step 2: get the role name from the metadata service, with the + # token as a header. + token_header: dict = { + "X-aws-ec2-metadata-token": + response.data.decode("utf-8").strip() + } + res = _urlopen(self._http_client, "GET", url, headers=token_header) role_names = res.data.decode("utf-8").split("\n") if not role_names: raise ValueError(f"no IAM roles attached to EC2 service {url}") url += "/" + role_names[0].strip("\r") - if not url: raise ValueError("url is empty; this should not happen") - - self._credentials = self.fetch(url) + self._credentials = self.fetch(url, headers=token_header) return self._credentials diff --git a/tests/unit/credentials_test.py b/tests/unit/credentials_test.py index 4d3b1ee4..d20e9569 100644 --- a/tests/unit/credentials_test.py +++ b/tests/unit/credentials_test.py @@ -45,11 +45,16 @@ def test_credentials_get(self): self.assertEqual(creds.session_token, None) -class CredListResponse(object): +class TokenResponse(object): status = 200 data = b"test-s3-full-access-for-minio-ec2" +class CredListResponse(object): + status = 200 + data = b"test-s3-token-for-minio" + + class CredsResponse(object): status = 200 data = json.dumps({ @@ -66,7 +71,9 @@ class CredsResponse(object): class IamAwsProviderTest(TestCase): @mock.patch("urllib3.PoolManager.urlopen") def test_iam(self, mock_connection): - mock_connection.side_effect = [CredListResponse(), CredsResponse()] + mock_connection.side_effect = [ + TokenResponse(), CredListResponse(), CredsResponse() + ] provider = IamAwsProvider() creds = provider.retrieve() self.assertEqual(creds.access_key, "accessKey") From 089cf640eb82b69a75fa50d4403706cf1b0a5ee3 Mon Sep 17 00:00:00 2001 From: Setu Shah Date: Tue, 20 Aug 2024 22:31:03 -0700 Subject: [PATCH 2/7] feat: Update per @balamurugana's PR feedback --- minio/credentials/providers.py | 111 +++++++++++++++++++++++---------- 1 file changed, 78 insertions(+), 33 deletions(-) diff --git a/minio/credentials/providers.py b/minio/credentials/providers.py index 73366eaf..d33561be 100644 --- a/minio/credentials/providers.py +++ b/minio/credentials/providers.py @@ -29,7 +29,8 @@ from datetime import timedelta from pathlib import Path from typing import Callable, cast -from urllib.parse import urlencode, urlsplit +from urllib.parse import urlencode, urlsplit, urlunsplit + from xml.etree import ElementTree as ET import certifi @@ -42,7 +43,7 @@ from urllib3.util import Retry, parse_url -from minio.helpers import sha256_hash +from minio.helpers import sha256_hash, url_replace from minio.signer import sign_v4_sts from minio.time import from_iso8601utc, to_amz_date, utcnow from minio.xml import find, findtext @@ -381,6 +382,13 @@ def __init__( self, custom_endpoint: str | None = None, http_client: PoolManager | None = None, + auth_token: str | None = None, + relative_uri: str | None = None, + full_uri: str | None = None, + token_file: str | None = None, + role_arn: str | None = None, + role_session_name: str | None = None, + region: str | None = None, ): self._custom_endpoint = custom_endpoint self._http_client = http_client or PoolManager( @@ -390,19 +398,39 @@ def __init__( status_forcelist=[500, 502, 503, 504], ), ) - self._token_file = os.environ.get("AWS_WEB_IDENTITY_TOKEN_FILE") - self._aws_region = os.environ.get("AWS_REGION") - self._role_arn = os.environ.get("AWS_ROLE_ARN") - self._role_session_name = os.environ.get("AWS_ROLE_SESSION_NAME") - self._relative_uri = os.environ.get( - "AWS_CONTAINER_CREDENTIALS_RELATIVE_URI", + self._token = ( + os.environ.get("AWS_CONTAINER_AUTHORIZATION_TOKEN") or + auth_token + ) + self._token_file = ( + os.environ.get("AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE") or + auth_token + ) + self._identity_file = ( + os.environ.get("AWS_WEB_IDENTITY_TOKEN_FILE") or token_file + ) + self._aws_region = os.environ.get("AWS_REGION") or region + self._role_arn = os.environ.get("AWS_ROLE_ARN") or role_arn + self._role_session_name = ( + os.environ.get("AWS_ROLE_SESSION_NAME") or role_session_name + ) + self._relative_uri = ( + os.environ.get("AWS_CONTAINER_CREDENTIALS_RELATIVE_URI") or + relative_uri ) if self._relative_uri and not self._relative_uri.startswith("/"): - self._relative_uri = "/" + self._relative_uri - self._full_uri = os.environ.get("AWS_CONTAINER_CREDENTIALS_FULL_URI") + self._relative_uri = "/" + self._relative_uri + self._full_uri = ( + os.environ.get("AWS_CONTAINER_CREDENTIALS_FULL_URI") or + full_uri + ) self._credentials: Credentials | None = None - def fetch(self, url: str, headers: dict) -> Credentials: + def fetch( + self, + url: str, + headers: dict[str, str | list[str] | tuple[str]] | None = None, + ) -> Credentials: """Fetch credentials from EC2/ECS.""" res = _urlopen(self._http_client, "GET", url, headers=headers) data = json.loads(res.data) @@ -427,14 +455,16 @@ def retrieve(self) -> Credentials: return self._credentials url = self._custom_endpoint - if self._token_file: + if self._identity_file: if not url: url = "https://sts.amazonaws.com" if self._aws_region: url = f"https://sts.{self._aws_region}.amazonaws.com" + if self._aws_region.startswith("cn-"): + url += ".cn" provider = WebIdentityProvider( - lambda: _get_jwt_token(cast(str, self._token_file)), + lambda: _get_jwt_token(cast(str, self._identity_file)), url, role_arn=self._role_arn, role_session_name=self._role_session_name, @@ -443,40 +473,55 @@ def retrieve(self) -> Credentials: self._credentials = provider.retrieve() return cast(Credentials, self._credentials) + headers: dict[str, str | list[str] | tuple[str]] | None = None if self._relative_uri: if not url: url = "http://169.254.170.2" + self._relative_uri + headers = {"Authorization": self._token} if self._token else None elif self._full_uri: - if not url: + token = self._token + if self._token_file: url = self._full_uri - _check_loopback_host(url) + with open(self._token_file, encoding="utf-8") as file: + token = file.read() + else: + if not url: + url = self._full_uri + _check_loopback_host(url) + headers = {"Authorization": token} if token else None else: if not url: - url = ( - "http://169.254.169.254" + - "/latest/meta-data/iam/security-credentials/" - ) - # Step 1 of the IMDSv2 protocol: get a token from the metadata - # service with a 6-hour TTL. - response = _urlopen( - self._http_client, "PUT", - "http://169.254.169.254/latest/api/token", - headers={"X-aws-ec2-metadata-token-ttl-seconds": "21600"} + url = "http://169.254.169.254" + + # Get IMDS Token + res = _urlopen( + self._http_client, + "GET", + url+"/latest/api/token", + headers={"X-aws-ec2-metadata-token-ttl-seconds": "21600"}, + ) + token = res.data.decode("utf-8") + headers = {"X-aws-ec2-metadata-token": token} if token else None + + # Get role name + res = _urlopen( + self._http_client, + "GET", + urlunsplit( + url_replace( + urlsplit(url), + path="/latest/meta-data/iam/security-credentials/", + ), + ), + headers=headers, ) - # Step 2: get the role name from the metadata service, with the - # token as a header. - token_header: dict = { - "X-aws-ec2-metadata-token": - response.data.decode("utf-8").strip() - } - res = _urlopen(self._http_client, "GET", url, headers=token_header) role_names = res.data.decode("utf-8").split("\n") if not role_names: raise ValueError(f"no IAM roles attached to EC2 service {url}") url += "/" + role_names[0].strip("\r") if not url: raise ValueError("url is empty; this should not happen") - self._credentials = self.fetch(url, headers=token_header) + self._credentials = self.fetch(url, headers=headers) return self._credentials From 200d564162d38cde04a53b60e6a4c50cdcf199e7 Mon Sep 17 00:00:00 2001 From: Setu Shah Date: Tue, 20 Aug 2024 22:34:46 -0700 Subject: [PATCH 3/7] fix: Switch `GET` => `PUT` --- minio/credentials/providers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/minio/credentials/providers.py b/minio/credentials/providers.py index d33561be..7d547111 100644 --- a/minio/credentials/providers.py +++ b/minio/credentials/providers.py @@ -419,7 +419,7 @@ def __init__( relative_uri ) if self._relative_uri and not self._relative_uri.startswith("/"): - self._relative_uri = "/" + self._relative_uri + self._relative_uri = "/" + self._relative_uri self._full_uri = ( os.environ.get("AWS_CONTAINER_CREDENTIALS_FULL_URI") or full_uri @@ -496,7 +496,7 @@ def retrieve(self) -> Credentials: # Get IMDS Token res = _urlopen( self._http_client, - "GET", + "PUT", url+"/latest/api/token", headers={"X-aws-ec2-metadata-token-ttl-seconds": "21600"}, ) From 62ba3f4d8bf145d4708cbeb392e0c06cae8c0554 Mon Sep 17 00:00:00 2001 From: Setu Shah Date: Tue, 20 Aug 2024 22:35:25 -0700 Subject: [PATCH 4/7] style: Ignore `pylint` for too many branches warning --- minio/credentials/providers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/minio/credentials/providers.py b/minio/credentials/providers.py index 7d547111..44fc71bb 100644 --- a/minio/credentials/providers.py +++ b/minio/credentials/providers.py @@ -14,6 +14,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +# pylint: disable=too-many-branches + """Credential providers.""" from __future__ import annotations From 208f82859cbcb9755577bc58e80c068b3df94e18 Mon Sep 17 00:00:00 2001 From: Setu Shah Date: Tue, 20 Aug 2024 22:36:38 -0700 Subject: [PATCH 5/7] style: Remove extra whitespace --- minio/credentials/providers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/minio/credentials/providers.py b/minio/credentials/providers.py index 44fc71bb..197d3298 100644 --- a/minio/credentials/providers.py +++ b/minio/credentials/providers.py @@ -32,7 +32,6 @@ from pathlib import Path from typing import Callable, cast from urllib.parse import urlencode, urlsplit, urlunsplit - from xml.etree import ElementTree as ET import certifi From dd05548e1f4284c1148d0a9409ec340b19665153 Mon Sep 17 00:00:00 2001 From: Setu Shah Date: Wed, 21 Aug 2024 20:31:19 -0700 Subject: [PATCH 6/7] test: Fix incorrect data --- tests/unit/credentials_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/credentials_test.py b/tests/unit/credentials_test.py index d20e9569..baaafbe8 100644 --- a/tests/unit/credentials_test.py +++ b/tests/unit/credentials_test.py @@ -47,12 +47,12 @@ def test_credentials_get(self): class TokenResponse(object): status = 200 - data = b"test-s3-full-access-for-minio-ec2" + data = b"test-s3-token-for-minio" class CredListResponse(object): status = 200 - data = b"test-s3-token-for-minio" + data = b"test-s3-full-access-for-minio-ec2" class CredsResponse(object): From 3a5cd228e85c9c44b9313ef72ef2bc865687aff9 Mon Sep 17 00:00:00 2001 From: Setu Shah Date: Thu, 22 Aug 2024 00:06:32 -0700 Subject: [PATCH 7/7] test: Remove incomplete unit test --- tests/unit/credentials_test.py | 42 ---------------------------------- 1 file changed, 42 deletions(-) diff --git a/tests/unit/credentials_test.py b/tests/unit/credentials_test.py index baaafbe8..87d72d06 100644 --- a/tests/unit/credentials_test.py +++ b/tests/unit/credentials_test.py @@ -14,16 +14,11 @@ # See the License for the specific language governing permissions and # limitations under the License. -import json import os -import unittest.mock as mock -from datetime import datetime, timedelta from unittest import TestCase -from minio.credentials.credentials import Credentials from minio.credentials.providers import (AWSConfigProvider, ChainedProvider, EnvAWSProvider, EnvMinioProvider, - IamAwsProvider, MinioClientConfigProvider, StaticProvider) @@ -45,43 +40,6 @@ def test_credentials_get(self): self.assertEqual(creds.session_token, None) -class TokenResponse(object): - status = 200 - data = b"test-s3-token-for-minio" - - -class CredListResponse(object): - status = 200 - data = b"test-s3-full-access-for-minio-ec2" - - -class CredsResponse(object): - status = 200 - data = json.dumps({ - "Code": "Success", - "Type": "AWS-HMAC", - "AccessKeyId": "accessKey", - "SecretAccessKey": "secret", - "Token": "token", - "Expiration": "2014-12-16T01:51:37Z", - "LastUpdated": "2009-11-23T0:00:00Z" - }) - - -class IamAwsProviderTest(TestCase): - @mock.patch("urllib3.PoolManager.urlopen") - def test_iam(self, mock_connection): - mock_connection.side_effect = [ - TokenResponse(), CredListResponse(), CredsResponse() - ] - provider = IamAwsProvider() - creds = provider.retrieve() - self.assertEqual(creds.access_key, "accessKey") - self.assertEqual(creds.secret_key, "secret") - self.assertEqual(creds.session_token, "token") - self.assertEqual(creds._expiration, datetime(2014, 12, 16, 1, 51, 37)) - - class ChainedProviderTest(TestCase): def test_chain_retrieve(self): # clear environment