allow services to pass their Request ID
refs CNVS-27581 this ensures we have one correlation ID for a request coming through the ecosystem rather than a seperate ID for each request at each service. Each request id must be signed to make sure others can't just submit whatever they want. TEST PLAN: 1) make an API request and provide the header 'X-Request-Context-Id' 2) make sure to include the X-Request-Context-Signature header, which should have the value of the sha512 signature with the shared secret for the services ecosystem 2) the logs should show your provided value as the context Id for that canvas request Change-Id: I610fbe8c4df355d43c05360670f80971d1459644 Reviewed-on: https://gerrit.instructure.com/73166 Tested-by: Jenkins Reviewed-by: Simon Williams <simon@instructure.com> QA-Review: Jeremy Putnam <jeremyp@instructure.com> Product-Review: Ethan Vizitei <evizitei@instructure.com>
This commit is contained in:
parent
3710d3b802
commit
3df3a34b56
|
@ -24,7 +24,7 @@ class RequestContextGenerator
|
|||
end
|
||||
|
||||
def call(env)
|
||||
request_id = SecureRandom.uuid
|
||||
request_id = generate_request_id(env)
|
||||
|
||||
# rack.session.options (where the session_id is saved by our session
|
||||
# store) isn't availalbe at this point in the middleware stack. It is
|
||||
|
@ -101,4 +101,18 @@ class RequestContextGenerator
|
|||
self.add_meta_header("e", page_view.asset_user_access_id)
|
||||
self.add_meta_header("f", page_view.created_at.try(:utc).try(:iso8601, 2))
|
||||
end
|
||||
|
||||
private
|
||||
def generate_request_id(env)
|
||||
if env['HTTP_X_REQUEST_CONTEXT_ID'] && env['HTTP_X_REQUEST_CONTEXT_SIGNATURE']
|
||||
request_context_id = Canvas::Security.base64_decode(env['HTTP_X_REQUEST_CONTEXT_ID'])
|
||||
request_context_signature = Canvas::Security.base64_decode(env['HTTP_X_REQUEST_CONTEXT_SIGNATURE'])
|
||||
if Canvas::Security.verify_hmac_sha512(request_context_id, request_context_signature)
|
||||
return request_context_id
|
||||
else
|
||||
Rails.logger.info("ignoring X-Request-Context-Id header, signature could not be verified")
|
||||
end
|
||||
end
|
||||
SecureRandom.uuid
|
||||
end
|
||||
end
|
||||
|
|
|
@ -92,6 +92,24 @@ module Canvas::Security
|
|||
false
|
||||
end
|
||||
|
||||
def self.sign_hmac_sha512(string_to_sign, signing_secret=services_signing_secret)
|
||||
OpenSSL::HMAC.digest('sha512', signing_secret, string_to_sign)
|
||||
end
|
||||
|
||||
def self.verify_hmac_sha512(message, signature, signing_secret=services_signing_secret)
|
||||
comparison = sign_hmac_sha512(message, signing_secret)
|
||||
|
||||
if CANVAS_RAILS4_0
|
||||
return false unless signature.bytesize == comparison.bytesize
|
||||
l = signature.unpack "C#{signature.bytesize}"
|
||||
res = 0
|
||||
comparison.each_byte { |byte| res |= byte ^ l.shift }
|
||||
res == 0
|
||||
else
|
||||
ActiveSupport::SecurityUtils.secure_compare(signature, comparison)
|
||||
end
|
||||
end
|
||||
|
||||
# Creates a JWT token string
|
||||
#
|
||||
# body (Hash) - The contents of the JWT token
|
||||
|
@ -158,8 +176,8 @@ module Canvas::Security
|
|||
end
|
||||
|
||||
def self.decrypt_services_jwt(token, signing_secret=nil, encryption_secret=nil)
|
||||
signing_secret ||= Canvas::DynamicSettings.from_cache("canvas")["signing-secret"]
|
||||
encryption_secret ||= Canvas::DynamicSettings.from_cache("canvas")["encryption-secret"]
|
||||
signing_secret ||= services_signing_secret
|
||||
encryption_secret ||= services_encryption_secret
|
||||
begin
|
||||
signed_coded_jwt = JSON::JWT.decode(token, encryption_secret)
|
||||
raw_jwt = JSON::JWT.decode(signed_coded_jwt.plain_text, signing_secret)
|
||||
|
@ -304,29 +322,40 @@ module Canvas::Security
|
|||
"login_attempts:#{pseudonym.global_id}"
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def self.verify_jwt(body)
|
||||
if body[:exp].present?
|
||||
if timestamp_is_expired?(body[:exp])
|
||||
raise Canvas::Security::TokenExpired
|
||||
class << self
|
||||
private
|
||||
def verify_jwt(body)
|
||||
if body[:exp].present?
|
||||
if timestamp_is_expired?(body[:exp])
|
||||
raise Canvas::Security::TokenExpired
|
||||
end
|
||||
end
|
||||
|
||||
if body[:nbf].present?
|
||||
if timestamp_is_future?(body[:nbf])
|
||||
raise Canvas::Security::TokenInvalid
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
if body[:nbf].present?
|
||||
if timestamp_is_future?(body[:nbf])
|
||||
raise Canvas::Security::TokenInvalid
|
||||
end
|
||||
def timestamp_is_expired?(exp_val)
|
||||
now = Time.zone.now
|
||||
(exp_val.is_a?(Time) && exp_val <= now) || exp_val <= now.to_i
|
||||
end
|
||||
|
||||
def timestamp_is_future?(nbf_val)
|
||||
now = Time.zone.now
|
||||
(nbf_val.is_a?(Time) && nbf_val > now) || nbf_val > now.to_i
|
||||
end
|
||||
|
||||
def services_encryption_secret
|
||||
Canvas::DynamicSettings.from_cache("canvas")["encryption-secret"]
|
||||
end
|
||||
|
||||
def services_signing_secret
|
||||
Canvas::DynamicSettings.from_cache("canvas")["signing-secret"]
|
||||
end
|
||||
end
|
||||
|
||||
def self.timestamp_is_expired?(exp_val)
|
||||
now = Time.zone.now
|
||||
(exp_val.is_a?(Time) && exp_val <= now) || exp_val <= now.to_i
|
||||
end
|
||||
|
||||
def self.timestamp_is_future?(nbf_val)
|
||||
now = Time.zone.now
|
||||
(nbf_val.is_a?(Time) && nbf_val > now) || nbf_val > now.to_i
|
||||
end
|
||||
end
|
||||
|
|
|
@ -138,6 +138,23 @@ describe Canvas::Security do
|
|||
end
|
||||
end
|
||||
|
||||
describe "hmac_sha512" do
|
||||
it "verifies items signed with the same secret" do
|
||||
message = "asdf1234"
|
||||
shared_secret = "super-sekrit"
|
||||
signature = Canvas::Security.sign_hmac_sha512(message, shared_secret)
|
||||
verification = Canvas::Security.verify_hmac_sha512(message, signature, shared_secret)
|
||||
expect(verification).to be_truthy
|
||||
end
|
||||
|
||||
it "rejects items signed with different secrets" do
|
||||
message = "asdf1234"
|
||||
signature = Canvas::Security.sign_hmac_sha512(message, "super-sekrit")
|
||||
verification = Canvas::Security.verify_hmac_sha512(message, signature, "sekrit-super")
|
||||
expect(verification).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
if Canvas.redis_enabled?
|
||||
describe "max login attempts" do
|
||||
before do
|
||||
|
|
|
@ -85,4 +85,58 @@ describe "RequestContextGenerator" do
|
|||
expect(q / 1000000).to eq 60.0
|
||||
end
|
||||
end
|
||||
|
||||
context "when request provides an override context id" do
|
||||
let(:shared_secret){ 'sup3rs3cr3t!!' }
|
||||
let(:remote_request_context_id){ '1234-5678-9012-3456-7890-1234-5678' }
|
||||
|
||||
let(:remote_signature) do
|
||||
Canvas::Security.sign_hmac_sha512(remote_request_context_id, shared_secret)
|
||||
end
|
||||
|
||||
before(:each) do
|
||||
Thread.current[:context] = nil
|
||||
Canvas::DynamicSettings.reset_cache!
|
||||
Canvas::DynamicSettings.cache['canvas'] = {
|
||||
timetamp: Time.zone.now.to_i,
|
||||
value: { "signing-secret" => shared_secret }
|
||||
}
|
||||
env['HTTP_X_REQUEST_CONTEXT_ID'] = Canvas::Security.base64_encode(remote_request_context_id)
|
||||
env['HTTP_X_REQUEST_CONTEXT_SIGNATURE'] = Canvas::Security.base64_encode(remote_signature)
|
||||
end
|
||||
|
||||
after(:each){ Canvas::DynamicSettings.reset_cache! }
|
||||
|
||||
def run_middleware
|
||||
_, headers, _msg = RequestContextGenerator.new(->(_){ [200, {}, []] }).call(env)
|
||||
headers
|
||||
end
|
||||
|
||||
it "uses a provided request context id if another service submits one that is correctly signed" do
|
||||
headers = run_middleware
|
||||
expect(Thread.current[:context][:request_id]).to eq(remote_request_context_id)
|
||||
expect(headers['X-Request-Context-Id']).to eq(remote_request_context_id)
|
||||
end
|
||||
|
||||
it "won't accept an override without a signature" do
|
||||
env['HTTP_X_REQUEST_CONTEXT_SIGNATURE'] = nil
|
||||
headers = run_middleware
|
||||
expect(Thread.current[:context][:request_id]).not_to eq(remote_request_context_id)
|
||||
expect(headers['X-Request-Context-Id']).to eq(Thread.current[:context][:request_id])
|
||||
end
|
||||
|
||||
it "rejects a wrong signature" do
|
||||
env['HTTP_X_REQUEST_CONTEXT_SIGNATURE'] = "nonsense"
|
||||
headers = run_middleware
|
||||
expect(Thread.current[:context][:request_id]).not_to eq(remote_request_context_id)
|
||||
expect(headers['X-Request-Context-Id']).to eq(Thread.current[:context][:request_id])
|
||||
end
|
||||
|
||||
it "rejects a tampered ID" do
|
||||
env['HTTP_X_REQUEST_CONTEXT_ID'] = "I-changed-it"
|
||||
headers = run_middleware
|
||||
expect(Thread.current[:context][:request_id]).not_to eq(remote_request_context_id)
|
||||
expect(headers['X-Request-Context-Id']).to eq(Thread.current[:context][:request_id])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue