From b28592a8bb7d945a899141989598e6c41777462f Mon Sep 17 00:00:00 2001 From: Thong Kuah 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 e06cb9be89f..ec2c5137f5b 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 b30f80a4b3e..c2ad0076e2d 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 -- GitLab