Skip to content

Add Dast::SiteProfileSecretVariables and associations

What does this MR do?

this merge request adds the ability to store secrets for dast on-demand scans via the introduction of a new table (dast_site_profile_secret_variables).

this merge request started as !54397 (closed) but was separated out to make review easier.

Why?

as part of the work on dast on-demand scans, we want to give users the ability to specify login credentials for their on-demand scans in order to perform authenticated scans.

during security review we identified that ci_variables seemed like the correct place to store this data but that one of the risks associated was that sensitive ci_variables could end up being sent to jobs that should not have access to them (e.g. if someone were set the environment scope incorrectly).

following discussion in review on this merge request, we opted to move away from using the ci_variables and simplify the solution by introducing a new table and extending Ci::Contextable.

Related Issue(s)

Related Merge Request(s)

Database

Migrations

% rails db:migrate:up VERSION=20210305031822 && rails db:migrate:down VERSION=20210305031822                                                                                                                                                                                                                                                                                
== 20210305031822 CreateDastSiteProfileVariables: migrating ===================
-- create_table(:dast_site_profile_secret_variables, {:comment=>"{\"owner\":\"group::dynamic analysis\",\"description\":\"Secret variables used in DAST on-demand scans\"}"})
-- quote_column_name(:key)
   -> 0.0000s
   -> 0.0143s
-- quote_table_name("check_8cbef204b2")
   -> 0.0000s
-- quote_table_name("check_236213f179")
   -> 0.0000s
-- quote_table_name("check_b49080abbf")
   -> 0.0000s
-- quote_table_name(:dast_site_profile_secret_variables)
   -> 0.0000s
-- execute("ALTER TABLE \"dast_site_profile_secret_variables\"\nADD CONSTRAINT \"check_8cbef204b2\" CHECK (char_length(\"key\") <= 255),\nADD CONSTRAINT \"check_236213f179\" CHECK (length(encrypted_value) <= 13352),\nADD CONSTRAINT \"check_b49080abbf\" CHECK (length(encrypted_value_iv) <= 17)\n")
   -> 0.0012s
== 20210305031822 CreateDastSiteProfileVariables: migrated (0.0222s) ==========

== 20210305031822 CreateDastSiteProfileVariables: reverting ===================
-- drop_table(:dast_site_profile_secret_variables)
   -> 0.0110s
== 20210305031822 CreateDastSiteProfileVariables: reverted (0.0110s) ==========

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Philip Cunningham

Merge request reports