Skip to content
Merged
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
3 changes: 2 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ jobs:
cp -a keys.example keys
cp -a certs.example certs
cp pwned_passwords/pwned_passwords.txt.sample pwned_passwords/pwned_passwords.txt
bundle exec rake db:create db:migrate db:seed --trace
bundle exec rake db:create db:migrate --trace
bundle exec rake db:seed
bundle exec rake assets:precompile
- run:
name: Run Tests
Expand Down
29 changes: 26 additions & 3 deletions app/controllers/concerns/saml_idp_auth_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,35 @@ def saml_response
)
end

def matching_cert
return @matching_cert if defined?(@matching_cert)

@matching_cert = current_service_provider.ssl_certs.find do |ssl_cert|
fingerprint = Fingerprinter.fingerprint_cert(ssl_cert)

saml_request = SamlIdp::Request.from_deflated_request(
params[:SAMLRequest],
get_params: params,
cert: ssl_cert,
)
if saml_request&.service_provider
# Plumb the fingerprint through to the internal service_provider representation
saml_request.service_provider.fingerprint = fingerprint
saml_request.valid_signature?
end
end
end

def encryption_opts
query_params = UriService.params(request.original_url)
if query_params[:skip_encryption].present? && current_service_provider.skip_encryption_allowed
nil
else
current_service_provider.encryption_opts
elsif current_service_provider.encrypt_responses?
{
cert: matching_cert || current_service_provider.certs.first,
block_encryption: current_service_provider.block_encryption,
key_transport: 'rsa-oaep-mgf1p',
}
end
end

Expand All @@ -183,7 +206,7 @@ def current_service_provider
end

def current_issuer
@_issuer ||= saml_request.service_provider.identifier
@_issuer ||= saml_request.service_provider&.identifier
end

def request_url
Expand Down
6 changes: 6 additions & 0 deletions app/controllers/saml_idp_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ def logout
return sign_out_with_flash if raw_saml_request.nil?

decode_request(raw_saml_request)

# Plumb the fingerprint through to the internal service_provider representation
if saml_request && matching_cert
saml_request.service_provider.fingerprint = Fingerprinter.fingerprint_cert(matching_cert)
end

track_logout_event

return head(:bad_request) unless valid_saml_request?
Expand Down
27 changes: 19 additions & 8 deletions app/forms/openid_connect_token_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,25 @@ def validate_code_verifier
def validate_client_assertion
return if identity.blank?

payload, _headers = JWT.decode(client_assertion, service_provider.ssl_cert.public_key, true,
algorithm: 'RS256', iss: client_id,
verify_iss: true, sub: client_id,
verify_sub: true)
validate_aud_claim(payload)
validate_iat(payload)
rescue JWT::DecodeError => err
errors.add(:client_assertion, err.message)
payload, _headers, err = nil

matching_cert = service_provider.ssl_certs.find do |ssl_cert|
err = nil
payload, _headers = JWT.decode(client_assertion, ssl_cert.public_key, true,
algorithm: 'RS256', iss: client_id,
verify_iss: true, sub: client_id,
verify_sub: true)
rescue JWT::DecodeError => err
next
end

if matching_cert && payload
validate_aud_claim(payload)
validate_iat(payload)
else
errors.add(:client_assertion,
err&.message || t('openid_connect.token.errors.invalid_signature'))
end
end

def validate_aud_claim(payload)
Expand Down
32 changes: 23 additions & 9 deletions app/forms/security_event_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,17 +95,31 @@ def check_public_key_error(public_key)
end

def validate_jwt
public_key = service_provider&.ssl_cert&.public_key
return if check_jwt_parse_error
return if check_public_key_error(public_key)

JWT.decode(body, public_key, true, algorithm: 'RS256', leeway: Float::INFINITY)
rescue JWT::IncorrectAlgorithm
@error_code = ErrorCodes::JWT_CRYPTO
errors.add(:jwt, t('risc.security_event.errors.alg_unsupported', expected_alg: 'RS256'))
rescue JWT::VerificationError => err
@error_code = ErrorCodes::JWS
errors.add(:jwt, err.message)
error_code = nil
error_message = nil

matching_public_key = service_provider&.ssl_certs&.find do |ssl_cert|
error_code = nil
error_message = nil
JWT.decode(body, ssl_cert.public_key, true, algorithm: 'RS256', leeway: Float::INFINITY)
rescue JWT::IncorrectAlgorithm
error_code = ErrorCodes::JWT_CRYPTO
error_message = t('risc.security_event.errors.alg_unsupported', expected_alg: 'RS256')
nil
rescue JWT::VerificationError => err
error_code = ErrorCodes::JWS
error_message = err.message
nil
end

if error_code && error_message
@error_code = error_code
errors.add(:jwt, error_message)
else
check_public_key_error(matching_public_key)
end
end

def validate_jti
Expand Down
8 changes: 5 additions & 3 deletions app/models/null_service_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class NullServiceProvider
attribute_bundle
block_encryption
cert
certs
created_at
default_aal
description
Expand All @@ -40,7 +41,6 @@ class NullServiceProvider
signature
signed_response_message_requested
sp_initiated_login_url
ssl_cert
updated_at
].freeze

Expand Down Expand Up @@ -85,13 +85,15 @@ def encrypt_responses?
false
end

def encryption_opts; end

def skip_encryption_allowed
false
end

def allow_prompt_login
false
end

def ssl_certs
[]
end
end
23 changes: 5 additions & 18 deletions app/models/service_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
require 'identity_validations'

class ServiceProvider < ApplicationRecord
self.ignored_columns = %w[deal_id agency aal]
self.ignored_columns = %w[deal_id agency aal fingerprint]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a very old column and adding it here makes sure we don't reference it at all

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a very old column and adding it here makes sure we don't reference it at all

Appears we may still be referencing it during local development make setup:

end.update!(config.except('agency',

fingerprint: '08:79:F5:B1:B8:CC:EC:8F:5C:2A:58:03:30:14:C9:E6:F1:67:78:F1:97:E8:3A:88:EB:8E:70:92:25:D2:2F:32'

This is preventing make setup from succeeding:

ActiveModel::UnknownAttributeError: unknown attribute 'fingerprint' for ServiceProvider.
/Users/Documents/Code/identity-idp/app/services/service_provider_seeder.rb:20:in `block in run'
/Users/Documents/Code/identity-idp/app/services/service_provider_seeder.rb:13:in `each'
/Users/Documents/Code/identity-idp/app/services/service_provider_seeder.rb:13:in `run'
/Users/Documents/Code/identity-idp/db/seeds.rb:2:in `<main>'

We do want to remove that configuration value from the second file?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For posterity: #4877

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should fingerprint be removed from COLUMNS in NullServiceProvider? Currently it behaves differently between ServiceProvider and NullServiceProvider:

[3] pry(main)> ServiceProvider.new.fingerprint
NoMethodError: undefined method `fingerprint' for #<ServiceProvider:0x00007fad4c15e810>
from /Users/andrewmduthie/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activemodel-6.1.3.1/lib/active_model/attribute_methods.rb:469:in `method_missing'
[7] pry(main)> NullServiceProvider.new(issuer:nil).fingerprint
=> nil

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for these catches... will make a PR to remedy

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


belongs_to :agency

Expand All @@ -27,33 +27,20 @@ def self.from_issuer(issuer)
end

def metadata
attributes.symbolize_keys.merge(fingerprint: fingerprint)
attributes.symbolize_keys
end

def ssl_cert
@ssl_cert ||= begin
return if cert.blank?
# @return [Array<OpenSSL::X509::Certificate>]
def ssl_certs
@ssl_certs ||= (certs.presence || Array(cert)).map do |cert|
OpenSSL::X509::Certificate.new(load_cert(cert))
end
end

def fingerprint
@_fingerprint ||= super || Fingerprinter.fingerprint_cert(ssl_cert)
end

def encrypt_responses?
block_encryption != 'none'
end

def encryption_opts
return nil unless encrypt_responses?
{
cert: ssl_cert,
block_encryption: block_encryption,
key_transport: 'rsa-oaep-mgf1p',
}
end

def skip_encryption_allowed
config = AppConfig.env.skip_encryption_allowed_list
return false if config.blank?
Expand Down
17 changes: 0 additions & 17 deletions app/services/service_provider_config.rb

This file was deleted.

20 changes: 20 additions & 0 deletions certs.example/sp/saml_test_sp2.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
-----BEGIN CERTIFICATE-----
MIIDNjCCAh4CCQDN39Nwta1XWzANBgkqhkiG9w0BAQsFADBdMQswCQYDVQQGEwJV
UzEdMBsGA1UECAwURGlzdHJpY3Qgb2YgQ29sdW1iaWExEzARBgNVBAcMCldhc2hp
bmd0b24xDDAKBgNVBAoMA0dTQTEMMAoGA1UECwwDVFRTMB4XDTIxMDMyOTE2Mzk1
M1oXDTIyMDMyOTE2Mzk1M1owXTELMAkGA1UEBhMCVVMxHTAbBgNVBAgMFERpc3Ry
aWN0IG9mIENvbHVtYmlhMRMwEQYDVQQHDApXYXNoaW5ndG9uMQwwCgYDVQQKDANH
U0ExDDAKBgNVBAsMA1RUUzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB
AMo28K48AyMI67bTrkXTk+THgyDLTj0aUVjmig8bTAlSZW8GLdG6I2BWlC3yCVL9
cDx8ENWxtfG/1BQwT/+pf2f23iDzTPYR33z4Q1QZmuqZt39LGP2k3Ew0euzptQzR
anKCzNo2FbO33LnXzktlVElv8YXR0rHNsAH0+sCH/sSn/dQ8cvqyIWzKAyVeZNVX
qzyrYmLde0tiefWXrCRAZ+gbn0Vgcsd6082FaFvTRLmOWGBJaYD3SZXOSIyBgv8k
FAd6ey69M5Qg2cGiiHYF+2rxv8MP5ddA2JIyxmdxajCDJZk7zJiUUWn1Lz0ravgI
eEW1fF0w9Ss1cPvUOCCWNE8CAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAw/T9Cm3n
lhuO3h8hsTJVIZqiwQw7m/wI2rY9c/on0gwnsnxsvWL3Lvl5ZyqTKqFgXWoV90ZK
PoY/Lv9c5RaPx90kV4ZwIJwzzgFVdR0gwsW6OpUXKZzt3LFNruWC+4KgmREsvRyK
8ATfC5I5wVMxKf93YWEX3MBiHEh2VaTbn3cSukNVqUQsNAwYPrl3Fs3lXE6GbJAI
OScEZWlLdC0/uSxrDj1WA4R/8NFMWZOlG6ImlkBqGIBIyByuucXn7ZzlUp453+LP
KasY2FT+qJtytP05bULKIZHXLftV1CXktvt4dPTHvxwqDjLNiylfHFs/O76UE8ox
tz6PXT2P0wJ0Cw==
-----END CERTIFICATE-----
4 changes: 1 addition & 3 deletions config/initializers/saml_idp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@

# Find ServiceProvider metadata_url and fingerprint based on our settings
config.service_provider.finder = lambda do |issuer_or_entity_id|
sp_config = ServiceProviderConfig.new(issuer: issuer_or_entity_id)
sp = sp_config.service_provider
sp_config.sp_attributes.merge(fingerprint: sp.fingerprint, cert: sp.ssl_cert)
ServiceProvider.from_issuer(issuer_or_entity_id).metadata
end
end
2 changes: 2 additions & 0 deletions config/locales/openid_connect/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ en:
invalid_code_verifier: code_verifier did not match code_challenge
invalid_iat: iat must be an integer or floating point Unix timestamp representing
a time in the past
invalid_signature: Could not validate assertion against any registered public
keys
user_info:
errors:
malformed_authorization: Malformed Authorization header
Expand Down
2 changes: 2 additions & 0 deletions config/locales/openid_connect/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ es:
invalid_code_verifier: code_verifier no coincide con code_challenge
invalid_iat: iat debe ser una marca de tiempo Unix de punto flotante o entero
que represente un tiempo en el pasado
invalid_signature: No se pudo validar la aserción contra ninguna clave pública
registrada
user_info:
errors:
malformed_authorization: Título de autorización mal formado
Expand Down
2 changes: 2 additions & 0 deletions config/locales/openid_connect/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ fr:
invalid_code_verifier: code_verifier ne correspondait pas à code_challenge
invalid_iat: iat doit être un horodatage Unix entier ou à virgule flottante
représentant une heure dans le passé
invalid_signature: Impossible de valider l'assertion contre les clés publiques
enregistrées
user_info:
errors:
malformed_authorization: Forme de l'en-tête d'autorisation non valide
Expand Down
4 changes: 3 additions & 1 deletion config/service_providers.localdev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ test:
assertion_consumer_logout_service_url: 'http://localhost:3000/test/saml/decode_slo_request'
sp_initiated_login_url: 'http://localhost:3000/test/saml'
block_encryption: 'none'
cert: 'saml_test_sp'
certs:
- 'saml_test_sp'
- 'saml_test_sp2'
agency: 'Test Government Agency'
agency_id: 1
uuid_priority: 10
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddMultipleCertsToServiceProviders < ActiveRecord::Migration[6.1]
def change
add_column :service_providers, :certs, :string, array: true
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2021_03_16_082419) do
ActiveRecord::Schema.define(version: 2021_03_29_162528) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -572,6 +572,7 @@
t.date "iaa_end_date"
t.string "app_id"
t.integer "default_aal"
t.string "certs", array: true
t.index ["issuer"], name: "index_service_providers_on_issuer", unique: true
end

Expand Down
28 changes: 28 additions & 0 deletions keys.example/saml_test_sp2.key
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDKNvCuPAMjCOu2
065F05Pkx4Mgy049GlFY5ooPG0wJUmVvBi3RuiNgVpQt8glS/XA8fBDVsbXxv9QU
ME//qX9n9t4g80z2Ed98+ENUGZrqmbd/Sxj9pNxMNHrs6bUM0WpygszaNhWzt9y5
185LZVRJb/GF0dKxzbAB9PrAh/7Ep/3UPHL6siFsygMlXmTVV6s8q2Ji3XtLYnn1
l6wkQGfoG59FYHLHetPNhWhb00S5jlhgSWmA90mVzkiMgYL/JBQHensuvTOUINnB
ooh2Bftq8b/DD+XXQNiSMsZncWowgyWZO8yYlFFp9S89K2r4CHhFtXxdMPUrNXD7
1DggljRPAgMBAAECggEAHLw77XaHt5XP8TYZgMC1NoCHiMR7RMGVp71zBvyJDJYR
5foJztDVsB39hp3rZ0iuh1nWBpfvVAA/gfLvm1QZz8tL+4C3ggw+JwMchjnxQr8/
TS59yaWAzK90fHAlk0G7D7S4qZWf9d791cbuANbQaHMo7ixH9Y5WIaEPdQaeVJGN
98hDb/HnwprqUiIT6qONkECUTB5DkxfFO9YpD4GbI8lnYc7iou/T4lCCEb+OGfSt
Wqy1EDgBRZkZu122xNWRXHbjh4vtRY5DeL9kY8aNPHCqve7T/XSQT35cUScQczwX
C8Ds8qN/eXIUdoHBRlA7LHDOZOjvmRb/U6c9YUwfOQKBgQDzYN3lm/LW7p47abB7
CUysj8+Y9QnG38BMxRkDZN/T5O1swDZbXtr9QK9gAF3ugLKae2AKL2EiZMB1scLK
M4E4XNjMJVrK4UH77Xon7Dk6r7y8N2VjhPDHjllEtYvjbopcl5JKptkFYsVO4vcA
m+OGsj2nd8QljUtwv0geD5aFLQKBgQDUs5N2IvpYI/2E6Eg7awX1eNGn7cw8gYhF
/qzCmIuUcQJnUdYMCIoZghPJ5Xz7lCBKGmcHfr+Jh1uGQkEYjNfgwEIUPLhl2qi9
scX9/NApPhwas6xvdOfPJq/BwaMR+oAvg+c6NhxUAFPAVvSCxI8eQPnNNQzOrnkm
PDOrqHJE6wKBgHJ/b+VFqMlVGTv6TPyVM207ev8KyL63JVD4qPvfyS121fwDsY7q
4Tuj4t3XTlmWUnA6+sPP5nK305OLPYjDElfh1ly0djJcJx7Oalm92G6znqctqJVZ
Ra2cWoLophcpOg61gC1+sTrHbOvf+zReInyL/lV7EtxXzNYOJ299BeNBAoGAOfSI
LHtRXSzJSiqEa/Q4Vm9KKQiJSr88o13GMuuftJ2qOv64ZOT6xAKGY8+s41u0BJz3
D7rAc7e2/3kUBZ1ywOGB38O/trkCm1VSDmeRTHuI6tmkFWZ0NyRiZVfel+p6fPfi
zCCsTVMdft3yl6L5IBQyPHDFAZfWmM10gsROBmsCgYEArmE3weGh7SBveHl1nmUi
klY5edpP76N+qcRSB30ydNw2i7fHf09LzMu+/m8RmQeipaURNnF6ToE4cbQC1x/1
sgYZNiXWR4gsdH2LjE8XWV/s9Cwf+AhFEVbvujJ9sb5xMjVol8HjV3rYog+4ID0u
gVcJW1KC4o9vRNAQPMr5kfc=
-----END PRIVATE KEY-----
Loading