diff --git a/Changelog.md b/Changelog.md index faf142a966..a15d2868c3 100644 --- a/Changelog.md +++ b/Changelog.md @@ -28,6 +28,7 @@ - Add pagination to Admin Users table for performance (#7997) - Added support for all annotation types for POST /add_annotations (#8007) - Added GET /test_runs API route (#8055) +- Add multi-key JWKS rotation for LTI 1.3 signing keys (#8056) ### 🐛 Bug fixes - Prevented grader assignment and unassignment operations from modifying groupings belonging to other assignments (#8072) diff --git a/app/controllers/lti_deployments_controller.rb b/app/controllers/lti_deployments_controller.rb index aca2a6576c..33cd3502f0 100644 --- a/app/controllers/lti_deployments_controller.rb +++ b/app/controllers/lti_deployments_controller.rb @@ -166,9 +166,8 @@ def redirect_login end def public_jwk - key = OpenSSL::PKey::RSA.new File.read(LtiClient::KEY_PATH) - jwk = JWT::JWK.new(key) - render json: { keys: [jwk.export] } + response.set_header('Cache-Control', 'public, max-age=300') + render json: LtiKeyStore.public_jwks end def course_not_set_up diff --git a/app/jobs/lti_key_maintenance_job.rb b/app/jobs/lti_key_maintenance_job.rb new file mode 100644 index 0000000000..c779f6689a --- /dev/null +++ b/app/jobs/lti_key_maintenance_job.rb @@ -0,0 +1,11 @@ +# Rotates the LTI signing key when due and prunes retired keys past the +# overlap window. Scheduled via Settings.resque_scheduler; only runs when +# Settings.lti.rotation.enabled is true. +class LtiKeyMaintenanceJob < ApplicationJob + def perform + return unless Settings.lti&.rotation&.enabled + + LtiKeyStore.rotate_if_due! + LtiKeyStore.prune! + end +end diff --git a/app/lib/lti_key_store.rb b/app/lib/lti_key_store.rb new file mode 100644 index 0000000000..eee07d3ca7 --- /dev/null +++ b/app/lib/lti_key_store.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'time' + +# Manages the RSA key material MarkUs uses to sign LTI 1.3 client_credentials +# assertions to Canvas, and to publish its public JWKS. +# +# Rotation model (zero-downtime): +# * Keys live as PEM files in key_dir, named lti_key_.pem. +# * The "current" signer is the newest file (paths sorted lexicographically, +# which matches chronological order given the zero-padded UTC timestamp), +# overridable via Settings.lti.rotation.current_key. +# * public_jwks publishes ALL keys present, so Canvas can still verify +# assertions signed by an outgoing key until they expire and its JWKS +# cache refreshes. +# * rake markus:rotate_if_due adds a new key when the current one is past +# its max age; rake markus:prune_keys removes keys past the overlap window. +module LtiKeyStore + module_function + + KEY_GLOB = 'lti_key_*.pem' + + def key_dir + Settings.lti&.rotation&.key_dir || + File.join(Settings.file_storage.default_root_path, 'lti', 'keys') + end + + # All private-key PEM paths, newest first. If the rotation dir is empty, + # fall back to the legacy single key.pem (LtiClient::KEY_PATH). + def key_paths + paths = Dir.glob(File.join(key_dir, KEY_GLOB)).sort.reverse + return paths if paths.any? + + File.exist?(LtiClient::KEY_PATH) ? [LtiClient::KEY_PATH] : [] + end + + # The RSA key MarkUs signs NEW assertions with. + def current_key + path = explicit_current || key_paths.first + raise 'No LTI signing key found' if path.nil? + + OpenSSL::PKey::RSA.new(File.read(path)) + end + + # JWK wrapper for the current signer (used to set the `kid` header). + def current_jwk + JWT::JWK.new(current_key) + end + + # Public JWKS: every published key, exported public-only. + # NB: JWT::JWK#export returns public members only unless include_private: true. + def public_jwks + { keys: key_paths.map { |p| JWT::JWK.new(OpenSSL::PKey::RSA.new(File.read(p))).export } } + end + + # Optional explicit override, e.g. Settings.lti.rotation.current_key = 'lti_key_20260101T000000Z.pem' + # Raises rather than silently falling back to the newest key: pinning is a + # deliberate operator action (typically compromise response), so quietly + # signing with a different key would defeat the point and hide the mistake. + def explicit_current + name = Settings.lti&.rotation&.current_key + return if name.nil? + + path = File.join(key_dir, name) + raise "Pinned LTI signing key not found: #{path} (check Settings.lti.rotation.current_key)" unless File.exist?(path) + + path + end + + # Creation time encoded in the filename (UTC); falls back to mtime. + def created_at(path) + ts = File.basename(path)[/\d{8}T\d{6}Z/] + ts ? Time.parse(ts).utc : File.mtime(path).utc + end + + # Mint a new key; it becomes the current signer. Returns its path. + def rotate! + FileUtils.mkdir_p(key_dir) + key = OpenSSL::PKey::RSA.new(2048) + path = File.join(key_dir, "lti_key_#{Time.now.utc.strftime('%Y%m%dT%H%M%SZ')}.pem") + File.open(path, File::WRONLY | File::CREAT | File::EXCL, 0o600) { |f| f.write(key.to_pem) } + Rails.logger.info("LTI key rotated: #{File.basename(path)} (kid=#{JWT::JWK.new(key).kid})") + path + end + + # Rotate only if the current signer is past its max age (or there is none). + def rotate_if_due! + max_age = Settings.lti.rotation.max_age_days.days + current = key_paths.first + age = current && (Time.now.utc - created_at(current)) + return rotate! if age.nil? || age > max_age + + Rails.logger.info("LTI key #{(age / 1.day).round(1)}d old; no rotation (threshold #{max_age / 1.day}d)") + nil + end + + # Delete retired keys past the overlap window. A key is retired when its + # successor was created. Neither the newest key nor a key pinned via + # Settings.lti.rotation.current_key is ever pruned -- deleting the current + # signer would leave MarkUs unable to sign. + def prune! + overlap = Settings.lti.rotation.overlap_days.days + paths = key_paths + pinned = explicit_current + now = Time.now.utc + + paths.each_with_index.filter_map do |path, i| + next if i.zero? + next if pinned && path == pinned + + age = now - created_at(paths[i - 1]) + next unless age > overlap + + File.delete(path) + Rails.logger.info("Pruned LTI key #{File.basename(path)} (retired #{(age / 1.day).round(1)}d ago)") + path + end + end +end diff --git a/app/models/lti_client.rb b/app/models/lti_client.rb index a2a9f56add..d8ccf6137c 100644 --- a/app/models/lti_client.rb +++ b/app/models/lti_client.rb @@ -42,8 +42,7 @@ def get_oauth_token(scopes) iat: iat, jti: jti } - key = OpenSSL::PKey::RSA.new File.read(KEY_PATH) - jwk = JWT::JWK.new(key) + jwk = LtiKeyStore.current_jwk token = JWT.encode payload, jwk.keypair, 'RS256', { kid: jwk.kid } # encode and add kid as a header # See https://canvas.instructure.com/doc/api/file.oauth_endpoints.html#post-login-oauth2-token client_credentials_request = { diff --git a/config/initializers/config.rb b/config/initializers/config.rb index 2fc56ec8bc..25d64f045f 100644 --- a/config/initializers/config.rb +++ b/config/initializers/config.rb @@ -198,6 +198,13 @@ required(:token_endpoint).filled(:string) optional(:unpermitted_new_course_message).filled(:string) required(:sync_schedule).filled(:string) + optional(:rotation).hash do + required(:enabled).filled(:bool) + required(:max_age_days).value(:integer, gt?: 0) + required(:overlap_days).value(:integer, gt?: 0) + optional(:key_dir).filled(:string) + optional(:current_key).filled(:string) + end end end end diff --git a/config/settings.yml b/config/settings.yml index f4c98f6d42..9f1d8a5254 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -96,6 +96,13 @@ resque_scheduler: job_class: CleanTmpJob arguments: - 5184000 # 60 days, in seconds + LtiKeyMaintenanceJob: + class: ActiveJob::QueueAdapters::ResqueAdapter::JobWrapper + queue: DEFAULT_QUEUE + every: 1d + args: + job_class: LtiKeyMaintenanceJob + arguments: [ ] autotest: student_test_buffer_minutes: 60 @@ -126,3 +133,9 @@ max_zip_total_size: 500000000 resque: permitted_hosts: [".localhost", ".internal"] + +lti: + rotation: + enabled: false # off everywhere by default + max_age_days: 90 # rotate the current key once it's older than this + overlap_days: 7 # keep a retired key published this long diff --git a/docs/docs/administrators/configuration.md b/docs/docs/administrators/configuration.md index 3773f524ac..f94d939719 100644 --- a/docs/docs/administrators/configuration.md +++ b/docs/docs/administrators/configuration.md @@ -290,7 +290,7 @@ If you wish to use Learning Tools Interoperability (LTI) with MarkUs, you'll nee - `lti.sync_schedule` must be a cron schedule dictating when MarkUs should attempt to automatically sync its roster via LTI. You must also create a private key for generating Javascript Web Tokens to sign LTI requests. -A private key can be automatically created with the `markus:lti_key` rake task. +A private key can be automatically created with the `markus:lti_key` rake task (see "LTI Key Rotation" below). If you wish to filter course creation requests from LTI deployments, add the following keys: @@ -298,6 +298,51 @@ If you wish to filter course creation requests from LTI deployments, add the fol - `lti.unpermitted_new_course_message` must be a message to display if an LTI deployment is rejected by the filter. The message must be a string with interpolation key `%{course_name}`, which will be bound to the `title` field in the launch claim `https://purl.imsglobal.org/spec/lti/claim/context`. - Example: `"You are not permitted to create a new MarkUs course for %{course_name}. Please contact your system administrator."` +### LTI Key Rotation + +MarkUs signs LTI requests with an RSA private key, and publishes the corresponding public key at the `public_jwk` endpoint. The external platform (for example, Canvas) fetches this endpoint to verify those signatures. + +Keys are stored as timestamped PEM files in a rotation directory (by default, an `lti/keys` subdirectory under `file_storage.default_root_path`). The newest key is the current signer, and **every** key in the directory is published. This means a key that has been rotated out can still verify assertions that were signed before the rotation, so keys can be rotated without disrupting grade passback or roster syncing. + +Rotation is disabled by default. To schedule it automatically, set the following keys: + +```yaml +lti: + rotation: + enabled: # boolean indicating whether to automatically rotate the LTI signing key (default: false) + max_age_days: # rotate the current key once it is older than this many days + overlap_days: # keep a rotated-out key published for this many days before deleting it + key_dir: # (optional) absolute path to the directory holding the key files (if null, a subdirectory under the default_root_path will be used) + current_key: # (optional) file name of a specific key to sign with, overriding the default of signing with the newest key (the file must exist in the key directory) +``` + +When `enabled` is true, the `LtiKeyMaintenanceJob` runs daily. It generates a new key once the current one is older than `max_age_days`, and deletes any key that has been rotated out for longer than `overlap_days`. The current signing key is never deleted. + +`overlap_days` should comfortably exceed the lifetime of a signed assertion (one hour) plus however long the platform caches the published keys. Seven days is a safe default. + +>**Note**: scheduled rotation requires the `resque-scheduler` process to be running. If your MarkUs instance runs on more than one application server, all of them must share the same key directory. + +#### Rake tasks + +- `markus:lti_key` generates a new key immediately and makes it the current signer. Use this to create the initial key, or to rotate right away if a key may have been compromised. +- `markus:rotate_if_due` generates a new key only if the current one is older than `max_age_days`. +- `markus:prune_keys` deletes keys that have been rotated out for longer than `overlap_days`. + +#### Migrating an existing instance + +If your instance predates key rotation, it has a single key at `lti/key.pem`. MarkUs continues to sign with this file if the rotation directory is empty, so upgrading requires no action and nothing is rotated unless you enable it. + +To start rotating without changing the key the platform already trusts, copy the existing key into the rotation directory **before** rotating for the first time. Copying preserves the key, and therefore the key id that the platform has already seen: + +```sh +mkdir -p /lti/keys +cp /lti/key.pem \ + /lti/keys/lti_key_$(date -u +%Y%m%dT%H%M%SZ).pem +chmod 600 /lti/keys/lti_key_*.pem +``` + +Confirm that the `public_jwk` endpoint still serves the same key, then delete the original `lti/key.pem`. Rotating before copying also works, but replaces the published key immediately instead of keeping the previous one available during the overlap window. + ## Optional Features ### Preview RMarkdown Files as HTML diff --git a/lib/tasks/lti_key.rake b/lib/tasks/lti_key.rake index 4c664fe9f1..fefedcf316 100644 --- a/lib/tasks/lti_key.rake +++ b/lib/tasks/lti_key.rake @@ -1,11 +1,16 @@ namespace :markus do - desc 'Rotate LTI private key' + desc 'Generate a new LTI signing key and make it the current signer' task lti_key: :environment do - print('Creating new private key') - key = OpenSSL::PKey::RSA.new(2048) - FileUtils.mkdir_p(File.dirname(LtiClient::KEY_PATH)) - f = File.new(LtiClient::KEY_PATH, 'w') - f.write(key.to_s) - f.close + puts "New current LTI key: #{LtiKeyStore.rotate!}" + end + + desc 'Rotate the LTI signing key if the current one is past Settings.lti.rotation.max_age_days' + task rotate_if_due: :environment do + LtiKeyStore.rotate_if_due! + end + + desc 'Remove retired LTI keys past Settings.lti.rotation.overlap_days' + task prune_keys: :environment do + LtiKeyStore.prune! end end diff --git a/spec/controllers/lti_deployments_controller_spec.rb b/spec/controllers/lti_deployments_controller_spec.rb index 391ad43a92..13dbca5ad0 100644 --- a/spec/controllers/lti_deployments_controller_spec.rb +++ b/spec/controllers/lti_deployments_controller_spec.rb @@ -222,6 +222,12 @@ end describe '#public_jwk' do + let(:key) { OpenSSL::PKey::RSA.new(2048) } + + before do + allow(LtiKeyStore).to receive(:public_jwks).and_return({ keys: [JWT::JWK.new(key).export] }) + end + it 'responds with success when logged out' do get :public_jwk expect(subject).to respond_with(:success) @@ -250,8 +256,6 @@ end context 'an individual key' do - let(:pub_jwk) { get :public_jwk } - let(:hash_jwk) { JSON.parse(pub_jwk.body) } let(:jwk_key) { hash_jwk['keys'][0] } it 'stores the correct signing algorithm' do @@ -264,12 +268,39 @@ it 'verifies a signed message' do payload = { test: 'data' } - token = JWT.encode payload, File.read(LtiClient::KEY_PATH), 'RS256', { kid: jwk_key['kid'] } + token = JWT.encode payload, key, 'RS256', { kid: jwk_key['kid'] } decoded = JWT.decode(token, nil, true, algorithms: ['RS256'], verify_iss: false, verify_aud: false, jwks: hash_jwk) expect(decoded[0]['test']).to match('data') end end end + + context 'with multiple keys published' do + let(:retired_key) { OpenSSL::PKey::RSA.new(2048) } + let(:hash_jwk) { JSON.parse(get(:public_jwk).body) } + + before do + allow(LtiKeyStore).to receive(:public_jwks).and_return( + { keys: [JWT::JWK.new(key).export, JWT::JWK.new(retired_key).export] } + ) + end + + it 'publishes every key in the set' do + expect(hash_jwk['keys'].length).to eq(2) + end + + it 'publishes a kid for each key' do + kids = hash_jwk['keys'].pluck('kid') + expect(kids).to contain_exactly(JWT::JWK.new(key).kid, JWT::JWK.new(retired_key).kid) + end + + it 'verifies a message signed by a retired key' do + token = JWT.encode({ test: 'data' }, retired_key, 'RS256', { kid: JWT::JWK.new(retired_key).kid }) + decoded = JWT.decode(token, nil, true, algorithms: ['RS256'], verify_iss: false, verify_aud: false, + jwks: hash_jwk) + expect(decoded[0]['test']).to match('data') + end + end end end diff --git a/spec/helpers/lti_helper_spec.rb b/spec/helpers/lti_helper_spec.rb index 2d3206273e..792fe59987 100644 --- a/spec/helpers/lti_helper_spec.rb +++ b/spec/helpers/lti_helper_spec.rb @@ -2,10 +2,10 @@ let(:scope) { LtiDeployment::LTI_SCOPES[:names_role] } let(:course) { create(:course) } let(:lti_deployment) { create(:lti_deployment, course: course) } + let(:signing_key) { OpenSSL::PKey::RSA.new(2048) } before do - allow(File).to receive(:read).and_call_original - allow(File).to receive(:read).with(LtiClient::KEY_PATH).and_return(OpenSSL::PKey::RSA.new(2048)) + allow(LtiKeyStore).to receive(:current_jwk).and_return(JWT::JWK.new(signing_key)) stub_request(:post, Settings.lti.token_endpoint) .with( body: hash_including( diff --git a/spec/jobs/lti_key_maintenance_job_spec.rb b/spec/jobs/lti_key_maintenance_job_spec.rb new file mode 100644 index 0000000000..c01778effb --- /dev/null +++ b/spec/jobs/lti_key_maintenance_job_spec.rb @@ -0,0 +1,33 @@ +describe LtiKeyMaintenanceJob do + describe '#perform' do + context 'when rotation is disabled' do + before { allow(Settings.lti.rotation).to receive(:enabled).and_return(false) } + + it 'does not rotate' do + expect(LtiKeyStore).not_to receive(:rotate_if_due!) + LtiKeyMaintenanceJob.perform_now + end + + it 'does not prune' do + expect(LtiKeyStore).not_to receive(:prune!) + LtiKeyMaintenanceJob.perform_now + end + end + + context 'when rotation is enabled' do + before { allow(Settings.lti.rotation).to receive(:enabled).and_return(true) } + + it 'rotates when due' do + allow(LtiKeyStore).to receive(:prune!) + expect(LtiKeyStore).to receive(:rotate_if_due!) + LtiKeyMaintenanceJob.perform_now + end + + it 'prunes retired keys' do + allow(LtiKeyStore).to receive(:rotate_if_due!) + expect(LtiKeyStore).to receive(:prune!) + LtiKeyMaintenanceJob.perform_now + end + end + end +end diff --git a/spec/jobs/lti_roster_sync_job_spec.rb b/spec/jobs/lti_roster_sync_job_spec.rb index 85e5844923..e79807900e 100644 --- a/spec/jobs/lti_roster_sync_job_spec.rb +++ b/spec/jobs/lti_roster_sync_job_spec.rb @@ -7,7 +7,7 @@ let(:assessment) { create(:assignment_with_criteria_and_results, course: course) } before do - allow(File).to receive(:read).with(LtiClient::KEY_PATH).and_return(OpenSSL::PKey::RSA.new(2048)) + allow(LtiKeyStore).to receive(:current_jwk).and_return(JWT::JWK.new(OpenSSL::PKey::RSA.new(2048))) stub_request(:post, Settings.lti.token_endpoint) .with( body: hash_including( diff --git a/spec/jobs/lti_sync_job_spec.rb b/spec/jobs/lti_sync_job_spec.rb index 8597c76058..855d79740f 100644 --- a/spec/jobs/lti_sync_job_spec.rb +++ b/spec/jobs/lti_sync_job_spec.rb @@ -7,7 +7,7 @@ let(:assessment) { create(:assignment_with_criteria_and_results, course: course) } before do - allow(File).to receive(:read).with(LtiClient::KEY_PATH).and_return(OpenSSL::PKey::RSA.new(2048)) + allow(LtiKeyStore).to receive(:current_jwk).and_return(JWT::JWK.new(OpenSSL::PKey::RSA.new(2048))) stub_request(:post, Settings.lti.token_endpoint) .with( body: hash_including( diff --git a/spec/lib/lti_key_store_spec.rb b/spec/lib/lti_key_store_spec.rb new file mode 100644 index 0000000000..91bf76a500 --- /dev/null +++ b/spec/lib/lti_key_store_spec.rb @@ -0,0 +1,274 @@ +describe LtiKeyStore do + # Real PEMs in a real temp dir: exercises the actual glob/sort/read logic + # rather than a mock of it. + let(:tmp_dir) { Dir.mktmpdir } + + # Writes a key whose filename encodes +created+, so `created_at` (and + # therefore all the age math) treats it as having been created then. + def write_key(created, key: OpenSSL::PKey::RSA.new(2048)) + path = File.join(tmp_dir, "lti_key_#{created.utc.strftime('%Y%m%dT%H%M%SZ')}.pem") + File.write(path, key.to_pem) + path + end + + before do + allow(Settings.lti.rotation).to receive_messages(key_dir: tmp_dir, max_age_days: 90, overlap_days: 7) + allow(Settings.lti.rotation).to receive(:current_key).and_return(nil) + # File.exist? is called by unrelated machinery (autoloading, etc.), so a + # default call-through is required before stubbing it for a specific path. + allow(File).to receive(:exist?).and_call_original + end + + after { FileUtils.remove_entry(tmp_dir) } + + describe '.key_paths' do + it 'returns an empty array when no keys exist anywhere' do + allow(File).to receive(:exist?).with(LtiClient::KEY_PATH).and_return(false) + expect(LtiKeyStore.key_paths).to be_empty + end + + it 'orders keys newest first' do + old = write_key(30.days.ago) + new = write_key(1.day.ago) + expect(LtiKeyStore.key_paths).to eq([new, old]) + end + + # Existing deployments upgrade with a key.pem and no keys/ directory; this + # fallback is what lets them keep signing until their first rotation. + # Remove only when the legacy path is formally deprecated. + context 'when the rotation directory is empty' do + it 'falls back to the legacy key.pem' do + allow(File).to receive(:exist?).with(LtiClient::KEY_PATH).and_return(true) + expect(LtiKeyStore.key_paths).to eq([LtiClient::KEY_PATH]) + end + end + + context 'when the rotation directory has keys' do + it 'ignores the legacy key.pem' do + allow(File).to receive(:exist?).with(LtiClient::KEY_PATH).and_return(true) + path = write_key(1.day.ago) + expect(LtiKeyStore.key_paths).to eq([path]) + end + end + end + + describe '.current_key' do + it 'raises when no key is available' do + allow(File).to receive(:exist?).with(LtiClient::KEY_PATH).and_return(false) + expect { LtiKeyStore.current_key }.to raise_error(/No LTI signing key/) + end + + it 'signs with the newest key' do + write_key(30.days.ago) + newest = OpenSSL::PKey::RSA.new(2048) + write_key(1.day.ago, key: newest) + expect(LtiKeyStore.current_key.to_pem).to eq(newest.to_pem) + end + + context 'when Settings.lti.rotation.current_key is set' do + it 'signs with the pinned key rather than the newest' do + pinned = OpenSSL::PKey::RSA.new(2048) + pinned_path = write_key(30.days.ago, key: pinned) + write_key(1.day.ago) # newer, but should be ignored + allow(Settings.lti.rotation).to receive(:current_key).and_return(File.basename(pinned_path)) + + expect(LtiKeyStore.current_key.to_pem).to eq(pinned.to_pem) + end + end + + context 'when the pinned key does not exist' do + it 'raises an error naming the missing file' do + write_key(1.day.ago) + allow(Settings.lti.rotation).to receive(:current_key).and_return('lti_key_nonexistent.pem') + + expect { LtiKeyStore.current_key }.to raise_error(/lti_key_nonexistent\.pem/) + end + + it 'does not silently fall back to the newest key' do + newest = OpenSSL::PKey::RSA.new(2048) + write_key(1.day.ago, key: newest) + allow(Settings.lti.rotation).to receive(:current_key).and_return('lti_key_nonexistent.pem') + + expect { LtiKeyStore.current_key }.to raise_error(/Pinned LTI signing key not found/) + end + end + end + + describe '.public_jwks' do + it 'publishes every key in the set' do + write_key(30.days.ago) + write_key(1.day.ago) + + kids = LtiKeyStore.public_jwks[:keys].pluck(:kid) + expect(kids.length).to eq(2) + end + + it 'publishes the current signer' do + write_key(1.day.ago) + kids = LtiKeyStore.public_jwks[:keys].pluck(:kid) + expect(kids).to include(LtiKeyStore.current_jwk.kid) + end + + it 'exports public members only' do + write_key(1.day.ago) + jwk = LtiKeyStore.public_jwks[:keys].first + # 'd' is the RSA private exponent; it must never be published. + expect(jwk.keys.map(&:to_s)).not_to include('d') + end + + it 'produces a key set that verifies a token signed by the current key' do + write_key(1.day.ago) + jwk = LtiKeyStore.current_jwk + token = JWT.encode({ test: 'payload' }, jwk.keypair, 'RS256', { kid: jwk.kid }) + jwks = JSON.parse(LtiKeyStore.public_jwks.to_json) # as Canvas receives it + + expect { JWT.decode(token, nil, true, algorithms: ['RS256'], jwks: jwks) }.not_to raise_error + end + + it 'still verifies a token signed by a retired key during the overlap' do + retired = OpenSSL::PKey::RSA.new(2048) + write_key(30.days.ago, key: retired) + write_key(1.day.ago) # newer key takes over as signer + + retired_jwk = JWT::JWK.new(retired) + token = JWT.encode({ test: 'payload' }, retired_jwk.keypair, 'RS256', { kid: retired_jwk.kid }) + jwks = JSON.parse(LtiKeyStore.public_jwks.to_json) + + expect { JWT.decode(token, nil, true, algorithms: ['RS256'], jwks: jwks) }.not_to raise_error + end + end + + describe '.rotate!' do + it 'creates a new key' do + expect { LtiKeyStore.rotate! }.to change { LtiKeyStore.key_paths.length }.by(1) + end + + it 'makes the new key the current signer' do + write_key(1.day.ago) + path = LtiKeyStore.rotate! + expect(LtiKeyStore.key_paths.first).to eq(path) + end + + it 'writes the key with owner-only permissions' do + path = LtiKeyStore.rotate! + expect(File.stat(path).mode & 0o777).to eq(0o600) + end + + it 'creates the key directory if it does not exist' do + nested = File.join(tmp_dir, 'nested') + allow(Settings.lti.rotation).to receive(:key_dir).and_return(nested) + LtiKeyStore.rotate! + expect(Dir.exist?(nested)).to be true + end + end + + describe '.rotate_if_due!' do + it 'rotates when no key exists' do + allow(File).to receive(:exist?).with(LtiClient::KEY_PATH).and_return(false) + expect { LtiKeyStore.rotate_if_due! }.to change { LtiKeyStore.key_paths.length }.by(1) + end + + it 'rotates when the current key is past its max age' do + write_key(91.days.ago) + expect { LtiKeyStore.rotate_if_due! }.to change { LtiKeyStore.key_paths.length }.by(1) + end + + it 'does not rotate when the current key is within its max age' do + write_key(89.days.ago) + expect { LtiKeyStore.rotate_if_due! }.not_to(change { LtiKeyStore.key_paths.length }) + end + + it 'returns nil when no rotation occurs' do + write_key(1.day.ago) + expect(LtiKeyStore.rotate_if_due!).to be_nil + end + end + + describe '.prune!' do + it 'never prunes the current signer, however old it is' do + write_key(500.days.ago) + expect { LtiKeyStore.prune! }.not_to(change { LtiKeyStore.key_paths.length }) + end + + it 'keeps a key retired less recently than the overlap window' do + # Retired 1 day ago (when its successor was created) -- inside the 7d window. + old = write_key(100.days.ago) + write_key(1.day.ago) + + LtiKeyStore.prune! + expect(LtiKeyStore.key_paths).to include(old) + end + + it 'prunes a key retired longer ago than the overlap window' do + # Retired 30 days ago (when its successor was created) -- past the 7d window. + stale = write_key(60.days.ago) + write_key(30.days.ago) + write_key(1.day.ago) + + LtiKeyStore.prune! + expect(LtiKeyStore.key_paths).not_to include(stale) + end + + it 'retains the most recently retired key while pruning older ones' do + stale = write_key(60.days.ago) + recently_retired = write_key(30.days.ago) + current = write_key(1.day.ago) + + LtiKeyStore.prune! + expect(LtiKeyStore.key_paths).to contain_exactly(current, recently_retired) + expect(LtiKeyStore.key_paths).not_to include(stale) + end + + it 'returns the paths it pruned' do + stale = write_key(60.days.ago) + write_key(30.days.ago) + write_key(1.day.ago) + + expect(LtiKeyStore.prune!).to eq([stale]) + end + + it 'is idempotent' do + write_key(60.days.ago) + write_key(30.days.ago) + write_key(1.day.ago) + + LtiKeyStore.prune! + expect { LtiKeyStore.prune! }.not_to(change { LtiKeyStore.key_paths.length }) + end + + it 'never prunes the pinned key, even when newer keys exist' do + pinned_path = write_key(60.days.ago) + write_key(30.days.ago) + write_key(1.day.ago) + allow(Settings.lti.rotation).to receive(:current_key).and_return(File.basename(pinned_path)) + + LtiKeyStore.prune! + expect(LtiKeyStore.key_paths).to include(pinned_path) + end + + it 'can still sign after pruning when an older key is pinned' do + pinned = OpenSSL::PKey::RSA.new(2048) + pinned_path = write_key(60.days.ago, key: pinned) + write_key(30.days.ago) + write_key(1.day.ago) + allow(Settings.lti.rotation).to receive(:current_key).and_return(File.basename(pinned_path)) + + LtiKeyStore.prune! + expect(LtiKeyStore.current_key.to_pem).to eq(pinned.to_pem) + end + end + + describe '.created_at' do + it 'parses the UTC timestamp from the filename' do + created = 5.days.ago + path = write_key(created) + expect(LtiKeyStore.created_at(path)).to be_within(1.second).of(created) + end + + it 'falls back to mtime for a file without a timestamp in its name' do + path = File.join(tmp_dir, 'key.pem') + File.write(path, OpenSSL::PKey::RSA.new(2048).to_pem) + expect(LtiKeyStore.created_at(path)).to be_within(1.minute).of(Time.now.utc) + end + end +end diff --git a/spec/models/lti_client_spec.rb b/spec/models/lti_client_spec.rb index fb26d729ed..8aadfe20b0 100644 --- a/spec/models/lti_client_spec.rb +++ b/spec/models/lti_client_spec.rb @@ -1,5 +1,7 @@ describe LtiClient do - before { allow(File).to receive(:read).with(LtiClient::KEY_PATH).and_return(OpenSSL::PKey::RSA.new(2048)) } + let(:signing_key) { OpenSSL::PKey::RSA.new(2048) } + + before { allow(LtiKeyStore).to receive(:current_jwk).and_return(JWT::JWK.new(signing_key)) } describe 'uniqueness_validation' do subject { create(:lti_client) }