Skip to content

Commit

Permalink
Minimum of expiration time and expires_in for presigned urls (#2872)
Browse files Browse the repository at this point in the history
  • Loading branch information
mullermp committed Jun 27, 2023
1 parent 08f1e3d commit 85fe907
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 29 deletions.
2 changes: 1 addition & 1 deletion build_tools/services.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class ServiceEnumerator
MANIFEST_PATH = File.expand_path('../../services.json', __FILE__)

# Minimum `aws-sdk-core` version for new gem builds
MINIMUM_CORE_VERSION = "3.174.0"
MINIMUM_CORE_VERSION = "3.176.0"

EVENTSTREAM_PLUGIN = "Aws::Plugins::EventStreamConfiguration"

Expand Down
2 changes: 2 additions & 0 deletions gems/aws-sdk-core/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Unreleased Changes
------------------

* Feature - Add :expiration accessor to `CredentialProvider` and do not refresh credentials when checking expiration.

3.175.0 (2023-06-15)
------------------

Expand Down
3 changes: 3 additions & 0 deletions gems/aws-sdk-core/lib/aws-sdk-core/credential_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ module CredentialProvider
# @return [Credentials]
attr_reader :credentials

# @param [Time]
attr_reader :expiration

# @return [Boolean]
def set?
!!credentials && credentials.set?
Expand Down
6 changes: 0 additions & 6 deletions gems/aws-sdk-core/lib/aws-sdk-core/refreshing_credentials.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,6 @@ def credentials
@credentials
end

# @return [Time,nil]
def expiration
refresh_if_near_expiration!
@expiration
end

# Refresh credentials.
# @return [void]
def refresh!
Expand Down
2 changes: 2 additions & 0 deletions gems/aws-sdk-s3/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Unreleased Changes
------------------

* Feature - Select minimum expiration time for presigned urls between the expiration time option and the credential expiration time.

1.126.0 (2023-06-16)
------------------

Expand Down
6 changes: 4 additions & 2 deletions gems/aws-sdk-s3/lib/aws-sdk-s3/presigner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ def initialize(options = {})
# before the presigned URL expires. Defaults to 15 minutes. As signature
# version 4 has a maximum expiry time of one week for presigned URLs,
# attempts to set this value to greater than one week (604800) will
# raise an exception.
# raise an exception. The min value of this option and the credentials
# expiration time is used in the presigned URL.
#
# @option params [Time] :time (Time.now) The starting time for when the
# presigned url becomes active.
Expand Down Expand Up @@ -96,7 +97,8 @@ def presigned_url(method, params = {})
# before the presigned URL expires. Defaults to 15 minutes. As signature
# version 4 has a maximum expiry time of one week for presigned URLs,
# attempts to set this value to greater than one week (604800) will
# raise an exception.
# raise an exception. The min value of this option and the credentials
# expiration time is used in the presigned URL.
#
# @option params [Time] :time (Time.now) The starting time for when the
# presigned url becomes active.
Expand Down
62 changes: 57 additions & 5 deletions gems/aws-sdk-s3/spec/presigner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,40 @@ module S3
describe Presigner do
let(:client) { Aws::S3::Client.new(**client_opts) }

let(:credentials) do
Credentials.new(
'ACCESS_KEY_ID',
'SECRET_ACCESS_KEY'
)
end
let(:client_opts) do
{
region: 'us-east-1',
credentials: Credentials.new(
'ACCESS_KEY_ID',
'SECRET_ACCESS_KEY'
),
credentials: credentials,
stub_responses: true
}
end

subject { Presigner.new(client: client) }

before { allow(Time).to receive(:now).and_return(Time.utc(2021, 8, 27)) }
let(:time) { Time.utc(2021, 8, 27) }
before { allow(Time).to receive(:now).and_return(time) }

let(:expiration_time) { time + 180 }
let(:credentials_provider_class) do
Class.new do
include CredentialProvider

def initialize(expiration_time)
@credentials = Credentials.new(
'akid',
'secret',
'session'
)
@expiration = expiration_time
end
end
end

describe '#initialize' do
it 'accepts an injected S3 client' do
Expand Down Expand Up @@ -170,6 +190,22 @@ module S3
)
expect(url).to match(/x-amz-acl=public-read/)
end

context 'credential expiration' do
let(:credentials) do
credentials_provider_class.new(expiration_time)
end

it 'picks the minimum time between expires_in and credential expiration' do
url = subject.presigned_url(
:get_object,
bucket: 'aws-sdk',
key: 'foo',
expires_in: 3600
)
expect(url).to match(/X-Amz-Expires=180/)
end
end
end

describe '#presigned_request' do
Expand Down Expand Up @@ -306,6 +342,22 @@ module S3
expect(url).to match(/X-Amz-SignedHeaders=host%3Bx-amz-acl/)
expect(headers).to eq('x-amz-acl' => 'public-read')
end

context 'credential expiration' do
let(:credentials) do
credentials_provider_class.new(expiration_time)
end

it 'picks the minimum time between expires_in and credential expiration' do
url, = subject.presigned_request(
:get_object,
bucket: 'aws-sdk',
key: 'foo',
expires_in: 3600
)
expect(url).to match(/X-Amz-Expires=180/)
end
end
end

context 'outpost access point ARNs' do
Expand Down
2 changes: 2 additions & 0 deletions gems/aws-sigv4/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Unreleased Changes
------------------

* Feature - Select the minimum expiration time for presigned urls between the expiration time option and the credential expiration time.

1.5.2 (2022-09-30)
------------------

Expand Down
39 changes: 25 additions & 14 deletions gems/aws-sigv4/lib/aws-sigv4/signer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ def sign_request(request)

return crt_sign_request(request) if Signer.use_crt?

creds = fetch_credentials
creds, _ = fetch_credentials

http_method = extract_http_method(request)
url = extract_url(request)
Expand Down Expand Up @@ -314,7 +314,7 @@ def sign_request(request)
# hex-encoded string using #unpack
def sign_event(prior_signature, payload, encoder)
# Note: CRT does not currently provide event stream signing, so we always use the ruby implementation.
creds = fetch_credentials
creds, _ = fetch_credentials
time = Time.now
headers = {}

Expand Down Expand Up @@ -403,7 +403,7 @@ def presign_url(options)

return crt_presign_url(options) if Signer.use_crt?

creds = fetch_credentials
creds, expiration = fetch_credentials

http_method = extract_http_method(options)
url = extract_url(options)
Expand All @@ -423,7 +423,7 @@ def presign_url(options)
params['X-Amz-Algorithm'] = 'AWS4-HMAC-SHA256'
params['X-Amz-Credential'] = credential(creds, date)
params['X-Amz-Date'] = datetime
params['X-Amz-Expires'] = extract_expires_in(options)
params['X-Amz-Expires'] = presigned_url_expiration(options, expiration).to_s
params['X-Amz-Security-Token'] = creds.session_token if creds.session_token
params['X-Amz-SignedHeaders'] = signed_headers(headers)

Expand Down Expand Up @@ -526,7 +526,6 @@ def event_signature(secret_access_key, date, string_to_sign)
hmac(k_credentials, string_to_sign)
end


def path(url)
path = url.path
path = '/' if path == ''
Expand Down Expand Up @@ -682,8 +681,8 @@ def downcase_headers(headers)

def extract_expires_in(options)
case options[:expires_in]
when nil then 900.to_s
when Integer then options[:expires_in].to_s
when nil then 900
when Integer then options[:expires_in]
else
msg = "expected :expires_in to be a number of seconds"
raise ArgumentError, msg
Expand All @@ -698,11 +697,14 @@ def uri_escape_path(string)
self.class.uri_escape_path(string)
end


def fetch_credentials
credentials = @credentials_provider.credentials
if credentials_set?(credentials)
credentials
expiration = nil
if @credentials_provider.respond_to?(:expiration)
expiration = @credentials_provider.expiration
end
[credentials, expiration]
else
raise Errors::MissingCredentialsError,
'unable to sign request without credentials set'
Expand All @@ -720,21 +722,30 @@ def credentials_set?(credentials)
!credentials.secret_access_key.empty?
end

def presigned_url_expiration(options, expiration)
expires_in = extract_expires_in(options)
return expires_in unless expiration

expiration_seconds = (expiration - Time.now).to_i
[expires_in, expiration_seconds].min
end

### CRT Code

# the credentials used by CRT must be a
# CRT StaticCredentialsProvider object
def crt_fetch_credentials
creds = fetch_credentials
Aws::Crt::Auth::StaticCredentialsProvider.new(
creds, expiration = fetch_credentials
crt_creds = Aws::Crt::Auth::StaticCredentialsProvider.new(
creds.access_key_id,
creds.secret_access_key,
creds.session_token
)
[crt_creds, expiration]
end

def crt_sign_request(request)
creds = crt_fetch_credentials
creds, _ = crt_fetch_credentials
http_method = extract_http_method(request)
url = extract_url(request)
headers = downcase_headers(request[:headers])
Expand Down Expand Up @@ -793,7 +804,7 @@ def crt_sign_request(request)
end

def crt_presign_url(options)
creds = crt_fetch_credentials
creds, expiration = crt_fetch_credentials

http_method = extract_http_method(options)
url = extract_url(options)
Expand Down Expand Up @@ -821,7 +832,7 @@ def crt_presign_url(options)
use_double_uri_encode: @uri_escape_path,
should_normalize_uri_path: @normalize_path,
omit_session_token: @omit_session_token,
expiration_in_seconds: options.fetch(:expires_in, 900)
expiration_in_seconds: presigned_url_expiration(options, expiration)
)
http_request = Aws::Crt::Http::Message.new(
http_method, url.to_s, headers
Expand Down
2 changes: 1 addition & 1 deletion services.json
Original file line number Diff line number Diff line change
Expand Up @@ -921,7 +921,7 @@
"models": "s3/2006-03-01",
"dependencies": {
"aws-sdk-kms": "~> 1",
"aws-sigv4": "~> 1.4"
"aws-sigv4": "~> 1.6"
},
"addPlugins": [
"Aws::S3::Plugins::Accelerate",
Expand Down

0 comments on commit 85fe907

Please sign in to comment.