From 4359d63ae75e505107a937997d17fcb25ed44557 Mon Sep 17 00:00:00 2001 From: Matan Benita Date: Sun, 21 Feb 2021 17:03:23 +0200 Subject: [PATCH 1/2] Raise proper error in case k8s host not found - Added RoleNotFound error message in case host not exist for hostId - Fixed bug when creating InjectClientCert inside log write command - Added UT --- CHANGELOG.md | 3 ++ .../authn_k8s/inject_client_cert.rb | 20 +++++++-- .../authn_k8s/inject_client_cert_spec.rb | 44 +++++++++++++++++++ 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d75304921..07c5fe1a44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Changed +- Conjur now raises a RoleNotFound when trying to authenticate a non-existing host in authn-k8s + [conjurinc/appliance#1546]https://github.com/conjurinc/appliance/issues/1546 ## [1.11.2] - 2021-02-02 ### Added diff --git a/app/domain/authentication/authn_k8s/inject_client_cert.rb b/app/domain/authentication/authn_k8s/inject_client_cert.rb index ee51e3472b..4cca09e784 100644 --- a/app/domain/authentication/authn_k8s/inject_client_cert.rb +++ b/app/domain/authentication/authn_k8s/inject_client_cert.rb @@ -115,7 +115,13 @@ def spiffe_id end def host - @host ||= @resource_class[host_id] + return @host if @host + + @host = @resource_class[host_id] + + raise Errors::Authentication::Security::RoleNotFound, host_id unless @host + + @host end def validate_spiffe_id_exists @@ -161,7 +167,7 @@ def audit_success Audit::Event::Authn::InjectClientCert.new( authenticator_name: AUTHENTICATOR_NAME, service: webservice, - role_id: host.id, + role_id: sanitized_role_id, client_ip: @client_ip, success: true, error_message: nil @@ -174,13 +180,21 @@ def audit_failure(err) Audit::Event::Authn::InjectClientCert.new( authenticator_name: AUTHENTICATOR_NAME, service: webservice, - role_id: host.id, + role_id: sanitized_role_id, client_ip: @client_ip, success: false, error_message: err.message ) ) end + # TODO: this logic is taken from app/models/audit/event/authn.rb. + # We should use that logic here. + # Masking role if it doesn't exist to avoid audit pollution + def sanitized_role_id + return "not-found" unless @resource_class[host_id] + + host.id + end end end end diff --git a/spec/app/domain/authentication/authn_k8s/inject_client_cert_spec.rb b/spec/app/domain/authentication/authn_k8s/inject_client_cert_spec.rb index fed9bd818d..518860a7ee 100644 --- a/spec/app/domain/authentication/authn_k8s/inject_client_cert_spec.rb +++ b/spec/app/domain/authentication/authn_k8s/inject_client_cert_spec.rb @@ -17,6 +17,7 @@ let(:csr) { "CSR" } let(:host_id) { "HostId" } + let(:non_existing_host_id) { "non-exist" } let(:host_role) { double("HostRole", id: host_id) } let(:k8s_authn_container_name) { 'kubernetes/authentication-container-name' } @@ -49,6 +50,14 @@ ) } + let(:non_existing_k8s_host) { + double( + "K8sHost", + account: account, + conjur_host_id: non_existing_host_id + ) + } + let(:spiffe_altname) { "SpiffeAltname" } let(:spiffe_id) { double( @@ -105,6 +114,10 @@ allow(resource_class).to receive(:[]) .with(host_id) .and_return(host) + + allow(resource_class).to receive(:[]) + .with(non_existing_host_id) + .and_return(nil) end end @@ -222,6 +235,37 @@ end end + context "when host does not exist" do + it "throws RoleNotFound" do + error_type = Errors::Authentication::Security::RoleNotFound + + allow(Authentication::AuthnK8s::K8sHost) + .to receive(:from_csr) + .with(hash_including(account: account, + service_name: service_id, + csr: anything)) + .and_return(non_existing_k8s_host) + + allow(Authentication::AuthnK8s::PodRequest) + .to receive(:new) + .with(hash_including(service_id: service_id, + k8s_host: non_existing_k8s_host, + spiffe_id: spiffe_id)) + .and_return(pod_request) + + allow(validate_pod_request) + .to receive(:call) + .with(hash_including(pod_request: anything)) + .and_return(nil) + + expect { injector.(conjur_account: account, + service_id: service_id, + csr: csr, + host_id_prefix: host_id_prefix, + client_ip: client_ip) }.to raise_error(error_type) + end + end + context "when validate_pod_request fails" do let(:audit_success) { false } From 27193bb7eae7cb7280e9d6c82f44dbb0b9687f06 Mon Sep 17 00:00:00 2001 From: Matan Benita Date: Mon, 22 Feb 2021 11:02:53 +0200 Subject: [PATCH 2/2] Raise proper error in case k8s host not found - Added RoleNotFound error message in case host not exist for hostId - Fixed bug when creating InjectClientCert inside log write command - Added UT --- CHANGELOG.md | 2 +- app/domain/authentication/authn_k8s/inject_client_cert.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07c5fe1a44..ec6eb892d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] ### Changed - Conjur now raises a RoleNotFound when trying to authenticate a non-existing host in authn-k8s - [conjurinc/appliance#1546]https://github.com/conjurinc/appliance/issues/1546 + [cyberark/conjur#2046](https://github.com/cyberark/conjur/issues/2046) ## [1.11.2] - 2021-02-02 ### Added diff --git a/app/domain/authentication/authn_k8s/inject_client_cert.rb b/app/domain/authentication/authn_k8s/inject_client_cert.rb index 4cca09e784..e31bd64e85 100644 --- a/app/domain/authentication/authn_k8s/inject_client_cert.rb +++ b/app/domain/authentication/authn_k8s/inject_client_cert.rb @@ -187,6 +187,7 @@ def audit_failure(err) ) ) end + # TODO: this logic is taken from app/models/audit/event/authn.rb. # We should use that logic here. # Masking role if it doesn't exist to avoid audit pollution