Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions app/controllers/lti_deployments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions app/jobs/lti_key_maintenance_job.rb
Original file line number Diff line number Diff line change
@@ -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
119 changes: 119 additions & 0 deletions app/lib/lti_key_store.rb
Original file line number Diff line number Diff line change
@@ -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_<UTC-timestamp>.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
Comment thread
donny-wong marked this conversation as resolved.
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!
Comment thread
donny-wong marked this conversation as resolved.
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
3 changes: 1 addition & 2 deletions app/models/lti_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
7 changes: 7 additions & 0 deletions config/initializers/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
47 changes: 46 additions & 1 deletion docs/docs/administrators/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,59 @@ 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:

- `lti.course_filter_file` must be the absolute path to a Ruby file that defines a method `LtiConfig::allowed_to_create_course?(lti_deployment)`, which takes an `LtiDeployment` model instance and returns `true` or `false`.
- `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 <default_root_path>/lti/keys
cp <default_root_path>/lti/key.pem \
<default_root_path>/lti/keys/lti_key_$(date -u +%Y%m%dT%H%M%SZ).pem
chmod 600 <default_root_path>/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
Expand Down
19 changes: 12 additions & 7 deletions lib/tasks/lti_key.rake
Original file line number Diff line number Diff line change
@@ -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
37 changes: 34 additions & 3 deletions spec/controllers/lti_deployments_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
4 changes: 2 additions & 2 deletions spec/helpers/lti_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
33 changes: 33 additions & 0 deletions spec/jobs/lti_key_maintenance_job_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Loading