Add analytics tracking for signature algorithm errors (#11552)

* changelog: Internal, Analytics, Add tracking for sha256 change

Gemfile

@@ -74,7 +74,7 @@ gem 'rqrcode'
 gem 'ruby-progressbar'
 gem 'ruby-saml'
 gem 'safe_target_blank', '>= 1.0.2'
-gem 'saml_idp', github: '18F/saml_idp', tag: '0.23.0-18f'
+gem 'saml_idp', github: '18F/saml_idp', tag: '0.23.3-18f'
 gem 'scrypt'
 gem 'simple_form', '>= 5.0.2'
 gem 'stringex', require: false

Gemfile.lock

@@ -36,10 +36,10 @@ GIT
 
 GIT
   remote: https://github.com/18F/saml_idp.git
-  revision: 23b69117593e9b9217910af1dd627febd8d18cf4
-  tag: 0.23.0-18f
+  revision: 752085a6f88cd3ce75ecc7a64afe064a0e4f9e35
+  tag: 0.23.3-18f
   specs:
-    saml_idp (0.23.0.pre.18f)
+    saml_idp (0.23.3.pre.18f)
       activesupport
       builder
       faraday

app/controllers/saml_idp_controller.rb

@@ -136,6 +136,13 @@ def capture_analytics
 
     if result.success? && saml_request.signed?
       analytics_payload[:cert_error_details] = saml_request.cert_errors
+
+      # analytics to determine if turning on SHA256 validation will break
+      # existing partners
+      if certs_different?
+        analytics_payload[:certs_different] = true
+        analytics_payload[:sha256_matching_cert] = sha256_alg_matching_cert_serial
+      end
     end
 
     analytics.saml_auth(**analytics_payload)
@@ -147,6 +154,21 @@ def matching_cert_serial
     nil
   end
 
+  def sha256_alg_matching_cert
+    # if sha256_alg_matching_cert is nil, fallback to the "first" cert
+    saml_request.sha256_validation_matching_cert ||
+      saml_request_service_provider&.ssl_certs&.first
+  rescue SamlIdp::XMLSecurity::SignedDocument::ValidationError
+  end
+
+  def sha256_alg_matching_cert_serial
+    sha256_alg_matching_cert&.serial&.to_s
+  end
+
+  def certs_different?
+    encryption_cert != sha256_alg_matching_cert
+  end
+
   def log_external_saml_auth_request
     return unless external_saml_request?
 

app/services/analytics_events.rb

@@ -6446,8 +6446,12 @@ def rules_of_use_visit
   # @param [Boolean] request_signed
   # @param [String] matching_cert_serial
   # matches the request certificate in a successful, signed request
+  # @param [Boolean] certs_different Whether the matching cert changes when SHA256 validations
+  # are turned on in the saml_idp gem
   # @param [Hash] cert_error_details Details for errors that occurred because of an invalid
   # signature
+  # @param [String] sha256_matching_cert serial of the cert that matches when sha256 validations
+  # are turned on
   # @param [String] unknown_authn_contexts space separated list of unknown contexts
   def saml_auth(
     success:,
@@ -6465,6 +6469,8 @@ def saml_auth(
     matching_cert_serial:,
     error_details: nil,
     cert_error_details: nil,
+    certs_different: nil,
+    sha256_matching_cert: nil,
     unknown_authn_contexts: nil,
     **extra
   )
@@ -6485,6 +6491,8 @@ def saml_auth(
       request_signed:,
       matching_cert_serial:,
       cert_error_details:,
+      certs_different:,
+      sha256_matching_cert:,
       unknown_authn_contexts:,
       **extra,
     )

spec/controllers/saml_idp_controller_spec.rb

@@ -1525,6 +1525,93 @@ def name_id_version(format_urn)
             )
           end
 
+          context 'when request is using SHA1 as the signature method algorithm' do
+            let(:auth_settings) do
+              saml_settings(
+                overrides: {
+                  security: {
+                    authn_requests_signed:,
+                    signature_method: 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha1',
+                  },
+                },
+              )
+            end
+
+            context 'when the certificate matches' do
+              it 'does not note that certs are different in the event' do
+                user.identities.last.update!(verified_attributes: ['email'])
+                generate_saml_response(user, auth_settings)
+
+                expect(response.status).to eq(200)
+                expect(@analytics).to have_logged_event(
+                  'SAML Auth', hash_not_including(
+                    certs_different: true,
+                    sha256_matching_cert: matching_cert_serial,
+                  )
+                )
+              end
+            end
+
+            context 'when the certificate does not match' do
+              let(:wrong_cert) do
+                OpenSSL::X509::Certificate.new(
+                  Rails.root.join('certs', 'sp', 'saml_test_sp2.crt').read,
+                )
+              end
+
+              before do
+                service_provider.update!(certs: [wrong_cert, saml_test_sp_cert])
+              end
+
+              it 'notes that certs are different in the event' do
+                user.identities.last.update!(verified_attributes: ['email'])
+                generate_saml_response(user, auth_settings)
+
+                expect(response.status).to eq(200)
+                expect(@analytics).to have_logged_event(
+                  'SAML Auth', hash_including(
+                    certs_different: true,
+                    sha256_matching_cert: wrong_cert.serial.to_s,
+                  )
+                )
+              end
+            end
+          end
+
+          context 'when request is using SHA1 as the digest method algorithm' do
+            let(:auth_settings) do
+              saml_settings(
+                overrides: {
+                  security: {
+                    authn_requests_signed:,
+                    digest_method: 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha1',
+                  },
+                },
+              )
+            end
+
+            it 'notes an error in the event' do
+              user.identities.last.update!(verified_attributes: ['email'])
+              generate_saml_response(user, auth_settings)
+
+              expect(response.status).to eq(200)
+              expect(@analytics).to have_logged_event(
+                'SAML Auth', hash_including(
+                  request_signed: authn_requests_signed,
+                  cert_error_details: [
+                    {
+                      cert: '16692258094164984098',
+                      error_code: :fingerprint_mismatch,
+                    },
+                    {
+                      cert: '14834808178619537243', error_code: :fingerprint_mismatch
+                    },
+                  ],
+                )
+              )
+            end
+          end
+
           context 'Certificate sig validation fails because of namespace bug' do
             let(:request_sp) { double }