From f9ba07756fef2e69ac8b5f72865a071e736852d6 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Sat, 9 Nov 2024 09:30:19 -0500 Subject: [PATCH] First cut on group read-only policy --- app/controllers/secrets_controller.rb | 2 +- app/interactors/write_secret.rb | 2 +- app/lib/clients/vault/certificate.rb | 2 +- app/lib/clients/vault/key_value.rb | 37 +++++++++++++++++++-------- app/lib/clients/vault/oidc.rb | 14 ++++++++++ app/lib/clients/vault/policy.rb | 20 +++++++++++++-- app/lib/requests/secret_request.rb | 5 ++++ app/lib/services/key_value.rb | 4 +-- app/models/domain.rb | 1 - doc/openapi/paths/secrets.yml | 4 +++ test/lib/clients/vault_test.rb | 10 ++++---- 11 files changed, 77 insertions(+), 24 deletions(-) diff --git a/app/controllers/secrets_controller.rb b/app/controllers/secrets_controller.rb index 331ec24..1f5b5d2 100644 --- a/app/controllers/secrets_controller.rb +++ b/app/controllers/secrets_controller.rb @@ -39,6 +39,6 @@ def destroy private def params_permitted - params.require(:secret).permit(:path, data: {}) + params.require(:secret).permit(:path, :groups, data: {}) end end diff --git a/app/interactors/write_secret.rb b/app/interactors/write_secret.rb index ec4ea4c..8a8141a 100644 --- a/app/interactors/write_secret.rb +++ b/app/interactors/write_secret.rb @@ -1,6 +1,6 @@ class WriteSecret < ApplicationInteractor def call - if secret = Services::KeyValue.write(context.identity, context.request.path, context.request.data) + if secret = Services::KeyValue.write(context.identity, context.request.groups, context.request.path, context.request.data) context.secret = secret else context.fail!(message: "Failed to store secret") diff --git a/app/lib/clients/vault/certificate.rb b/app/lib/clients/vault/certificate.rb index 5f82ac5..25da971 100644 --- a/app/lib/clients/vault/certificate.rb +++ b/app/lib/clients/vault/certificate.rb @@ -9,7 +9,7 @@ def issue_cert(identity, cert_issue_request) opts = cert_issue_request.attributes # Generate the TLS certificate using the intermediate CA tls_cert = client.logical.write(cert_path, opts) - assign_policy(identity, GENERIC_CERT_POLICY_NAME) + assign_identity_policy(identity, GENERIC_CERT_POLICY_NAME) OpenStruct.new tls_cert.data end diff --git a/app/lib/clients/vault/key_value.rb b/app/lib/clients/vault/key_value.rb index a43c033..18c9c7a 100644 --- a/app/lib/clients/vault/key_value.rb +++ b/app/lib/clients/vault/key_value.rb @@ -4,20 +4,22 @@ module KeyValue extend Policy def kv_read(identity, path) - verify_policy(identity, policy_path(path)) + verify_policy(identity, producer_policy_path(path)) client.kv(kv_mount).read(path) end - def kv_write(identity, path, data) - create_kv_policy(path) - assign_policy(identity, policy_path(path)) + def kv_write(identity, groups, path, data) + create_kv_policies(path) + assign_identity_policy(identity, producer_policy_path(path)) + assign_groups_policy(groups, consumer_policy_path(path)) client.logical.write("#{kv_mount}/data/#{path}", data: data) end def kv_delete(identity, path) - verify_policy(identity, policy_path(path)) + verify_policy(identity, producer_policy_path(path)) client.logical.delete("#{kv_mount}/data/#{path}") - remove_policy(identity, policy_path(path)) + remove_identity_policy(identity, producer_policy_path(path)) + remove_groups_policy(consumer_policy_path(path)) end def configure_kv @@ -36,21 +38,34 @@ def kv_engine_type "kv-v2" end - def create_kv_policy(path) - client.sys.put_policy(policy_path(path), kv_policy(path)) + def create_kv_policies(path) + client.sys.put_policy(producer_policy_path(path), kv_producer_policy(path)) + client.sys.put_policy(consumer_policy_path(path), kv_consumer_policy(path)) end - def policy_path(path) - "kv_policy/#{path}" + def producer_policy_path(path) + "kv_policy/#{path}/producer" end - def kv_policy(path) + def consumer_policy_path(path) + "kv_policy/#{path}/consumer" + end + + def kv_producer_policy(path) policy = <<-EOH path "#{path}" { capabilities = ["create", "read", "update", "delete"] } EOH end + + def kv_consumer_policy(path) + policy = <<-EOH + path "#{path}" { + capabilities = ["read"] + } + EOH + end end end end diff --git a/app/lib/clients/vault/oidc.rb b/app/lib/clients/vault/oidc.rb index ac1dd36..ccf61c7 100644 --- a/app/lib/clients/vault/oidc.rb +++ b/app/lib/clients/vault/oidc.rb @@ -19,6 +19,20 @@ def get_oidc_client_config client.logical.read("auth/oidc/config") end + def create_oidc_role(role_name, groups, policy_name) + client.logical.write("auth/oidc/role/#{role_name}", + user_claim: "sub", + groups_claim: "groups", + bound_claims: { "groups" => groups }, + policies: policy_name, + oidc_scopes: "email groups", + allowed_redirect_uris: Config[:oidc_redirect_uris]) + end + + def remove_oidc_role(role_name) + client.logical.delete("auth/oidc/role/#{role_name}") + end + private def create_client_config(issuer, client_id, client_secret) diff --git a/app/lib/clients/vault/policy.rb b/app/lib/clients/vault/policy.rb index 61dd38a..2aa0da0 100644 --- a/app/lib/clients/vault/policy.rb +++ b/app/lib/clients/vault/policy.rb @@ -2,6 +2,7 @@ module Clients class Vault module Policy extend Entity + extend Oidc def rotate_token create_astral_policy @@ -9,7 +10,7 @@ def rotate_token Clients::Vault.token = token end - def assign_policy(identity, policy_name) + def assign_identity_policy(identity, policy_name) sub = identity.sub email = identity.email Domain.with_advisory_lock(sub) do @@ -20,6 +21,10 @@ def assign_policy(identity, policy_name) end end + def assign_groups_policy(groups, policy_name) + create_oidc_role(make_role_name(policy_name), groups, policy_name) + end + def verify_policy(identity, policy_name) sub = identity.sub policies, _ = get_entity_data(sub) @@ -28,7 +33,7 @@ def verify_policy(identity, policy_name) end end - def remove_policy(identity, policy_name) + def remove_identity_policy(identity, policy_name) sub = identity.sub Domain.with_advisory_lock(sub) do policies, metadata = get_entity_data(sub) @@ -38,8 +43,16 @@ def remove_policy(identity, policy_name) client.sys.delete_policy(policy_name) end + def remove_groups_policy(policy_name) + remove_oidc_role(make_role_name(policy_name)) + end + private + def make_role_name(policy_name) + %Q(#{policy_name.gsub("/", "_")}-role) + end + def create_astral_policy policy = <<-HCL path "#{intermediate_ca_mount}/roles/astral" { @@ -66,6 +79,9 @@ def create_astral_policy path "auth/oidc/config" { capabilities = ["read"] } + path "auth/oidc/role/*" { + capabilities = ["create", "read", "update", "delete", "list"] + } path "/sys/policy/*" { capabilities = ["create", "read", "update", "delete", "list"] } diff --git a/app/lib/requests/secret_request.rb b/app/lib/requests/secret_request.rb index 2ec6517..9dd98a4 100644 --- a/app/lib/requests/secret_request.rb +++ b/app/lib/requests/secret_request.rb @@ -4,9 +4,14 @@ class SecretRequest include ActiveModel::Attributes attribute :path, :string + attribute :groups, :string attribute :data alias_attribute :kv_path, :path validates :path, presence: true + + def groups_array + (groups || "").split(",").sort.uniq + end end end diff --git a/app/lib/services/key_value.rb b/app/lib/services/key_value.rb index 9bf2ad4..4c2345d 100644 --- a/app/lib/services/key_value.rb +++ b/app/lib/services/key_value.rb @@ -5,8 +5,8 @@ def read(identity, path) impl.kv_read(identity, path) end - def write(identity, path, data) - impl.kv_write(identity, path, data) + def write(identity, read_groups, path, data) + impl.kv_write(identity, read_groups, path, data) end def delete(identity, path) diff --git a/app/models/domain.rb b/app/models/domain.rb index a7a1ca2..8692fe9 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -5,7 +5,6 @@ class Domain < ApplicationRecord encrypts :fqdn, :users, :groups end - def groups_array (groups || "").split(",").sort.uniq end diff --git a/doc/openapi/paths/secrets.yml b/doc/openapi/paths/secrets.yml index 02ae979..8feec22 100644 --- a/doc/openapi/paths/secrets.yml +++ b/doc/openapi/paths/secrets.yml @@ -15,6 +15,10 @@ postSecrets: type: string description: "Path where the secret is stored" example: "secret/storage/path" + gropus: + type: string + description: "Comma-separated list of groups allowed to read the secret" + example: "svc-account1,dev-group1" data: type: object description: "The secret data" diff --git a/test/lib/clients/vault_test.rb b/test/lib/clients/vault_test.rb index 0d80b9c..4e4223f 100644 --- a/test/lib/clients/vault_test.rb +++ b/test/lib/clients/vault_test.rb @@ -68,7 +68,7 @@ class VaultTest < ActiveSupport::TestCase # now has a new token assert_not_equal vault_token, @client.token # ensure we can write with the new token - assert_instance_of Vault::Secret, @client.kv_write(@identity, "testing/secret", { password: "sicr3t" }) + assert_instance_of Vault::Secret, @client.kv_write(@identity, [], "testing/secret", { password: "sicr3t" }) end test "entity methods" do @@ -87,7 +87,7 @@ class VaultTest < ActiveSupport::TestCase test "kv methods" do # check kv_write path = "test/path/#{SecureRandom.hex}" - secret = @client.kv_write(@identity, path, { data: "data" }) + secret = @client.kv_write(@identity, [], path, { data: "data" }) assert_kind_of Vault::Secret, secret # check kv_read @@ -96,7 +96,7 @@ class VaultTest < ActiveSupport::TestCase # check policy is created entity = @client.read_entity(@identity.sub) - assert_includes entity.data[:policies], "kv_policy/#{path}" + assert_includes entity.data[:policies], "kv_policy/#{path}/producer" # check kv_read denied to other identity alt_identity = Identity.new @@ -146,8 +146,8 @@ class VaultTest < ActiveSupport::TestCase assert_match /no such alias/, err.message end - test ".assign_policy creates valid entity" do - @client.assign_policy(@identity, "test_path") + test ".assign_identity_policy creates valid entity" do + @client.assign_identity_policy(@identity, "test_path") entity = @client.read_entity(@identity.sub) assert entity.data[:policies].any? { |p| p == "test_path" }