Skip to content

Backfill SSL verification for integrations with known-good hostnames

Markus Koller requested to merge 349697-backfill-enable-ssl-verification-for-known-hostnames into master

What does this MR do and why?

Some integrations were hard-coded to disable SSL verification, and we added this setting in a security fix and enabled it by default for new configurations.

For existing configurations this was handled in the application code, but now that the security fix is released we can update the settings in the DB, and then in the next release we can remove our temporary code in the app again.

Issue: #349697 (closed), follow-up to https://gitlab.com/gitlab-org/gitlab/-/issues/296632 / https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/2043.

Migration details

db/post_migrate/20220425121410_add_temporary_index_for_backfill_integrations_enable_ssl_verification.rb:

Execution time in #database-lab: 36.897 s

Output:

$ rails db:migrate:up VERSION=20220425121410
== 20220209121410 AddTemporaryIndexForBackfillIntegrationsEnableSslVerification: migrating
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:integrations, :id, {:where=>"type_new IN ('Integrations::DroneCi', 'Integrations::Teamcity') AND encrypted_properties IS NOT NULL", :name=>"tmp_index_integrations_on_id_where_type_droneci_or_teamcity", :algorithm=>:concurrently})
   -> 0.0250s
-- execute("SET statement_timeout TO 0")
   -> 0.0016s
-- add_index(:integrations, :id, {:where=>"type_new IN ('Integrations::DroneCi', 'Integrations::Teamcity') AND encrypted_properties IS NOT NULL", :name=>"tmp_index_integrations_on_id_where_type_droneci_or_teamcity", :algorithm=>:concurrently})
   -> 0.0086s
-- execute("RESET statement_timeout")
   -> 0.0013s
== 20220425121410 AddTemporaryIndexForBackfillIntegrationsEnableSslVerification: migrated (0.0482s)

$ rails db:migrate:down VERSION=20220425121410
== 20220425121410 AddTemporaryIndexForBackfillIntegrationsEnableSslVerification: reverting
-- transaction_open?()
   -> 0.0000s
-- indexes(:integrations)
   -> 0.0122s
-- execute("SET statement_timeout TO 0")
   -> 0.0006s
-- remove_index(:integrations, {:algorithm=>:concurrently, :name=>"tmp_index_integrations_on_id_where_type_droneci_or_teamcity"})
   -> 0.0045s
-- execute("RESET statement_timeout")
   -> 0.0007s
== 20220425121410 AddTemporaryIndexForBackfillIntegrationsEnableSslVerification: reverted (0.0254s)

db/post_migrate/20220425121435_backfill_integrations_enable_ssl_verification.rb

Example query plans:

Output:

$ rails db:migrate:up VERSION=20220425121435
== 20220425121435 BackfillIntegrationsEnableSslVerification: migrating ========
-- Scheduled 1 BackfillIntegrationsEnableSslVerification jobs with a maximum of 1000 records per batch and an interval of 300 seconds.

The migration is expected to take at least 300 seconds. Expect all jobs to have completed after 2022-04-25 15:12:32 UTC."
== 20220425121435 BackfillIntegrationsEnableSslVerification: migrated (0.0817s)

$ rails db:migrate:down VERSION=20220425121435
== 20220425121435 BackfillIntegrationsEnableSslVerification: reverting ========
== 20220425121435 BackfillIntegrationsEnableSslVerification: reverted (0.0000s)

lib/gitlab/background_migration/backfill_integrations_enable_ssl_verification.rb

  • The SELECT query per batch looks like:

    SELECT "integrations".* FROM "integrations" WHERE "integrations"."type_new" IN ('Integrations::DroneCi', 'Integrations::Teamcity') AND "integrations"."encrypted_properties" IS NOT NULL AND "integrations"."id" BETWEEN 1000 AND 2000

  • The UPDATE query per integration looks like:

    UPDATE "integrations" SET "encrypted_properties" = '...(encrypted value)...' WHERE "integrations"."id" = 74

  • On gitlab.com we have around 3464 Integrations::DroneCi records, and 1812 Integrations::Teamcity records, but only some of them have known-good hostnames and will be updated.

    • This will result in (up to) 6 batches and an expected total runtime of ~30 minutes.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #349697 (closed)

Edited by Markus Koller

Merge request reports