Merge pull request from GHSA-qcmx-gv2g-v767

Advisory fix cascade delete

cucumber/kubernetes/features/step_definitions/authenticate_steps.rb

@@ -5,6 +5,16 @@ def conjur_resource_id(namespace, resource_id)
 def gen_cert(host_id)
   username = [namespace, host_id].join('/')
   webservice_resource_id = "#{ENV['CONJUR_ACCOUNT']}:webservice:#{username}"
+  role_id = "#{ENV['CONJUR_ACCOUNT']}:policy:#{username}"
+  Role.create(role_id: role_id)
+  Resource.create(
+    resource_id: "#{ENV['CONJUR_ACCOUNT']}:variable:#{username}/ca/cert",
+    owner_id: role_id
+  )
+  Resource.create(
+    resource_id: "#{ENV['CONJUR_ACCOUNT']}:variable:#{username}/ca/key",
+    owner_id: role_id
+  )
   ::Repos::ConjurCA.create(webservice_resource_id)
 end
 

cucumber/policy/features/deletion.feature

@@ -16,6 +16,90 @@ Feature: Deleting objects and relationships.
     """
     Then group "developers" does not exist
 
+  Scenario: Deleting variable value is unrecoverable.
+    Given I load a policy:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    And variable "test/db-password" exists
+    # Variable loaded twice so we verify we delete all of its versions
+    And I can add a secret to variable resource "test/db-password"
+    And I can add a secret to variable resource "test/db-password"
+    And I can fetch a secret from variable resource "test/db-password"
+    When I update the policy with:
+    """
+    - !policy
+      id: test
+      body:
+      - !delete
+        record: !variable db-password
+    """
+    And I extend the policy with:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    Then variable "test/db-password" exists
+    And variable resource "test/db-password" does not have a secret value
+
+  Scenario: Deleting variable value is unrecoverable even if we add same variable with the delete policy
+    Given I load a policy:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    And variable "test/db-password" exists
+    # Variable loaded twice so we verify we delete all of its versions
+    And I can add a secret to variable resource "test/db-password"
+    And I can add a secret to variable resource "test/db-password"
+    And I can fetch a secret from variable resource "test/db-password"
+    When I update the policy with:
+    """
+    - !policy
+      id: test
+      body:
+      - !delete
+        record: !variable db-password
+      - !variable db-password
+    """
+    Then variable "test/db-password" exists
+    And variable resource "test/db-password" does not have a secret value
+
+  Scenario: Deleting variable value is unrecoverable when we delete the policy itself and then add it again
+    Given I load a policy:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    And variable "test/db-password" exists
+    # Variable loaded twice so we verify we delete all of its versions
+    And I can add a secret to variable resource "test/db-password"
+    And I can add a secret to variable resource "test/db-password"
+    And I can fetch a secret from variable resource "test/db-password"
+    When I update the policy with:
+    """
+    - !delete
+      record: !policy test
+    """
+    And I extend the policy with:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    Then variable "test/db-password" exists
+    And variable resource "test/db-password" does not have a secret value
+
   Scenario: The !revoke statement can be used to revoke a role grant.
     Given I load a policy:
     """

cucumber/policy/features/replace.feature

@@ -85,6 +85,103 @@ A policy can be reloaded using the --replace flag
     And the error code is "not_found"
     And the error message is "Role cucumber:group:security-admin does not exist"
 
+  Scenario: Removing variable declaration from policy deletes its value
+    Given I load a policy:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    # Variable loaded twice so we verify we delete all of its versions
+    And I can add a secret to variable resource "test/db-password"
+    And I can add a secret to variable resource "test/db-password"
+    And I can fetch a secret from variable resource "test/db-password"
+    When I load a policy:
+    """
+    - !policy
+      id: test
+    """
+    And I load a policy:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    Then variable "test/db-password" exists
+    And variable resource "test/db-password" does not have a secret value
+
+  Scenario: Removing policy with variable declaration deletes its value
+    Given I load a policy:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    # Variable loaded twice so we verify we delete all of its versions
+    And I can add a secret to variable resource "test/db-password"
+    And I can add a secret to variable resource "test/db-password"
+    And I can fetch a secret from variable resource "test/db-password"
+    When I load a policy:
+    """
+    - !policy empty
+    """
+    And I load a policy:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    Then variable "test/db-password" exists
+    And variable resource "test/db-password" does not have a secret value
+
+  Scenario: Replacing policy with variable declaration keeps variable's secret value
+    Given I load a policy:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    # Variable loaded twice so we verify we delete all of its versions
+    And I can add a secret to variable resource "test/db-password"
+    And I can add a secret to variable resource "test/db-password"
+    And I can fetch a secret from variable resource "test/db-password"
+    When I load a policy:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    Then variable "test/db-password" exists
+    And I can fetch a secret from variable resource "test/db-password"
+
+  Scenario: Replacing policy root with same policy tests replaces the variable
+    Given I load a policy:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    # Variable loaded twice so we verify we delete all of its versions
+    And I can add a secret to variable resource "test/db-password"
+    And I can add a secret to variable resource "test/db-password"
+    And I can fetch a secret from variable resource "test/db-password"
+    When I replace the "root" policy with:
+    """
+    - !policy
+      id: test
+      body:
+      - !variable db-password
+    """
+    Then variable "test/db-password" exists
+    And I can fetch a secret from variable resource "test/db-password"
+
   Scenario: A multifile policy successfully reloads when files are concatenated
 
     Given I load a policy:

cucumber/policy/features/step_definitions/secrets_steps.rb

@@ -9,8 +9,17 @@
 end
 
 Then(/^I can( not)? fetch a secret from ([\w_]+) resource "([^"]*)"$/) do |fail, kind, id|
-  status = fail ? 403 : 200
-  invoke status: status do
+  expected_status = fail ? 403 : 200
+  try_get_secret_value(id, kind, expected_status)
+end
+
+Then(/^variable resource "([^"]*)" does not have a secret value$/) do |id|
+  expected_status = 404
+  try_get_secret_value(id, expected_status)
+end
+
+def try_get_secret_value(id, kind = "variable", expected_status)
+  invoke status: expected_status do
     conjur_api.resource(make_full_id(kind, id)).value
   end
 end

db/migrate/20210316160127_secret_cascade_delete.rb

@@ -0,0 +1,24 @@
+Sequel.migration do
+  up do
+    # Remove orphan secret so turning its resource ID to foreign key will work
+    execute <<-DELETE
+      DELETE FROM secrets
+      WHERE NOT EXISTS (
+        SELECT 1 FROM resources
+        WHERE secrets.resource_id = resource_id
+      )
+    DELETE
+    # Create cascade delete relationship between resource and secret so when resource deleted its secrets are deleted too
+    alter_table :secrets do
+      add_foreign_key [:resource_id], :resources, on_delete: :cascade
+      add_index :resource_id
+    end
+  end
+
+  down do
+    alter_table :secrets do
+      drop_foreign_key [:resource_id]
+      drop_index :resource_id
+    end
+  end
+end