Merge pull request #3 from arkedge/validate-signer

Validate the `signer` field in the JWT header

.rubocop.yml

@@ -44,3 +44,6 @@ RSpec/ExampleLength:
 
 RSpec/MultipleExpectations:
   Enabled: false
+
+RSpec/MultipleMemoizedHelpers:
+  Enabled: false

README.md

@@ -30,6 +30,8 @@ The plugin can be configured using the following environment variables:
 
 - `REDMINE_AMZN_ALB_AUTHN_KEY_ENDPOINT`
   - **(required)** Public key endpoint, e.g., `https://public-keys.auth.elb.ap-northeast-1.amazonaws.com` when the ALB is in the `ap-northeast-1` region.
+- `REDMINE_AMZN_ALB_AUTHN_ALB_ARN`
+  - **(required)** The ARN of the Application Load Balancer expected by the `signer` field in the JWT header.
 - `REDMINE_AMZN_ALB_AUTHN_ISS`
   - If set, the plugin will verify that the `iss` claim has the same value.
 

init.rb

@@ -12,6 +12,7 @@
 end
 
 RedmineAmznAlbAuthn.key_endpoint = ENV.fetch('REDMINE_AMZN_ALB_AUTHN_KEY_ENDPOINT')
+RedmineAmznAlbAuthn.alb_arn = ENV.fetch('REDMINE_AMZN_ALB_AUTHN_ALB_ARN')
 RedmineAmznAlbAuthn.iss = ENV.fetch('REDMINE_AMZN_ALB_AUTHN_ISS', nil)
 
 ApplicationController.prepend(RedmineAmznAlbAuthn::ApplicationControllerPatch)

lib/redmine_amzn_alb_authn.rb

@@ -6,5 +6,5 @@
 
 # Plugin namespace and configuration store.
 module RedmineAmznAlbAuthn
-  mattr_accessor :key_endpoint, :iss
+  mattr_accessor :key_endpoint, :alb_arn, :iss
 end

lib/redmine_amzn_alb_authn/application_controller_patch.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require_relative 'errors'
 require_relative 'oidc_data_decoder'
 
 module RedmineAmznAlbAuthn
@@ -27,11 +28,12 @@ def find_user_by_amzn_alb_header
 
       decoder = OidcDataDecoder.new(
         key_endpoint: RedmineAmznAlbAuthn.key_endpoint,
+        alb_arn: RedmineAmznAlbAuthn.alb_arn,
         iss: RedmineAmznAlbAuthn.iss,
       )
       begin
         payload, _header = decoder.verify_and_decode!(amzn_oidc_data)
-      rescue JWT::DecodeError, Net::HTTPExceptions => e
+      rescue InvalidSignerError, JWT::DecodeError, Net::HTTPExceptions => e
         logger.error(e)
         return
       end

lib/redmine_amzn_alb_authn/errors.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+module RedmineAmznAlbAuthn
+  Error = Class.new(StandardError)
+  InvalidSignerError = Class.new(Error)
+end

lib/redmine_amzn_alb_authn/oidc_data_decoder.rb

@@ -7,18 +7,21 @@
 require 'active_support/core_ext/numeric/time'
 require 'jwt'
 
+require_relative 'errors'
+
 module RedmineAmznAlbAuthn
   # Verifies and decodes the JWT from the X-Amzn-Oidc-Data header sent by ALB.
   class OidcDataDecoder
     class_attribute :key_cache, default: ActiveSupport::Cache::MemoryStore.new(expires_in: 1.hour)
 
-    def initialize(key_endpoint:, iss: nil)
+    def initialize(key_endpoint:, alb_arn:, iss: nil)
       @key_endpoint = key_endpoint
+      @alb_arn = alb_arn
       @iss = iss
     end
 
     def verify_and_decode!(oidc_data)
-      JWT.decode(oidc_data, nil, true, algorithm: 'ES256', iss: @iss, verify_iss: true) do |header|
+      payload, header = JWT.decode(oidc_data, nil, true, algorithm: 'ES256', iss: @iss, verify_iss: true) do |header|
         key_str = key_cache.fetch(header.fetch('kid')) do
           key_uri = URI.join(@key_endpoint, header['kid'])
           key_response = Net::HTTP.get_response(key_uri)
@@ -27,6 +30,10 @@ def verify_and_decode!(oidc_data)
         end
         OpenSSL::PKey::EC.new(key_str)
       end
+
+      raise InvalidSignerError, "Invalid signer: #{header['signer']}" if header['signer'] != @alb_arn
+
+      [payload, header]
     end
   end
 end

spec/redmine_amzn_alb_authn/oidc_data_decoder_spec.rb

@@ -1,14 +1,17 @@
 # frozen_string_literal: true
 
 RSpec.describe RedmineAmznAlbAuthn::OidcDataDecoder do
-  subject(:decoder) { described_class.new(key_endpoint: 'https://example.com', iss:) }
+  subject(:decoder) { described_class.new(key_endpoint: 'https://example.com', alb_arn:, iss: expected_iss) }
 
-  let(:iss) { 'my-issuer' }
+  let(:alb_arn) { 'arn:aws:elasticloadbalancing:ap-northeast-1:012345678901:loadbalancer/app/my-alb/0123456789abcdef' }
+  let(:expected_iss) { 'https://iss.example.com' }
 
   describe '#verify_and_decode!' do
     let(:oidc_data) do
-      JWT.encode({ sub: 1, exp: 2.minutes.from_now.to_i, iss: }, private_key, 'ES256', typ: 'JWT', kid:)
+      JWT.encode({ sub: 1, exp: 2.minutes.from_now.to_i, iss: }, private_key, 'ES256', kid:, signer:)
     end
+    let(:iss) { expected_iss }
+    let(:signer) { alb_arn }
     let(:kid) { SecureRandom.uuid }
     let(:private_key) { OpenSSL::PKey::EC.generate('prime256v1') }
 
@@ -21,12 +24,12 @@
       payload, header = decoder.verify_and_decode!(oidc_data)
 
       expect(payload).to include 'exp', 'sub' => 1
-      expect(header).to include 'kid', 'typ' => 'JWT', 'alg' => 'ES256'
+      expect(header).to include 'kid', 'signer', 'alg' => 'ES256'
     end
 
     context 'with a payload-modified JWT' do
       let(:oidc_data) do
-        jwt = JWT.encode({ sub: 1, exp: 2.minutes.from_now.to_i }, private_key, 'ES256', typ: 'JWT', kid:)
+        jwt = JWT.encode({ sub: 1, exp: 2.minutes.from_now.to_i }, private_key, 'ES256', kid:, signer:)
         header, _payload, signature = jwt.split('.')
         modified_payload = JWT::Base64.url_encode({ sub: 'modified' }.to_json)
         "#{header}.#{modified_payload}.#{signature}"
@@ -38,18 +41,24 @@
     end
 
     context 'with a JWT that has unexpected iss' do
-      let(:oidc_data) do
-        JWT.encode({ sub: 1, exp: 2.minutes.from_now.to_i, iss: 'unknown' }, private_key, 'ES256', typ: 'JWT', kid:)
-      end
+      let(:iss) { 'unexpected' }
 
       it 'raises JWT::InvalidIssuerError' do
         expect { decoder.verify_and_decode!(oidc_data) }.to raise_error(JWT::InvalidIssuerError)
       end
     end
 
+    context 'with a JWT that has unexpected signer' do
+      let(:signer) { 'unexpected' }
+
+      it 'raises RedmineAmznAlbAuthn::InvalidSignerError' do
+        expect { decoder.verify_and_decode!(oidc_data) }.to raise_error(RedmineAmznAlbAuthn::InvalidSignerError)
+      end
+    end
+
     context 'when the method is called twice with JWTs signed by the same key' do
       let(:oidc_data2) do
-        JWT.encode({ sub: 2, exp: 2.minutes.from_now.to_i, iss: }, private_key, 'ES256', typ: 'JWT', kid:)
+        JWT.encode({ sub: 2, exp: 2.minutes.from_now.to_i, iss: }, private_key, 'ES256', kid:, signer:)
       end
 
       it 'uses a cached key' do