Merge branch 'security-kubernetes-local-ssrf' into 'master'

Block local URLs for Kubernetes integration

See merge request gitlab/gitlabhq!2901

app/models/clusters/platforms/kubernetes.rb

@@ -41,7 +41,7 @@ module Clusters
       validate :no_namespace, unless: :allow_user_defined_namespace?
 
       # We expect to be `active?` only when enabled and cluster is created (the api_url is assigned)
-      validates :api_url, url: true, presence: true
+      validates :api_url, public_url: true, presence: true
       validates :token, presence: true
       validates :ca_cert, certificate: true, allow_blank: true, if: :ca_cert_changed?
 

changelogs/unreleased/security-kubernetes-local-ssrf.yml

+---
+title: Block local URLs for Kubernetes integration
+merge_request:
+author:
+type: security

lib/gitlab/kubernetes/kube_client.rb

@@ -82,6 +82,8 @@ module Gitlab
       def initialize(api_prefix, **kubeclient_options)
         @api_prefix = api_prefix
         @kubeclient_options = kubeclient_options.merge(http_max_redirects: 0)
+
+        validate_url!
       end
 
       def create_or_update_cluster_role_binding(resource)
@@ -118,6 +120,12 @@ module Gitlab
 
       private
 
+      def validate_url!
+        return if Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
+
+        Gitlab::UrlBlocker.validate!(api_prefix, allow_local_network: false)
+      end
+
       def cluster_role_binding_exists?(resource)
         get_cluster_role_binding(resource.metadata.name)
       rescue ::Kubeclient::ResourceNotFoundError

spec/lib/gitlab/kubernetes/kube_client_spec.rb

@@ -50,6 +50,36 @@ describe Gitlab::Kubernetes::KubeClient do
     end
   end
 
+  describe '#initialize' do
+    shared_examples 'local address' do
+      it 'blocks local addresses' do
+        expect { client }.to raise_error(Gitlab::UrlBlocker::BlockedUrlError)
+      end
+
+      context 'when local requests are allowed' do
+        before do
+          stub_application_setting(allow_local_requests_from_hooks_and_services: true)
+        end
+
+        it 'allows local addresses' do
+          expect { client }.not_to raise_error
+        end
+      end
+    end
+
+    context 'localhost address' do
+      let(:api_url) { 'http://localhost:22' }
+
+      it_behaves_like 'local address'
+    end
+
+    context 'private network address' do
+      let(:api_url) { 'http://192.168.1.2:3003' }
+
+      it_behaves_like 'local address'
+    end
+  end
+
   describe '#core_client' do
     subject { client.core_client }
 

spec/models/clusters/platforms/kubernetes_spec.rb

@@ -98,6 +98,22 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
 
         it { expect(kubernetes.save).to be_truthy }
       end
+
+      context 'when api_url is localhost' do
+        let(:api_url) { 'http://localhost:22' }
+
+        it { expect(kubernetes.save).to be_falsey }
+
+        context 'Application settings allows local requests' do
+          before do
+            allow(ApplicationSetting)
+              .to receive(:current)
+              .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true))
+          end
+
+          it { expect(kubernetes.save).to be_truthy }
+        end
+      end
     end
 
     context 'when validates token' do