Commit 1555544

mo <mo@mokhan.ca>
2017-11-17 00:05:24
partially implement HTTP-Redirect binding signature verification.
1 parent f8b962a
lib/saml/kit/binding.rb
@@ -33,13 +33,9 @@ module Saml
       end
 
       def deserialize(params)
-        if params['SAMLRequest'].present?
-          Saml::Kit::Request.deserialize(CGI.unescape(params['SAMLRequest']))
-        elsif params['SAMLResponse'].present?
-          Saml::Kit::Response.deserialize(CGI.unescape(params['SAMLResponse']))
-        else
-          raise ArgumentError.new("SAMLRequest or SAMLResponse parameter is required.")
-        end
+        document = deserialize_document_from!(params)
+        ensure_valid_signature!(params, document)
+        document
       end
 
       def http_redirect?
@@ -53,6 +49,46 @@ module Saml
       def to_h
         { binding: binding, location: location }
       end
+
+      private
+
+      def ensure_valid_signature!(params, document)
+        return if params['Signature'].blank? || params['SigAlg'].blank?
+
+        signature = CGI.unescape(Base64.decode64(params['Signature']))
+        algorithm_uri = params['SigAlg']
+
+        canonical_form = ['SAMLRequest', 'RelayState', 'SigAlg'].map do |key|
+          value = params[key]
+          value.present? ? "#{key}=#{value}" : nil
+        end.compact.join('&')
+
+        valid = document.provider.verify(algorithm_for(algorithm_uri), signature, canonical_form)
+        raise ArgumentError.new("Invalid Signature") unless valid
+      end
+
+      def deserialize_document_from!(params)
+        if params['SAMLRequest'].present?
+          Saml::Kit::Request.deserialize(CGI.unescape(params['SAMLRequest']))
+        elsif params['SAMLResponse'].present?
+          Saml::Kit::Response.deserialize(CGI.unescape(params['SAMLResponse']))
+        else
+          raise ArgumentError.new("SAMLRequest or SAMLResponse parameter is required.")
+        end
+      end
+
+      def algorithm_for(algorithm)
+        case algorithm =~ /(rsa-)?sha(.*?)$/i && $2.to_i
+        when 256
+          OpenSSL::Digest::SHA256.new
+        when 384
+          OpenSSL::Digest::SHA384.new
+        when 512
+          OpenSSL::Digest::SHA512.new
+        else
+          OpenSSL::Digest::SHA1.new
+        end
+      end
     end
   end
 end
lib/saml/kit/logout_response.rb
@@ -49,10 +49,20 @@ module Saml
         Saml::Kit::Content.serialize(to_xml, compress: compress)
       end
 
+      def provider
+        registry.metadata_for(issuer)
+      end
+
       def to_xml
         content
       end
 
+      private
+
+      def registry
+        Saml::Kit.configuration.registry
+      end
+
       class Builder
         attr_accessor :id, :issuer, :version, :status_code, :sign, :now, :destination
         attr_reader :request
lib/saml/kit/metadata.rb
@@ -91,6 +91,14 @@ module Saml
         to_xml
       end
 
+      def verify(algorithm, signature, data)
+        signing_certificates.find do |cert|
+          x509 = OpenSSL::X509::Certificate.new(Base64.decode64(cert[:text]))
+          public_key = x509.public_key
+          public_key.verify(algorithm, signature, data)
+        end
+      end
+
       def self.from(content)
         hash = Hash.from_xml(content)
         entity_descriptor = hash["EntityDescriptor"]
lib/saml/kit/url_builder.rb
@@ -6,7 +6,7 @@ module Saml
       end
 
       def build(saml_document, relay_state: nil)
-        payload = build_payload(saml_document, relay_state)
+        payload = canonicalize(saml_document, relay_state)
         "#{saml_document.destination}?#{payload}&Signature=#{signature_for(payload)}"
       end
 
@@ -18,7 +18,7 @@ module Saml
         Base64.strict_encode64(private_key.sign(OpenSSL::Digest::SHA256.new, payload))
       end
 
-      def build_payload(saml_document, relay_state)
+      def canonicalize(saml_document, relay_state)
         {
           saml_document.query_string_parameter => Content.serialize(saml_document.to_xml, compress: true),
           'RelayState' => relay_state,
spec/saml/binding_spec.rb
@@ -82,6 +82,13 @@ RSpec.describe Saml::Kit::Binding do
   describe "#deserialize" do
     describe "HTTP-Redirect binding" do
       let(:subject) { Saml::Kit::Binding.new(binding: Saml::Kit::Namespaces::HTTP_REDIRECT, location: location) }
+      let(:issuer) { FFaker::Internet.http_url }
+      let(:provider) { Saml::Kit::IdentityProviderMetadata::Builder.new.build }
+
+      before :each do
+        allow(Saml::Kit.configuration.registry).to receive(:metadata_for).with(issuer).and_return(provider)
+        allow(Saml::Kit.configuration).to receive(:issuer).and_return(issuer)
+      end
 
       it 'deserializes the SAMLRequest to an AuthnRequest' do
         url, _ = subject.serialize(Saml::Kit::AuthenticationRequest::Builder.new)
@@ -103,7 +110,7 @@ RSpec.describe Saml::Kit::Binding do
 
       it 'deserializes the SAMLResponse to a Response' do
         user = double(:user, name_id_for: SecureRandom.uuid, assertion_attributes_for: [])
-        request = double(:request, id: SecureRandom.uuid, provider: nil, acs_url: FFaker::Internet.http_url, name_id_format: Saml::Kit::Namespaces::PERSISTENT, issuer: FFaker::Internet.http_url)
+        request = double(:request, id: SecureRandom.uuid, provider: nil, acs_url: FFaker::Internet.http_url, name_id_format: Saml::Kit::Namespaces::PERSISTENT, issuer: issuer)
         url, _ = subject.serialize(Saml::Kit::Response::Builder.new(user, request))
         result = subject.deserialize(query_params_from(url))
         expect(result).to be_instance_of(Saml::Kit::Response)
@@ -111,7 +118,7 @@ RSpec.describe Saml::Kit::Binding do
 
       it 'deserializes the SAMLResponse to a LogoutResponse' do
         user = double(:user, name_id_for: SecureRandom.uuid, assertion_attributes_for: [])
-        request = double(:request, id: SecureRandom.uuid, provider: nil, acs_url: FFaker::Internet.http_url, name_id_format: Saml::Kit::Namespaces::PERSISTENT, issuer: FFaker::Internet.http_url)
+        request = double(:request, id: SecureRandom.uuid, provider: provider, acs_url: FFaker::Internet.http_url, name_id_format: Saml::Kit::Namespaces::PERSISTENT, issuer: FFaker::Internet.http_url)
         url, _ = subject.serialize(Saml::Kit::LogoutResponse::Builder.new(user, request))
         result = subject.deserialize(query_params_from(url))
         expect(result).to be_instance_of(Saml::Kit::LogoutResponse)
@@ -127,6 +134,15 @@ RSpec.describe Saml::Kit::Binding do
           subject.deserialize({ })
         end.to raise_error(ArgumentError)
       end
+
+      it 'raises an error when the signature does not match' do
+        url, _ = subject.serialize(Saml::Kit::AuthenticationRequest::Builder.new)
+        query_params = query_params_from(url)
+        query_params['Signature'] = 'invalid'
+        expect do
+          subject.deserialize(query_params)
+        end.to raise_error(/Invalid Signature/)
+      end
     end
   end
 end