From af16fd687e2e5b15a63e6e51d76847512ae8ee72 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Wed, 13 Feb 2019 09:46:59 +1300
Subject: [PATCH] Do not allow local urls in Kubernetes form

Use existing `public_url` validation to block various local urls. Note
that this validation will allow local urls if the "Allow requests to the
local network from hooks and services" admin setting is enabled.

Block KubeClient from using local addresses

It will also respect `allow_local_requests_from_hooks_and_services` so
if that is enabled KubeClinet will allow local addresses
---
 app/models/clusters/platforms/kubernetes.rb   |  2 +-
 .../security-kubernetes-local-ssrf.yml        |  5 ++++
 lib/gitlab/kubernetes/kube_client.rb          |  8 +++++
 .../lib/gitlab/kubernetes/kube_client_spec.rb | 30 +++++++++++++++++++
 .../clusters/platforms/kubernetes_spec.rb     | 16 ++++++++++
 5 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/security-kubernetes-local-ssrf.yml

diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb
index 46d0898014e..814fc591408 100644
--- a/app/models/clusters/platforms/kubernetes.rb
+++ b/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?
 
diff --git a/changelogs/unreleased/security-kubernetes-local-ssrf.yml b/changelogs/unreleased/security-kubernetes-local-ssrf.yml
new file mode 100644
index 00000000000..7a2ad092339
--- /dev/null
+++ b/changelogs/unreleased/security-kubernetes-local-ssrf.yml
@@ -0,0 +1,5 @@
+---
+title: Block local URLs for Kubernetes integration
+merge_request:
+author:
+type: security
diff --git a/lib/gitlab/kubernetes/kube_client.rb b/lib/gitlab/kubernetes/kube_client.rb
index 624c2c67551..de14df56555 100644
--- a/lib/gitlab/kubernetes/kube_client.rb
+++ b/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
diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb
index 02364e92149..978e64c4407 100644
--- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb
+++ b/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 }
 
diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb
index 4068d98d8f7..3b32ca8df05 100644
--- a/spec/models/clusters/platforms/kubernetes_spec.rb
+++ b/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