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

Extend AwsService:getAuthorizationHeader() / AwsV4Signature:getAuthorizationHeader() #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions src/lua/api-gateway/aws/AwsService.lua
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,12 @@ function _M:getHttpClient()
end

function _M:getAWSHost()
return self.aws_service .. "." .. self.aws_region .. ".amazonaws.com"
-- FIXME: the endpoint cannot always be determined mechanically; http://docs.aws.amazon.com/general/latest/gr/rande.html
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe an S3Service.lua implementation ( similar to LambdaService.lua ) may "override" in lua-style this method:

S3Service.lua:

local AwsService = require "api-gateway.aws.AwsService"
local cjson = require "cjson"
local error = error

local _M = AwsService:new({ ___super = true })
local super = {
    instance = _M,
    constructor = _M.constructor
}

function _M.new(self, o)
    ngx.log(ngx.DEBUG, "S3Service() o=", tostring(o))
    local o = o or {}
    o.aws_service = "s3"
    -- aws_service_name is used in the X-Amz-Target Header: i.e Kinesis_20131202.ListStreams
    o.aws_service_name = "S3"

    super.constructor(_M, o)

    setmetatable(o, self)
    self.__index = self
    return o
end

function _M:getAWSHost()
    return return self.aws_service .. "-" .. self.aws_region  .. ".amazonaws.com"
end

Note _M:getAWSHost() method. WDYT ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed a quick access to S3 service with authentication at that moment and I didn't know what these are supposed to be like, so I decided not to create a "subclass" for it. Overriding the function perfectly makes sense to me. That said, I'd bet the part of the code which determines the correct endpoint for a given service would be easier to maintain if it was centralized in one place.

if self.aws_service == "s3" then
return self.aws_service .. "-" .. self.aws_region .. ".amazonaws.com"
else
return self.aws_service .. "." .. self.aws_region .. ".amazonaws.com"
end
end

function _M:getCredentials()
Expand All @@ -201,16 +206,18 @@ function _M:getCredentials()
return return_obj
end

function _M:getAuthorizationHeader(http_method, path, uri_args, body)
function _M:getAuthorizationHeader(http_method, path, uri_args, body, host_override)
local credentials = self:getCredentials()
credentials.aws_endpoint = self:getAWSHost()
credentials.aws_region = self.aws_region
credentials.aws_service = self.aws_service
local awsAuth = AWSV4S:new(credentials)
local authorization = awsAuth:getAuthorizationHeader(http_method,
local authorization, payload_hash = awsAuth:getAuthorizationHeader(http_method,
path, -- "/"
uri_args, -- ngx.req.get_uri_args()
body)
return authorization, awsAuth, credentials.token
body,
host_override)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this host_override come from self:getAWSHost() ?

Copy link
Author

@moriyoshi moriyoshi Jun 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In S3, the endpoint URL can take two different forms, one is like https://BUCKETNAME.s3-us-west-1.amazonaws.com/ and the other is like https://s3-us-west-1.amazonaws.com/BUCKETNAME. The host_override argument was intended to handle those. In addition to it, I wanted to use getAuthorizationHeader from outside AwsService where the endpoint URL was specified directly in the settings instead of the bucket name. But this should be S3-specific, and doesn't always need to be taken care of.

return authorization, awsAuth, credentials.token, payload_hash
end

---
Expand Down Expand Up @@ -278,7 +285,7 @@ function _M:performAction(actionName, arguments, path, http_method, useSSL, time
end


local authorization, awsAuth, authToken = self:getAuthorizationHeader(request_method, request_path, uri_args, request_body)
local authorization, awsAuth, authToken, payloadHash = self:getAuthorizationHeader(request_method, request_path, uri_args, request_body)

local t = self.aws_service_name .. "." .. actionName
local request_headers = {
Expand All @@ -287,7 +294,8 @@ function _M:performAction(actionName, arguments, path, http_method, useSSL, time
["Accept"] = "application/json",
["Content-Type"] = content_type,
["X-Amz-Target"] = t,
["x-amz-security-token"] = authToken
["x-amz-security-token"] = authToken,
["x-amz-content-sha256"] = payloadHash
}
if ( extra_headers ~= nil ) then
for headerName, headerValue in pairs(extra_headers) do
Expand Down Expand Up @@ -329,4 +337,4 @@ function _M:performAction(actionName, arguments, path, http_method, useSSL, time
return ok, code, headers, status, body
end

return _M
return _M
42 changes: 27 additions & 15 deletions src/lua/api-gateway/aws/AwsV4Signature.lua
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ function HmacAuthV4Handler:new(o)
setmetatable(o, self)
self.__index = self
if ( o ~= nil) then
self.aws_endpoint = o.aws_endpoint
self.aws_service = o.aws_service
self.aws_region = o.aws_region
self.aws_secret_key = o.aws_secret_key
self.aws_access_key = o.aws_access_key
self.token = o.token
end
-- set amazon formatted dates
local utc = ngx.utctime()
Expand Down Expand Up @@ -52,7 +54,8 @@ local function get_hashed_canonical_request(method, uri, querystring, headers, r
-- add canonicalHeaders
local canonicalHeaders = ""
local signedHeaders = ""
for h_n,h_v in pairs(headers) do
for _, p in ipairs(headers) do
local h_n, h_v = p[1], p[2]
-- todo: trim and lowercase
canonicalHeaders = canonicalHeaders .. h_n .. ":" .. h_v .. "\n"
signedHeaders = signedHeaders .. h_n .. ";"
Expand All @@ -63,13 +66,14 @@ local function get_hashed_canonical_request(method, uri, querystring, headers, r
hash = hash .. canonicalHeaders .. "\n"
.. signedHeaders .. "\n"

hash = hash .. _hash(requestPayload or "")
requestPayloadHash = _hash(requestPayload or "")
hash = hash .. requestPayloadHash

ngx.log(ngx.DEBUG, "Canonical String to Sign is:\n" .. hash)

local final_hash = _hash(hash)
ngx.log(ngx.DEBUG, "Canonical String HASHED is:\n" .. final_hash .. "\n")
return final_hash
return final_hash, signedHeaders, requestPayloadHash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for returning the signedHeaders as well as requestPayloadHash.

end

local function get_string_to_sign(algorithm, request_date, credential_scope, hashed_canonical_request)
Expand Down Expand Up @@ -141,36 +145,44 @@ function HmacAuthV4Handler:formatQueryString(uri_args)
return uri
end

function HmacAuthV4Handler:getSignature(http_method, request_uri, uri_arg_table, request_payload )
function HmacAuthV4Handler:getSignature(http_method, request_uri, uri_arg_table, request_payload, host_override)
local uri_args = self:formatQueryString(uri_arg_table)
local utc = ngx.utctime()
local date1 = self.aws_date_short
local date2 = self.aws_date
local host = self.aws_endpoint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does aws_endpoint differ from host ? For S3 use-case does it include the full path ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explained here.

if host_override ~= nil then
host = host_override
end

local headers = {}
headers.host = self.aws_service .. "." .. self.aws_region .. ".amazonaws.com"
headers["x-amz-date"] = date2
table.insert(headers, {"host", host})
table.insert(headers, {"x-amz-date", date2})
if self.token ~= nil then
table.insert(headers, {"x-amz-security-token", self.token})
end

-- ensure parameters in query string are in order
local hashed_canonical_request, signed_headers, request_payload_hash = get_hashed_canonical_request(
http_method, request_uri,
uri_args,
headers, request_payload)
local sign = _sign( get_derived_signing_key( self.aws_secret_key,
date1,
self.aws_region,
self.aws_service),
get_string_to_sign("AWS4-HMAC-SHA256",
date2,
date1 .. "/" .. self.aws_region .. "/" .. self.aws_service .. "/aws4_request",
get_hashed_canonical_request(
http_method, request_uri,
uri_args,
headers, request_payload) ) )
return sign
hashed_canonical_request) )
return sign, signed_headers, request_payload_hash
end

function HmacAuthV4Handler:getAuthorizationHeader(http_method, request_uri, uri_arg_table, request_payload )
local auth_signature = self:getSignature(http_method, request_uri, uri_arg_table, request_payload)
function HmacAuthV4Handler:getAuthorizationHeader(http_method, request_uri, uri_arg_table, request_payload, host_override)
local auth_signature, signed_headers, request_payload_hash = self:getSignature(http_method, request_uri, uri_arg_table, request_payload, host_override)
local authHeader = "AWS4-HMAC-SHA256 Credential=" .. self.aws_access_key.."/" .. self.aws_date_short .. "/" .. self.aws_region
.."/" .. self.aws_service.."/aws4_request,SignedHeaders=host;x-amz-date,Signature="..auth_signature
return authHeader
.."/" .. self.aws_service.."/aws4_request,SignedHeaders="..signed_headers..",Signature="..auth_signature
return authHeader, request_payload_hash
end

return HmacAuthV4Handler
Expand Down