Dmm/add invalid sig errors

lib/saml_idp.rb

@@ -75,8 +75,9 @@ def options_have_signature(options)
       private :options_have_signature
 
       def valid_signature?(fingerprint, options = {})
+        soft_fail = options[:soft].nil? ? true : options[:soft]
         (signed? || options_have_signature(options)) &&
-          signed_document.validate(fingerprint, :soft, options)
+          signed_document.validate(fingerprint, soft_fail, options)
       end
 
       def signed_document

lib/saml_idp/request.rb

@@ -141,7 +141,8 @@ def valid?
       end
 
       unless authn_request? || logout_request?
-        log "One and only one of authnrequest and logout request is required. authnrequest: #{authn_request?} logout_request: #{logout_request?} "
+        log 'One and only one of authnrequest and logout request is required. '
+          + "authnrequest: #{authn_request?} logout_request: #{logout_request?} "
         errors.push(:no_auth_or_logout_request)
       end
 
@@ -150,10 +151,15 @@ def valid?
         errors.push(:no_response_url)
       end
 
-      unless service_provider? && valid_signature?
-        log "Signature is invalid in #{raw_xml}"
-        # TODO: We should get more specific errors
-        errors.push(:invalid_signature)
+      if service_provider?
+        begin
+          unless valid_signature?
+            log "Signature is invalid in #{raw_xml}"
+            errors.push(:invalid_signature)
+          end
+        rescue SamlIdp::XMLSecurity::SignedDocument::ValidationError => e
+          errors.push(e.error_code)
+        end
       end
 
       errors.blank?
@@ -166,7 +172,7 @@ def signed?
     def valid_signature?
       # Force signatures for logout requests because there is no other
       # protection against a cross-site DoS.
-      service_provider.valid_signature?(document, logout_request?, options)
+      service_provider.valid_signature?(document, logout_request?, options.merge(soft: false))
     end
 
     def service_provider?
@@ -262,5 +268,6 @@ def service_provider_finder
       config.service_provider.finder
     end
     private :service_provider_finder
+
   end
 end

lib/saml_idp/service_provider.rb

@@ -21,13 +21,19 @@ def valid?
     end
 
     def valid_signature?(doc, require_signature = false, options = {})
-      Array(certs).any? do |cert|
-        if doc.valid_signature?(fingerprint_cert(cert), options.merge(cert: cert))
-          @matching_cert = cert
-        end
+      @matching_cert = Array(certs).find do |cert|
+        doc.valid_signature?(fingerprint_cert(cert), options.merge(cert: cert))
       end
 
+      no_raising_errors = options[:soft].nil? ? true : options[:soft]
+
       if require_signature || should_validate_signature?
+        if certs.blank? && !no_raising_errors
+          raise SamlIdp::XMLSecurity::SignedDocument::ValidationError.new(
+            'No certificate registered',
+            :no_cert_registered
+          )
+        end
         !!@matching_cert
       else
         true

lib/saml_idp/xml_security.rb

@@ -32,7 +32,15 @@
 module SamlIdp
   module XMLSecurity
     class SignedDocument < REXML::Document
-      ValidationError = Class.new(StandardError)
+      class ValidationError < StandardError
+        attr_reader :error_code
+
+        def initialize(msg=nil, error_code=nil)
+          @error_code = error_code
+          super(msg)
+        end
+      end
+
       C14N = "http://www.w3.org/2001/10/xml-exc-c14n#"
       DSIG = "http://www.w3.org/2000/09/xmldsig#"
 
@@ -51,7 +59,10 @@ def validate(idp_cert_fingerprint, soft = true, options = {})
           begin
             OpenSSL::X509::Certificate.new(cert_text)
           rescue OpenSSL::X509::CertificateError => e
-            return soft ? false : (raise ValidationError.new("Invalid certificate"))
+            return soft ? false : (raise ValidationError.new(
+              'Invalid certificate',
+              :invalid_certificate
+            ))
           end
 
         # check cert matches registered idp cert
@@ -60,7 +71,10 @@ def validate(idp_cert_fingerprint, soft = true, options = {})
         plain_idp_cert_fingerprint = idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/,"").downcase
 
         if fingerprint != plain_idp_cert_fingerprint && sha1_fingerprint != plain_idp_cert_fingerprint
-          return soft ? false : (raise ValidationError.new("Fingerprint mismatch"))
+          return soft ? false : (raise ValidationError.new(
+            'Fingerprint mismatch',
+            :fingerprint_mismatch
+          ))
         end
 
         validate_doc(base64_cert, soft, options)
@@ -100,17 +114,25 @@ def find_base64_cert(options)
         if cert_element
           return cert_element.text unless cert_element.text.blank?
 
-          raise ValidationError.new("Certificate element present in response (ds:X509Certificate) but evaluating to nil")
+          raise ValidationError.new(
+            "Certificate element present in response (ds:X509Certificate) but evaluating to nil",
+            :present_but_nil
+          )
         elsif options[:cert]
           if options[:cert].is_a?(String)
             options[:cert]
           elsif options[:cert].is_a?(OpenSSL::X509::Certificate)
             Base64.encode64(options[:cert].to_pem)
           else
-            raise ValidationError.new("options[:cert] must be Base64-encoded String or OpenSSL::X509::Certificate")
+            raise ValidationError.new(
+              'options[:cert] must be Base64-encoded String or OpenSSL::X509::Certificate',
+              :not_base64_or_cert
+            )
           end
         else
-          raise ValidationError.new("Certificate element missing in response (ds:X509Certificate) and not provided in options[:cert]")
+          raise ValidationError.new(
+            'Certificate element missing in response (ds:X509Certificate) and not provided in options[:cert]',
+            :cert_missing)
         end
       end
 
@@ -178,7 +200,11 @@ def validate_doc_embedded_signature(base64_cert, soft = true)
           digest_value                  = Base64.decode64(REXML::XPath.first(ref, "//ds:DigestValue", sig_namespace_hash).text)
 
           unless digests_match?(hash, digest_value)
-            return soft ? false : (raise ValidationError.new("Digest mismatch"))
+            return soft ? false : (raise ValidationError.new(
+              "Digest mismatch",
+              :digest_mismatch
+            ))
+
           end
         end
 
@@ -196,7 +222,10 @@ def verify_signature(base64_cert, sig_alg, signature, canon_string, soft)
         signature_algorithm = algorithm(sig_alg)
 
         unless cert.public_key.verify(signature_algorithm.new, signature, canon_string)
-          return soft ? false : (raise ValidationError.new("Key validation error"))
+          return soft ? false : (raise ValidationError.new(
+            'Key validation error',
+            :key_validation_error
+          ))
         end
 
         return true