Fixes #25001 - CVE-2018-14643 - ensure auth (#54)

Author: iNecas

lib/smart_proxy_dynflow/api.rb

@@ -6,18 +6,53 @@ module Proxy
   class Dynflow
     class Api < ::Sinatra::Base
       helpers ::Proxy::Helpers
+      helpers ::Proxy::Log
       helpers ::Proxy::Dynflow::Helpers
 
       before do
-        logger = Proxy::LogBuffer::Decorator.instance
         content_type :json
         if request.env['HTTP_AUTHORIZATION'] && request.env['PATH_INFO'].end_with?('/done')
           # Halt running before callbacks if a token is provided and the request is notifying about task being done
           return
+        else
+          do_authorize_with_ssl_client
+          do_authorize_with_trusted_hosts
         end
       end
 
-      helpers Sinatra::Authorization
+
+      # TODO: move this to foreman-proxy to reduce code duplicities
+      def do_authorize_with_trusted_hosts
+        # When :trusted_hosts is given, we check the client against the list
+        # HTTPS: test the certificate CN
+        # HTTP: test the reverse DNS entry of the remote IP
+        trusted_hosts = Proxy::SETTINGS.trusted_hosts
+        if trusted_hosts
+          if [ 'yes', 'on', 1 ].include? request.env['HTTPS'].to_s
+            fqdn = https_cert_cn
+            source = 'SSL_CLIENT_CERT'
+          else
+            fqdn = remote_fqdn(Proxy::SETTINGS.forward_verify)
+            source = 'REMOTE_ADDR'
+          end
+          fqdn = fqdn.downcase
+          logger.debug "verifying remote client #{fqdn} (based on #{source}) against trusted_hosts #{trusted_hosts}"
+
+          unless Proxy::SETTINGS.trusted_hosts.include?(fqdn)
+            log_halt 403, "Untrusted client #{fqdn} attempted to access #{request.path_info}. Check :trusted_hosts: in settings.yml"
+          end
+        end
+      end
+
+      def do_authorize_with_ssl_client
+        if ['yes', 'on', '1'].include? request.env['HTTPS'].to_s
+          if request.env['SSL_CLIENT_CERT'].to_s.empty?
+            log_halt 403, "No client SSL certificate supplied"
+          end
+        else
+          logger.debug('require_ssl_client_verification: skipping, non-HTTPS request')
+        end
+      end
 
       post "/*" do
         relay_request

test/api_test/api_test.rb

@@ -14,27 +14,34 @@ def hostname
       'somehost.somedomain.org:9000'
     end
 
-    def request_factory(kind, path)
+    def request_factory(kind, path, env = {})
       body = mock()
       body.stubs(:read).returns("")
-      env = {
+      env = env.merge(
         'REQUEST_METHOD' => kind,
         'rack.request.query_hash' => {},
-        'HTTP_HOST' => hostname
-      }
-      OpenStruct.new(:env => env, :body => body, :path => '/dynflow' + path)
+        'HTTP_HOST' => hostname,
+        'PATH_INFO' => "/dynflow#{path}"
+      )
+      Sinatra::Request.new(env).tap do |r|
+        r.stubs(:body).returns(body)
+      end
     end
 
     let(:new_request) { Net::HTTP::Get.new 'example.org' }
 
-    it 'relays GET requests' do
+    def mock_core_service(method, path, response)
       factory = mock()
-      factory.expects(:create_get).with('/tasks/count', {}).returns(new_request)
+      factory.expects(method).with { |p| p == path }.returns(new_request)
       Proxy::Dynflow::Callback::Core.any_instance.expects(:request_factory).returns(factory)
       Proxy::Dynflow::Callback::Core.any_instance
-                                    .expects(:send_request).with(new_request)
-                                    .returns(OpenStruct.new(:code => 200, :body => {'count' => 0}))
-      Sinatra::Base.any_instance.expects(:request).times(4).returns(request_factory('GET', '/tasks/count'))
+        .expects(:send_request).with(new_request)
+        .returns(OpenStruct.new(response))
+    end
+
+    it 'relays GET requests' do
+      mock_core_service(:create_get, '/tasks/count', :code => 200, :body => {'count' => 0})
+      Proxy::Dynflow::Api.any_instance.stubs(:request).returns(request_factory('GET', '/tasks/count'))
       get '/tasks/count'
       new_request['X-Forwarded-For'].must_equal hostname
     end
@@ -46,9 +53,64 @@ def request_factory(kind, path)
       Proxy::Dynflow::Callback::Core.any_instance
                                     .expects(:send_request).with(new_request)
                                     .returns(OpenStruct.new(:code => 200, :body => {'count' => 0}))
-      Sinatra::Base.any_instance.expects(:request).times(4).returns(request_factory('POST', '/tasks/12345/cancel'))
+      Proxy::Dynflow::Api.any_instance.stubs(:request).returns(request_factory('POST', '/tasks/12345/cancel'))
       post '/tasks/12345/cancel', {}
       new_request['X-Forwarded-For'].must_equal hostname
     end
+
+    it 'refuses unauthorized http connections (using remote_fqdn)' do
+      Proxy::Dynflow::Api.any_instance.stubs(:request).returns(request_factory('POST', '/tasks'))
+      Proxy::Dynflow::Api.any_instance.stubs(:remote_fqdn).returns('unauthorized_host.example.com')
+      Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"])
+      post '/tasks'
+      assert(last_response.forbidden?, 'The request should be forbidden')
+    end
+
+    it 'accepts authorized http connections (using remote_fqdn)' do
+      mock_core_service(:create_post, '/tasks', :code => 200, :body => {})
+      Proxy::Dynflow::Api.any_instance.stubs(:request).returns(request_factory('POST', '/tasks'))
+      Proxy::Dynflow::Api.any_instance.stubs(:remote_fqdn).returns('mytrustedhost.example.com')
+      Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"])
+      post '/tasks'
+      assert(last_response.ok?, 'The response should be ok')
+    end
+
+    it 'refuses unauthorized https connections (using https_cert_cn)' do
+      Proxy::Dynflow::Api.any_instance.stubs(:request).
+        returns(request_factory('POST', '/tasks', 'HTTPS' => 'yes','SSL_CLIENT_CERT' => 'mytrustedcert'))
+      Proxy::Dynflow::Api.any_instance.stubs(:https_cert_cn).returns('unauthorized_host.example.com')
+      Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"])
+      post '/tasks'
+      assert(last_response.forbidden?, 'The request should be forbidden')
+    end
+
+    it 'accepts unauthorized https connections (using https_cert_cn)' do
+      mock_core_service(:create_post, '/tasks', :code => 200, :body => {})
+      Proxy::Dynflow::Api.any_instance.stubs(:request).
+        returns(request_factory('POST', '/tasks', 'HTTPS' => 'yes', 'SSL_CLIENT_CERT' => 'mytrustedcert'))
+      Proxy::Dynflow::Api.any_instance.stubs(:https_cert_cn).returns('mytrustedhost.example.com')
+      Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"])
+      post '/tasks'
+      assert(last_response.ok?, 'The response should be ok')
+    end
+
+    it 'refuses unauthorized https connections (when client cert is not supplied)' do
+      Proxy::Dynflow::Api.any_instance.stubs(:request).
+        returns(request_factory('POST', '/tasks', 'HTTPS' => 'yes'))
+      Proxy::Dynflow::Api.any_instance.stubs(:https_cert_cn).returns('mytrustedhost.example.com')
+      Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"])
+      post '/tasks'
+      assert(last_response.forbidden?, 'The request should be forbidden')
+    end
+
+    it 'passes the done requests to the core service, when authorization keys are provided' do
+      mock_core_service(:create_post, '/tasks/123/done', :code => 200, :body => {})
+      Proxy::Dynflow::Api.any_instance.stubs(:request).
+        returns(request_factory('POST', '/tasks/123/done', 'HTTPS' => 'yes', 'HTTP_AUTHORIZATION' => 'Basic ValidToken'))
+      Proxy::Dynflow::Api.any_instance.stubs(:https_cert_cn).returns('mytrustedhost.example.com')
+      Proxy::SETTINGS.stubs(:trusted_hosts).returns(["mytrustedhost.example.com"])
+      post '/tasks'
+      assert(last_response.ok?, 'The response should be ok')
+    end
   end
 end