Merge pull request #123 from bodewig/hmac-signature-support-for-id-to…

…kens

Hmac signature support for id tokens

ChangeLog

@@ -1,3 +1,12 @@
+11/17/2017
+- added support for verifying HMAC signatures on id tokens - it has
+  already been supported for access tokens before
+- fix handling of signatures using unsupported algorithms on id
+  tokens. Such signatures will cause a warning to be logged but the id
+  tokens are considered valid. access tokens signed with an
+  unsupported algorithm are considered invalid. This is the same
+  behavior as has been present in 1.4.1. See #122
+
 11/15/2017
 - fix return of access_token when renew_access_token_on_expiry = false ; see #121
 

lib/resty/openidc.lua

@@ -608,8 +608,23 @@ local function openidc_pem_from_jwk(opts, kid)
   return pem
 end
 
+-- does lua-resty-jwt and/or we know how to handle the algorithm of the JWT?
+local function is_algorithm_supported(jwt_header)
+  return jwt_header and jwt_header.alg and (
+    jwt_header.alg == "none"
+    or string.sub(jwt_header.alg, 1, 2) == "RS"
+    or string.sub(jwt_header.alg, 1, 2) == "HS"
+  )
+end
+
+-- is the JWT signing algorithm an asymmetric one whose key might be
+-- obtained from the discovery endpoint?
+local function uses_asymmetric_algorithm(jwt_header)
+  return string.sub(jwt_header.alg, 1, 2) == "RS"
+end
+
 -- parse a JWT and verify its signature (if present)
-local function openidc_load_jwt_and_verify_crypto(opts, jwt_string, ...)
+local function openidc_load_jwt_and_verify_crypto(opts, jwt_string, asymmetric_secret, symmetric_secret, ...)
   local enc_hdr, enc_payload, enc_sign = string.match(jwt_string, '^(.+)%.(.+)%.(.*)$')
   if enc_payload and (not enc_sign or enc_sign == "") then
     local jwt = openidc_load_jwt_none_alg(enc_hdr, enc_payload)
@@ -625,15 +640,22 @@ local function openidc_load_jwt_and_verify_crypto(opts, jwt_string, ...)
     return nil, reason
   end
 
-  local secret = opts.secret
-  if not secret and opts.discovery then
-    ngx.log(ngx.DEBUG, "using discovery to find key")
-    local err
-    secret, err = openidc_pem_from_jwk(opts, jwt_obj.header.kid)
+  local secret
+  if is_algorithm_supported(jwt_obj.header) then
+    if uses_asymmetric_algorithm(jwt_obj.header) then
+      secret = asymmetric_secret
+      if not secret and opts.discovery then
+        ngx.log(ngx.DEBUG, "using discovery to find key")
+        local err
+        secret, err = openidc_pem_from_jwk(opts, jwt_obj.header.kid)
 
-    if secret == nil then
-      ngx.log(ngx.ERR, err)
-      return nil, err
+        if secret == nil then
+          ngx.log(ngx.ERR, err)
+          return nil, err
+        end
+      end
+    else
+      secret = symmetric_secret
     end
   end
 
@@ -655,7 +677,7 @@ local function openidc_load_jwt_and_verify_crypto(opts, jwt_string, ...)
     if jwt_obj.reason then
       reason = reason .. ": " .. jwt_obj.reason
     end
-    return nil, reason
+    return jwt_obj, reason
   end
   return jwt_obj
 end
@@ -709,9 +731,14 @@ local function openidc_authorization_response(opts, session)
   end
 
   local jwt_obj
-  jwt_obj, err = openidc_load_jwt_and_verify_crypto(opts, json.id_token)
+  jwt_obj, err = openidc_load_jwt_and_verify_crypto(opts, json.id_token, opts.secret, opts.client_secret)
   if err then
-    return nil, err, session.data.original_url, session
+    local is_unsupported_signature_error = jwt_obj and not jwt_obj.verified and not is_algorithm_supported(jwt_obj.header)
+    if is_unsupported_signature_error then
+      ngx.log(ngx.WARN, "ignored id_token signature as algorithm '" .. jwt_obj.header.alg .. "' is not supported")
+    else
+      return nil, err, session.data.original_url, session
+    end
   end
   local id_token = jwt_obj.payload
 
@@ -1126,7 +1153,7 @@ function openidc.jwt_verify(access_token, opts, ...)
   local v = openidc_cache_get("introspection", access_token)
   if not v then
     local jwt_obj
-    jwt_obj, err = openidc_load_jwt_and_verify_crypto(opts, access_token, ...)
+    jwt_obj, err = openidc_load_jwt_and_verify_crypto(opts, access_token, opts.secret, opts.secret, ...)
     if not err then
       json = jwt_obj.payload
       ngx.log(ngx.DEBUG, "jwt: ", cjson.encode(json))

tests/spec/bearer_token_verification_spec.lua

@@ -2,19 +2,6 @@ local http = require("socket.http")
 local test_support = require("test_support")
 require 'busted.runner'()
 
-local function none_signature_jwt(payload, alg)
-  local function b64url(s)
-    local dkjson = require "dkjson"
-    local mime = require "mime"
-    return mime.b64(dkjson.encode(s)):gsub('+','-'):gsub('/','_')
-  end
-  local header = b64url({
-      typ = "JWT",
-      alg = alg or "none"
-  })
-  return header .. "." .. b64url(payload) .. "."
-end
-
 local function base_checks()
   local jwt = test_support.trim(http.request("http://127.0.0.1/jwt"))
   describe("and not sending any Authorization header", function()
@@ -237,7 +224,7 @@ describe("when using a JWT not signed but using the 'none' alg", function()
     jwk = test_support.load("/spec/jwks_with_two_keys.json"),
   })
   teardown(test_support.stop_server)
-  local jwt = none_signature_jwt({
+  local jwt = test_support.self_signed_jwt({
       exp = os.time() + 3600,
   })
   local _, status = http.request({
@@ -259,7 +246,7 @@ describe("when using an unsigned JWT with an alg different from 'none'", functio
     jwk = test_support.load("/spec/jwks_with_two_keys.json"),
   })
   teardown(test_support.stop_server)
-  local jwt = none_signature_jwt({
+  local jwt = test_support.self_signed_jwt({
       exp = os.time() + 3600,
   }, "RS256")
   local _, status = http.request({
@@ -291,6 +278,21 @@ describe("when the bearer token doesn't contain JSON but looks like an unsigned
   end)
 end)
 
+describe("when the JWT claims to be signed by an unsupported algorithm", function()
+  test_support.start_server({
+    fake_access_token_signature = "true"
+  })
+  teardown(test_support.stop_server)
+  local jwt = test_support.trim(http.request("http://127.0.0.1/jwt"))
+  local _, status = http.request({
+      url = "http://127.0.0.1/verify_bearer_token",
+      headers = { authorization = "Bearer " .. jwt }
+  })
+  it("the token is invalid", function()
+    assert.are.equals(401, status)
+  end)
+end)
+
 describe("when discovery endpoint is not resolvable", function()
   test_support.start_server({
     verify_opts = {

tests/spec/id_token_validation_spec.lua

@@ -219,17 +219,30 @@ describe("when the id token signature key isn't part of the JWK", function()
   end)
 end)
 
-describe("when the id token signature uses a known symmetric key", function()
+describe("when the id token signature uses a symmetric algorithm", function()
   test_support.start_server({
-    jwt_verify_secret = "secret",
+    jwt_verify_secret = "client_secret",
     token_header = {
       alg = "HS256",
     },
-    oidc_opts = { secret = "secret" }
   })
   teardown(test_support.stop_server)
   local _, status = test_support.login()
   it("login succeeds", function()
     assert.are.equals(302, status)
   end)
 end)
+
+describe("when the id claims to be signed by an unsupported algorithm", function()
+  test_support.start_server({
+    fake_id_token_signature = "true"
+  })
+  teardown(test_support.stop_server)
+  local _, status = test_support.login()
+  it("login succeeds", function()
+    assert.are.equals(302, status)
+  end)
+  it("an error is logged", function()
+    assert.error_log_contains("ignored id_token signature as algorithm 'AB256' is not supported")
+  end)
+end)

tests/spec/test_support.lua

@@ -49,6 +49,19 @@ function test_support.trim(s)
   return s:gsub("^%s*(.-)%s*$", "%1")
 end
 
+function test_support.self_signed_jwt(payload, alg, signature)
+  local function b64url(s)
+    local dkjson = require "dkjson"
+    local mime = require "mime"
+    return mime.b64(dkjson.encode(s)):gsub('+','-'):gsub('/','_')
+  end
+  local header = b64url({
+      typ = "JWT",
+      alg = alg or "none"
+  })
+  return header .. "." .. b64url(payload) .. "." .. (signature or "")
+end
+
 local DEFAULT_JWT_VERIFY_SECRET = test_support.load("/spec/private_rsa_key.pem")
 
 local DEFAULT_JWK = test_support.load("/spec/rsa_key_jwk_with_x5c.json")
@@ -66,6 +79,8 @@ local DEFAULT_TOKEN_RESPONSE_EXPIRES_IN = "3600"
 
 local DEFAULT_TOKEN_RESPONSE_CONTAINS_REFRESH_TOKEN = "true"
 local DEFAULT_REFRESHING_TOKEN_FAILS = "false"
+local DEFAULT_FAKE_ACCESS_TOKEN_SIGNATURE = "false"
+local DEFAULT_FAKE_ID_TOKEN_SIGNATURE = "false"
 
 local DEFAULT_DELAY_RESPONSE = "0"
 
@@ -83,6 +98,9 @@ http {
     lua_package_path '~/lua/?.lua;;';
     lua_shared_dict discovery 1m;
     init_by_lua_block {
+        jwks = [=[JWK]=]
+        secret = [=[
+JWT_VERIFY_SECRET]=]
         if os.getenv('coverage') then
           require("luacov.runner")("/spec/luacov/settings.luacov")
         end
@@ -95,6 +113,25 @@ JWT_VERIFY_SECRET]=]
             ngx.sleep(delay_response / 1000)
           end
         end
+        create_jwt = function(payload, fake_signature)
+          if not fake_signature then
+            local jwt_content = {
+              header = TOKEN_HEADER,
+              payload = payload
+            }
+            local jwt = require "resty.jwt"
+            return jwt:sign(secret, jwt_content)
+          else
+            local function b64url(s)
+              return ngx.encode_base64(cjson.encode(s)):gsub('+','-'):gsub('/','_')
+            end
+            local header = b64url({
+                typ = "JWT",
+                alg = "AB256"
+            })
+            return header .. "." .. b64url(payload) .. ".NOT_A_VALID_SIGNATURE"
+          end
+        end
     }
 
     resolver      8.8.8.8;
@@ -109,12 +146,7 @@ JWT_VERIFY_SECRET]=]
 
         location /jwt {
             content_by_lua_block {
-                local jwt_content = {
-                  header = TOKEN_HEADER,
-                  payload = ACCESS_TOKEN
-                }
-                local jwt = require "resty.jwt"
-                local jwt_token = jwt:sign(secret, jwt_content)
+                local jwt_token = create_jwt(ACCESS_TOKEN, FAKE_ACCESS_TOKEN_SIGNATURE)
                 ngx.header.content_type = 'text/plain'
                 ngx.say(jwt_token)
             }
@@ -124,7 +156,7 @@ JWT_VERIFY_SECRET]=]
             content_by_lua_block {
                 ngx.header.content_type = 'application/json;charset=UTF-8'
                 delay(JWK_DELAY_RESPONSE)
-                ngx.say([=[JWK]=])
+                ngx.say(jwks)
             }
         }
 
@@ -175,12 +207,7 @@ JWT_VERIFY_SECRET]=]
                   access_token = access_token .. "2"
                   refresh_token = refresh_token .. "2"
                 end
-                local jwt_content = {
-                  header = TOKEN_HEADER,
-                  payload = id_token
-                }
-                local jwt = require "resty.jwt"
-                local jwt_token = jwt:sign(secret, jwt_content)
+                local jwt_token = create_jwt(id_token, FAKE_ID_TOKEN_SIGNATURE)
                 local token_response = {
                   access_token = access_token,
                   expires_in = TOKEN_RESPONSE_EXPIRES_IN,
@@ -323,7 +350,6 @@ local function write_config(out, custom_config)
   end
   local config = DEFAULT_CONFIG_TEMPLATE
     :gsub("OIDC_CONFIG", serpent.block(oidc_config, {comment = false }))
-    :gsub("ID_TOKEN", serpent.block(id_token, {comment = false }))
     :gsub("TOKEN_HEADER", serpent.block(token_header, {comment = false }))
     :gsub("JWT_VERIFY_SECRET", custom_config["jwt_verify_secret"] or DEFAULT_JWT_VERIFY_SECRET)
     :gsub("VERIFY_OPTS", serpent.block(verify_opts, {comment = false }))
@@ -333,14 +359,17 @@ local function write_config(out, custom_config)
     :gsub("TOKEN_RESPONSE_CONTAINS_REFRESH_TOKEN", token_response_contains_refresh_token)
     :gsub("REFRESHING_TOKEN_FAILS", refreshing_token_fails)
     :gsub("ACCESS_TOKEN_OPTS", serpent.block(access_token_opts, {comment = false }))
-    :gsub("ACCESS_TOKEN", serpent.block(access_token, {comment = false }))
     :gsub("JWK_DELAY_RESPONSE", ((custom_config["delay_response"] or {}).jwk or DEFAULT_DELAY_RESPONSE))
     :gsub("TOKEN_DELAY_RESPONSE", ((custom_config["delay_response"] or {}).token or DEFAULT_DELAY_RESPONSE))
     :gsub("DISCOVERY_DELAY_RESPONSE", ((custom_config["delay_response"] or {}).discovery or DEFAULT_DELAY_RESPONSE))
     :gsub("USERINFO_DELAY_RESPONSE", ((custom_config["delay_response"] or {}).userinfo or DEFAULT_DELAY_RESPONSE))
     :gsub("INTROSPECTION_DELAY_RESPONSE", ((custom_config["delay_response"] or {}).introspection or DEFAULT_DELAY_RESPONSE))
     :gsub("JWK", custom_config["jwk"] or DEFAULT_JWK)
     :gsub("USERINFO", serpent.block(userinfo, {comment = false }))
+    :gsub("FAKE_ACCESS_TOKEN_SIGNATURE", custom_config["fake_access_token_signature"] or DEFAULT_FAKE_ACCESS_TOKEN_SIGNATURE)
+    :gsub("FAKE_ID_TOKEN_SIGNATURE", custom_config["fake_id_token_signature"] or DEFAULT_FAKE_ID_TOKEN_SIGNATURE)
+    :gsub("ID_TOKEN", serpent.block(id_token, {comment = false }))
+    :gsub("ACCESS_TOKEN", serpent.block(access_token, {comment = false }))
   out:write(config)
 end
 
@@ -370,6 +399,10 @@ end
 -- - delay_response is a table specifying a delay for the response of various endpoint in ms
 --   { jwk = 1, token = 1, discovery = 1, userinfo = 1, introspection = 1}
 -- - refreshing_token_fails whether to grant an access token via the refresh token grant
+-- - fake_access_token_signature whether to fake a JWT signature with unknown algorithm for the
+--   JWT returned by /jwt
+-- - fake_id_token_signature whether to fake a JWT signature with unknown algorithm for the
+--   id_token
 function test_support.start_server(custom_config)
   assert(os.execute("rm -rf /tmp/server"), "failed to remove old server dir")
   assert(os.execute("mkdir -p /tmp/server/conf"), "failed to create server dir")