fixed swedbank notification security issue

Gemfile.lock

@@ -42,6 +42,8 @@ GEM
       coderay (~> 1.1.0)
       method_source (~> 0.8.1)
       slop (~> 3.4)
+    pry-nav (0.2.4)
+      pry (>= 0.9.10, < 0.11.0)
     rake (10.1.1)
     rest-client (1.8.0)
       http-cookie (>= 1.0.2, < 2.0)
@@ -78,10 +80,11 @@ DEPENDENCIES
   dotenv
   priscilla
   pry
+  pry-nav
   rake
   rspec
   sacps!
   timecop
 
 BUNDLED WITH
-   1.10.6
+   1.11.2

lib/sacps/auth/auth.rb

@@ -16,11 +16,12 @@ def func_p(val)
       def generate_data_string(service_msg_number, sigparams, required_service_params)
         str = ''
         required_params = required_service_params[service_msg_number.to_i] || required_service_params[service_msg_number]
-        
+
         required_params.each do |param|
           val = sigparams[param].to_s # nil goes to ''
           str << func_p(val) << val
         end
+        binding.pry
         str
       end
 
@@ -31,7 +32,7 @@ def encode_to_utf8 string
       # Take the posted data and move the relevant data into a hash
       def parse(post)
         @raw = post.to_s
-        
+
         for line in @raw.split('&')
           key, value = *line.scan( %r{^([A-Za-z0-9_.]+)\=(.*)$} ).flatten
           params[key] = CGI.unescape(value)
@@ -49,4 +50,4 @@ def generate_random_string length=6
 
     end
   end
-end
+end

lib/sacps/auth/banklink.rb

@@ -1,16 +1,14 @@
 module SacPS
   module Auth
     module Banklink
-
       def generate_signature(service_msg_number, sigparams, required_service_params)
-        privkey = self.class.parent.get_private_key
-        privkey.sign(OpenSSL::Digest::SHA1.new, generate_data_string(service_msg_number, sigparams, required_service_params))
+        private_key = self.class.parent.get_private_key
+        private_key.sign(OpenSSL::Digest::SHA1.new, generate_data_string(service_msg_number, sigparams, required_service_params))
       end
 
       def generate_mac(service_msg_number, sigparams, required_service_params)
         Base64.encode64(generate_signature(service_msg_number, sigparams, required_service_params)).gsub(/\n/,'')
       end
-
     end
   end
-end
+end

lib/sacps/auth/swedbank.rb

@@ -7,6 +7,7 @@ module Auth
     module Swedbank
       mattr_accessor :public_key
       mattr_accessor :private_key
+      mattr_accessor :bank_public_key
       mattr_accessor :identifier
       mattr_accessor :service_url
       mattr_accessor :return_url
@@ -22,6 +23,11 @@ def self.get_private_key
         OpenSSL::PKey::RSA.new(private_key.gsub(/  /, ''))
       end
 
+      def self.get_bank_public_key
+        cert = self.bank_public_key
+        OpenSSL::X509::Certificate.new(cert.gsub(/  /, '')).public_key
+      end
+
       def self.notification post
         Notification.new post
       end
@@ -50,4 +56,4 @@ def self.helper options={}
       }
     end
   end
-end
+end

lib/sacps/auth/swedbank/notification.rb

@@ -4,7 +4,7 @@ module Swedbank
       class Notification
         include SacPS::Auth::Common
         include SacPS::Auth::Banklink
-        
+
         attr_accessor :params
         attr_accessor :raw
         attr_accessor :signature
@@ -44,8 +44,7 @@ def user_language
         end
 
         def valid?
-          true
-          # bank_signature_valid?(params['VK_SERVICE'], params)
+          bank_signature_valid?(params['VK_SERVICE'], params)
         end
 
         private
@@ -61,9 +60,9 @@ def signature
           def bank_signature_valid? service_msg_number, sigparams
             digest = OpenSSL::Digest::SHA1.new
             data_string = generate_data_string(service_msg_number, sigparams, SacPS::Auth::Swedbank.required_service_params)
-            SacPS::Auth::Swedbank.get_public_key.verify digest, signature, data_string
+            SacPS::Auth::Swedbank.get_bank_public_key.verify digest, signature, data_string
           end
       end
     end
   end
-end
+end

sacps.gemspec

@@ -25,6 +25,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency "dotenv"
   s.add_development_dependency "timecop"
   s.add_development_dependency "pry"
+  s.add_development_dependency "pry-nav"
 
   s.add_dependency 'activesupport'
   s.add_dependency "builder", '>= 2.0'

spec/auth/notifications/sacps_auth_swedbank_notification_spec.rb

@@ -5,8 +5,12 @@
 describe SacPS::Auth::Swedbank::Notification do
   context "validation" do
     before :all do
-      valid_http_raw_data =   "VK_SERVICE=3003&VK_VERSION=008&VK_SND_ID=HP&VK_REC_ID=REFEREND&VK_NONCE=48064728456631139890576881998733355098303532232665&VK_INFO=ISIK:160992-10610;NIMI:KRISTIĀNS PURVIŅŠ&VK_ENCODING=UTF-8&VK_MAC=BdTGaM90+aSDAVDfHFooTFEUlkdqxCWpr+Vxn6SkKj27mJyllqQP9Hxqw+PfYUhUUsDbqHIUN95qPl9t9aabn+caIbdcMq6aHbqNBoc7svhlNv0qBse8NaNIp7xnwjPVDzZA4ExyBvlRMGmrsSNbjc2P9SkdSWDfxgwb3GEPkoE="
+      Mock::Auth::Swedbank.public_key = File.read(File.expand_path("spec/example_files/swedbank_test_cert.pem"))
+      Mock::Auth::Swedbank.private_key = File.read(File.expand_path("spec/example_files/swedbank_test_key.pem"))
+      valid_http_raw_data = Mock::Auth::Swedbank::NotificationGenerator.new.generate_raw_response
       invalid_http_raw_data = "VK_SERVICE=3003&VK_VERSION=008&VK_SND_ID=HP&VK_REC_ID=REFEREND&VK_NONCE=63244166086213189956175700685245263586579875921384&VK_INFO=ISIK:123456-10312;NIMI:MĀRIS CUKURS&VK_ENCODING=UTF-8&VK_MAC=S+Gt91E1I9WEL/qIWOgOUJB0h4WDeBHrfETqC4UG9Xe58YOP30iIRoMJOlBSHvfswVWEuSwCbBVsWIQt7tqHsY1IbidEsJ2ffdkCgofO9qPDBTuS9CI3bbPhK3gMtxkFs5BzxmXEu4bJHvuVJF40g4HyFZe3ozrUem2BHQS6u0Y="
+
+      SacPS::Auth::Swedbank.bank_public_key = File.read(File.expand_path("spec/example_files/swedbank_test_cert.pem"))
       @invalid_http_raw_data = SacPS::Auth::Swedbank.notification invalid_http_raw_data
       @valid_notification = SacPS::Auth::Swedbank.notification valid_http_raw_data
     end
@@ -43,4 +47,19 @@
     end
   end
 
+  private
+
+  # swedbank responds to our authentication request with an encrypted notification,
+  # that is encrypted the same way our authentication request is.
+  # To test this we pretend to be the bank and use a self key pair to sign
+  # and verify the notification
+  # the keys used for test purpases are in spec/expample_files and can be generated
+  # by calling
+  # openssl req  -nodes -new -x509 -keyout swedbank_test_key.pem -out swedbank_test_cert.pem -days 3650
+  def generate_notification
+    service_msg_number =
+    sigparams =
+    requried_service_params =
+    SacPS::Auth::Banklink.generate_signature(service_msg_number, sigparams, required_service_params)
+  end
 end

spec/example_files/swedbank_test_cert.pem

@@ -0,0 +1,17 @@
+-----BEGIN CERTIFICATE-----
+MIICsDCCAhmgAwIBAgIJAJ3net+nsGZhMA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNV
+BAYTAkFVMRMwEQYDVQQIEwpTb21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBX
+aWRnaXRzIFB0eSBMdGQwHhcNMTYwNTEzMDY0MTMyWhcNMjYwNTExMDY0MTMyWjBF
+MQswCQYDVQQGEwJBVTETMBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50
+ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKB
+gQC6ws539UnzhBlgml3XyoAJWPox+BbbPVGhsFlCfitqRJbsFAd0EEQVODaQaHGI
+JgfHUdeG/1n/KmF0qPN2Zio7ULaKPb99UKVjHdEHyD1/r74wh79KYzr0A4wR2JjA
+o6cD3cfXpyM8/YQ6LEhnrLpqoGqZE9y1Bn8cpUjMCt1cuQIDAQABo4GnMIGkMB0G
+A1UdDgQWBBSL2IbcJCA4fz04lPEH5adjutYRFTB1BgNVHSMEbjBsgBSL2IbcJCA4
+fz04lPEH5adjutYRFaFJpEcwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgTClNvbWUt
+U3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZIIJAJ3net+n
+sGZhMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADgYEAax9F0uP2/1nGNS57
+QVZ7dZbgworZ4C/8HM6wqeVUaianMOdms5YWVRgApJOOCvrZQu+EL5XVFkmuh05e
+zTltaUq7oeu89O/+V5YLhuaNfS24Ku7xde8niheSuqPxGSPbEz8FAA4IFlL4cyy2
+hs71mAJJNeAMoYzUQbwcC8Pmm9U=
+-----END CERTIFICATE-----

spec/example_files/swedbank_test_key.pem

@@ -0,0 +1,15 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIICXQIBAAKBgQC6ws539UnzhBlgml3XyoAJWPox+BbbPVGhsFlCfitqRJbsFAd0
+EEQVODaQaHGIJgfHUdeG/1n/KmF0qPN2Zio7ULaKPb99UKVjHdEHyD1/r74wh79K
+Yzr0A4wR2JjAo6cD3cfXpyM8/YQ6LEhnrLpqoGqZE9y1Bn8cpUjMCt1cuQIDAQAB
+AoGAGBvFo16aqUDINdw3eP++/3Xo9kJcUPjdbM3i995ppFIuWVNMkwL2hva2tHWH
+Elg2cNhzxg14hKPn6LNWmlYd5UlwYtHnsDsMSz/VPkPQQL7Eh50i6X91+K0+KuPv
+twEplE+RDI8ceqwJpvMU9ikmLE3CBTARrtzE4x4flIW6Q3ECQQD2MdbiwoitcyhQ
+RWOjlXMZJXY3X/0WJUohZpDZN9yPECoFR5kbY/WfS22kPctmqsFKISc7YbFRlwj+
+Npf+Ed9PAkEAwjL9mtJI63D6481Nckm98RiYX04I13MaO4JWcuSdXc0ISlvOsyzE
+JRTCdn5JihNeqEglKfmIp9d/aIhhKIzBdwJAENsSsk3NW8rBnNVTYBTQX41gDaSF
+yGlfLPA/xI99i1H4/omLYwOyAmkApbkRXwMb3r5sWDV1FXf4xqboOx3wIwJBAK/4
++KlkR8NhG1d1X+piPAXOnbQuyABDQtAN6TwvQRIQiqSm0IS32f0n9JbfFNBTgQDI
+bzS780L0GXWlrzTJZz0CQQDLVVbVo1Jt2q53du5kbmzeWAwHjfs19vshhIB1vR+A
+q87qQ7T/XFW27gFfBItQpch4yDKZDPRupcYVEMgMSKeB
+-----END RSA PRIVATE KEY-----

spec/mocks/auth/swedbank.rb

@@ -0,0 +1,33 @@
+module Mock
+  module Auth
+    module Swedbank
+      include SacPS::Auth::Common
+      include SacPS::Auth::Banklink
+
+      mattr_accessor :public_key
+      mattr_accessor :private_key
+      mattr_accessor :required_service_params
+
+      def self.get_public_key
+        cert = self.public_key
+        OpenSSL::X509::Certificate.new(cert.gsub(/  /, '')).public_key
+      end
+
+      def self.get_private_key
+        private_key = self.private_key
+        OpenSSL::PKey::RSA.new(private_key.gsub(/  /, ''))
+      end
+
+      self.required_service_params = {
+        3003 => [
+          'VK_SERVICE',
+          'VK_VERSION',
+          'VK_SND_ID',
+          'VK_REC_ID',
+          'VK_NONCE',
+          'VK_INFO'
+        ]
+      }
+    end
+  end
+end

spec/mocks/auth/swedbank/notification_generator.rb

@@ -0,0 +1,37 @@
+module Mock
+  module Auth
+    module Swedbank
+      class NotificationGenerator
+        include SacPS::Auth::Common
+        include SacPS::Auth::Banklink
+
+        attr_reader :fields
+        # VK_SERVICE
+        # VK_VERSION
+        # VK_SND_ID
+        # VK_REC_ID
+        # VK_NONCE
+        # VK_INFO
+        # VK_ENCODING
+        # VK_MAC
+        def initialize options={}
+
+          @fields = {}
+
+          @fields['VK_SERVICE']  = '3003'
+          @fields['VK_VERSION']  = '008'
+          @fields['VK_SND_ID']   = 'HP'
+          @fields['VK_REC_ID']   = 'REFEREND'
+          @fields['VK_NONCE']    = generate_random_string 50
+          @fields['VK_INFO']     = "ISIK:150174-12312;NIMI:KRISTIĀNS BĒRZIŅŠ"
+          @fields['VK_ENCODING'] = "UTF-8"
+          @fields['VK_MAC'] = generate_mac(@fields['VK_SERVICE'], @fields, SacPS::Auth::Swedbank.required_service_params)
+        end
+
+        def generate_raw_response
+          values = @fields.collect{|key, value| "#{key}=#{value}"}.join('&')
+        end
+      end
+    end
+  end
+end

spec/mocks/mock.rb

@@ -0,0 +1,2 @@
+require 'auth/swedbank'
+require 'auth/swedbank/notification_generator'

spec/spec_helper.rb

@@ -1,7 +1,9 @@
 require 'bundler/setup'
 Bundler.setup
 
+$LOAD_PATH << File.expand_path("spec/mocks")
 require 'sacps'
+require 'mock'
 require 'priscilla'
 require 'timecop'
 require 'pry'