From 4014a971e0b818bc501c41a4161eb6c8c91c9344 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 28 Jun 2018 11:12:37 -0600 Subject: [PATCH] allow 5 minutes of future clock skew when verifying JWTs fixes CORE-1439 Change-Id: Ia3ed12dd79cee475bedd0879323eacf3a0325476 Reviewed-on: https://gerrit.instructure.com/155617 Tested-by: Jenkins Reviewed-by: Brent Burgoyne Product-Review: Cody Cutrer QA-Review: Cody Cutrer --- lib/canvas/security.rb | 22 ++++++++++++---------- spec/lib/canvas/security_spec.rb | 6 ++++++ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/canvas/security.rb b/lib/canvas/security.rb index 91aed9acc0c..6a03f819756 100644 --- a/lib/canvas/security.rb +++ b/lib/canvas/security.rb @@ -329,27 +329,29 @@ module Canvas::Security class << self private def verify_jwt(body, ignore_expiration: false) + verification_time = Time.now.utc + if body[:iat].present? + iat = timestamp_as_integer(body[:iat]) + if iat > verification_time.to_i && iat < verification_time.to_i + 300 + verification_time = iat + end + end + if body[:exp].present? && !ignore_expiration - if timestamp_is_expired?(body[:exp]) + if timestamp_as_integer(body[:exp]) < verification_time.to_i raise Canvas::Security::TokenExpired end end if body[:nbf].present? - if timestamp_is_future?(body[:nbf]) + if timestamp_as_integer(body[:nbf]) > verification_time.to_i raise Canvas::Security::InvalidToken end end 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 + def timestamp_as_integer(timestamp) + timestamp.is_a?(Time) ? timestamp.to_i : timestamp end def services_encryption_secret diff --git a/spec/lib/canvas/security_spec.rb b/spec/lib/canvas/security_spec.rb index 50f63d2c6f5..be8ea186f2c 100644 --- a/spec/lib/canvas/security_spec.rb +++ b/spec/lib/canvas/security_spec.rb @@ -136,6 +136,12 @@ describe Canvas::Security do ) end + it "allows 5 minutes of future clock skew" do + back_to_the_future_jwt = test_jwt(exp: 1.hour.from_now, nbf: 1.minutes.from_now, iat: 1.minutes.from_now) + body = Canvas::Security.decode_jwt(back_to_the_future_jwt, [ key ]) + expect(body[:a]).to eq 1 + end + it "produces an InvalidToken error if string isn't a jwt (even if it looks like one)" do # this is an example token which base64_decodes to a thing that looks like a jwt because of the periods not_a_jwt = Canvas::Security.base64_decode("1050~LvwezC5Dd3ZK9CR1lusJTRv24dN0263txia3KF3mU6pDjOv5PaoX8Jv4ikdcvoiy")