summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThong Kuah <tkuah@gitlab.com>2019-02-13 09:46:59 +1300
committerThong Kuah <tkuah@gitlab.com>2019-02-21 23:24:21 +1300
commit0e53c34f669fd6449d8ce636e31a5f85c7082300 (patch)
tree72ffa8561a65da660c262386c8c14215a6f8b972
parentc5b5b18b3f1c5b683ceb4471e667d675de9200eb (diff)
downloadgitlab-ce-0e53c34f669fd6449d8ce636e31a5f85c7082300.tar.gz
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
-rw-r--r--app/models/clusters/platforms/kubernetes.rb2
-rw-r--r--changelogs/unreleased/security-kubernetes-local-ssrf.yml5
-rw-r--r--lib/gitlab/kubernetes/kube_client.rb8
-rw-r--r--spec/lib/gitlab/kubernetes/kube_client_spec.rb30
-rw-r--r--spec/models/clusters/platforms/kubernetes_spec.rb16
5 files changed, 60 insertions, 1 deletions
diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb
index 1cc170c8c4d..c8484923502 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
validate :prevent_modification, on: :update
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 fe839940f74..22125e7a716 100644
--- a/lib/gitlab/kubernetes/kube_client.rb
+++ b/lib/gitlab/kubernetes/kube_client.rb
@@ -79,6 +79,8 @@ module Gitlab
def initialize(api_prefix, **kubeclient_options)
@api_prefix = api_prefix
@kubeclient_options = kubeclient_options
+
+ validate_url!
end
def create_or_update_cluster_role_binding(resource)
@@ -115,6 +117,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 8fc85301304..59dbb903734 100644
--- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb
+++ b/spec/lib/gitlab/kubernetes/kube_client_spec.rb
@@ -24,6 +24,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 6c8a223092e..f612b75d949 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