From 1e83265eb37c3616787475e24c1f7055080820bd Mon Sep 17 00:00:00 2001 From: Avielle Wolfe Date: Tue, 2 Aug 2022 13:13:10 +0200 Subject: [PATCH 1/5] Add `id_token` keyword to `secrets` When the JTW opt-in feature is enabled, the `id_token` keyword is required for a job to define a JWT. The `id_token` keyword must have a sub-keyword `aud` that defines a string or array of audiences that can access the Vault service that the JWT is used with. Changelog: added --- ee/lib/gitlab/ci/config/entry/secret.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/lib/gitlab/ci/config/entry/secret.rb b/ee/lib/gitlab/ci/config/entry/secret.rb index d0f595f2a1d632..24bad66037adb4 100644 --- a/ee/lib/gitlab/ci/config/entry/secret.rb +++ b/ee/lib/gitlab/ci/config/entry/secret.rb @@ -11,8 +11,7 @@ class Secret < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Configurable include ::Gitlab::Config::Entry::Attributable - REQUIRED_KEYS = %i[vault].freeze - ALLOWED_KEYS = (REQUIRED_KEYS + %i[file]).freeze + ALLOWED_KEYS = %i[vault file aud].freeze attributes ALLOWED_KEYS @@ -20,7 +19,8 @@ class Secret < ::Gitlab::Config::Entry::Node entry :file, ::Gitlab::Config::Entry::Boolean, description: 'Should the created variable be of file type' validations do - validates :config, allowed_keys: ALLOWED_KEYS, required_keys: REQUIRED_KEYS + validates :config, allowed_keys: ALLOWED_KEYS + validates :aud, type: String, presence: false end end end -- GitLab From 2aaee5c03aa54895aac9aff6a019f1d7ea15806e Mon Sep 17 00:00:00 2001 From: Avielle Wolfe Date: Tue, 2 Aug 2022 21:48:25 +0200 Subject: [PATCH 2/5] Add IdToken entry class Also updates build metadata schema to include `id_token` and `aud` --- .../json_schemas/build_metadata_secrets.json | 10 ++++++- ee/lib/gitlab/ci/config/entry/id_token.rb | 26 +++++++++++++++++++ ee/lib/gitlab/ci/config/entry/secret.rb | 4 +-- 3 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 ee/lib/gitlab/ci/config/entry/id_token.rb diff --git a/app/validators/json_schemas/build_metadata_secrets.json b/app/validators/json_schemas/build_metadata_secrets.json index 3c8035d0dcf41a..c6e19685a2f939 100644 --- a/app/validators/json_schemas/build_metadata_secrets.json +++ b/app/validators/json_schemas/build_metadata_secrets.json @@ -24,7 +24,15 @@ }, "additionalProperties": false }, - "^file$": { "type": "boolean" } + "^file$": { "type": "boolean" }, + "^id_token$": { + "type": "object", + "required": ["aud"], + "properties": { + "aud": { "type": "string" } + }, + "additionalProperties": false + } }, "additionalProperties": false } diff --git a/ee/lib/gitlab/ci/config/entry/id_token.rb b/ee/lib/gitlab/ci/config/entry/id_token.rb new file mode 100644 index 00000000000000..e04bff1f5a8cae --- /dev/null +++ b/ee/lib/gitlab/ci/config/entry/id_token.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module Entry + ## + # Entry that represents a JWT definition. + # + class IdToken < ::Gitlab::Config::Entry::Node + include ::Gitlab::Config::Entry::Attributable + include ::Gitlab::Config::Entry::Validatable + + REQUIRED_KEYS = %i[aud].freeze + + attributes REQUIRED_KEYS + + validations do + validates :config, required_keys: REQUIRED_KEYS + validates :aud, type: String + end + end + end + end + end +end diff --git a/ee/lib/gitlab/ci/config/entry/secret.rb b/ee/lib/gitlab/ci/config/entry/secret.rb index 24bad66037adb4..f73662dc829e98 100644 --- a/ee/lib/gitlab/ci/config/entry/secret.rb +++ b/ee/lib/gitlab/ci/config/entry/secret.rb @@ -11,16 +11,16 @@ class Secret < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Configurable include ::Gitlab::Config::Entry::Attributable - ALLOWED_KEYS = %i[vault file aud].freeze + ALLOWED_KEYS = %i[vault file id_token].freeze attributes ALLOWED_KEYS entry :vault, Entry::Vault::Secret, description: 'Vault secrets engine configuration' entry :file, ::Gitlab::Config::Entry::Boolean, description: 'Should the created variable be of file type' + entry :id_token, ::Gitlab::Ci::Config::Entry::IdToken, description: 'Configuration for the JWT' validations do validates :config, allowed_keys: ALLOWED_KEYS - validates :aud, type: String, presence: false end end end -- GitLab From 61c1da8ca55a54a9ee0876b990b358172fb9d548 Mon Sep 17 00:00:00 2001 From: Avielle Wolfe Date: Fri, 5 Aug 2022 20:48:38 +0200 Subject: [PATCH 3/5] Add `id_tokens` field to `ci_builds_metadata` This field will store one (at first) or more (maybe later) ID tokens that will be transformed into CI variables with a JWT as their value. --- .../20220805181402_add_id_token_to_ci_builds_metadata.rb | 7 +++++++ db/schema_migrations/20220805181402 | 1 + ee/app/models/concerns/ee/ci/metadatable.rb | 1 + 3 files changed, 9 insertions(+) create mode 100644 db/migrate/20220805181402_add_id_token_to_ci_builds_metadata.rb create mode 100644 db/schema_migrations/20220805181402 diff --git a/db/migrate/20220805181402_add_id_token_to_ci_builds_metadata.rb b/db/migrate/20220805181402_add_id_token_to_ci_builds_metadata.rb new file mode 100644 index 00000000000000..3fd7d4a6897d7f --- /dev/null +++ b/db/migrate/20220805181402_add_id_token_to_ci_builds_metadata.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddIdTokenToCiBuildsMetadata < Gitlab::Database::Migration[2.0] + def change + add_column :ci_builds_metadata, :id_tokens, :jsonb, null: false, default: {} + end +end diff --git a/db/schema_migrations/20220805181402 b/db/schema_migrations/20220805181402 new file mode 100644 index 00000000000000..4631c156268e9d --- /dev/null +++ b/db/schema_migrations/20220805181402 @@ -0,0 +1 @@ +20cb099ce9e6e96048bd401707b7e362114177f16a82252625c59d643a3d7e45 \ No newline at end of file diff --git a/ee/app/models/concerns/ee/ci/metadatable.rb b/ee/app/models/concerns/ee/ci/metadatable.rb index 015f2c72d10bc3..77fc2ba264cfed 100644 --- a/ee/app/models/concerns/ee/ci/metadatable.rb +++ b/ee/app/models/concerns/ee/ci/metadatable.rb @@ -7,6 +7,7 @@ module Metadatable prepended do delegate :secrets, to: :metadata, allow_nil: true + delegate :id_tokens, to: :metadata, allow_nil: true end def secrets? -- GitLab From d642e3ed291d4b6fa4006c87094ae1b91c980358 Mon Sep 17 00:00:00 2001 From: Avielle Wolfe Date: Fri, 5 Aug 2022 20:50:06 +0200 Subject: [PATCH 4/5] Add Secrets CI entry class This class handles the separation of ID token variables from Vault secret variables in the CI `secrets` keyword --- ee/lib/ee/gitlab/ci/config/entry/job.rb | 7 ++--- ee/lib/gitlab/ci/config/entry/id_token.rb | 6 +++- ee/lib/gitlab/ci/config/entry/secrets.rb | 29 +++++++++++++++++++ .../lib/gitlab/ci/config/entry/job_spec.rb | 4 +++ 4 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 ee/lib/gitlab/ci/config/entry/secrets.rb diff --git a/ee/lib/ee/gitlab/ci/config/entry/job.rb b/ee/lib/ee/gitlab/ci/config/entry/job.rb index 965e9f6ad84334..e6cb8de013fd73 100644 --- a/ee/lib/ee/gitlab/ci/config/entry/job.rb +++ b/ee/lib/ee/gitlab/ci/config/entry/job.rb @@ -16,17 +16,16 @@ module Job description: 'DAST configuration for this job', inherit: false - entry :secrets, ::Gitlab::Config::Entry::ComposableHash, + entry :secrets, ::Gitlab::Ci::Config::Entry::Secrets, description: 'Configured secrets for this job', - inherit: false, - metadata: { composable_class: ::Gitlab::Ci::Config::Entry::Secret } + inherit: false end EE_ALLOWED_KEYS = %i[dast_configuration secrets].freeze override :value def value - super.merge({ dast_configuration: dast_configuration_value, secrets: secrets_value }.compact) + super.merge({ dast_configuration: dast_configuration_value }.merge(secrets_value).compact) end class_methods do diff --git a/ee/lib/gitlab/ci/config/entry/id_token.rb b/ee/lib/gitlab/ci/config/entry/id_token.rb index e04bff1f5a8cae..86ea453fd0ef13 100644 --- a/ee/lib/gitlab/ci/config/entry/id_token.rb +++ b/ee/lib/gitlab/ci/config/entry/id_token.rb @@ -17,7 +17,11 @@ class IdToken < ::Gitlab::Config::Entry::Node validations do validates :config, required_keys: REQUIRED_KEYS - validates :aud, type: String + validates :aud, presence: true, type: String + end + + def value + { aud: aud } end end end diff --git a/ee/lib/gitlab/ci/config/entry/secrets.rb b/ee/lib/gitlab/ci/config/entry/secrets.rb new file mode 100644 index 00000000000000..fb19aac92b493d --- /dev/null +++ b/ee/lib/gitlab/ci/config/entry/secrets.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module Entry + class Secrets < ::Gitlab::Config::Entry::ComposableHash + def composable_class(_name, _config) + Secret + end + + def value + secrets = super + + id_tokens = secrets.select do |secret_key, secret_value| + secret_value.is_a?(Hash) && secret_value.has_key?(:id_token) + end + + id_tokens.each do |token_key, token_value| + secrets.delete(token_key) + end + + { id_tokens: id_tokens, secrets: secrets } + end + end + end + end + end +end diff --git a/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb b/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb index d0e1d4939ba52d..0655c9632c7a23 100644 --- a/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -93,6 +93,7 @@ let(:config) { { script: 'echo', secrets: secrets } } let(:secrets) do { + TEST_ID_TOKEN: { id_token: { aud: 'https://gitlab.test' } }, DATABASE_PASSWORD: { vault: 'production/db/password' }, SSL_PRIVATE_KEY: { vault: 'production/ssl/private-key@ops' }, S3_SECRET_KEY: { @@ -134,6 +135,9 @@ } } }) + expect(entry.value[:id_tokens]).to eq({ + TEST_ID_TOKEN: { id_token: { aud: 'https://gitlab.test' } } + }) end end end -- GitLab From 2f96fe0c0b4f5a9926d66b2bde06366c05810ce5 Mon Sep 17 00:00:00 2001 From: Avielle Wolfe Date: Mon, 8 Aug 2022 20:28:55 +0200 Subject: [PATCH 5/5] Hook it all up Adds necessary methods and logic to save ID token configuration to the database and send it to the runner. --- app/models/ci/build.rb | 6 ++++++ ee/app/models/concerns/ee/ci/metadatable.rb | 8 ++++++++ ee/app/models/ee/ci/build.rb | 2 +- ee/app/presenters/ee/ci/build_runner_presenter.rb | 12 +++++++++++- ee/lib/ee/gitlab/ci/config/entry/job.rb | 3 ++- ee/lib/ee/gitlab/ci/yaml_processor/result.rb | 3 ++- lib/gitlab/ci/jwt.rb | 2 +- lib/gitlab/ci/jwt_v2.rb | 14 +++++++++++++- 8 files changed, 44 insertions(+), 6 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 1228dfd87d6781..cb85f09802d3c6 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -1266,6 +1266,12 @@ def job_jwt_variables variables.append(key: 'CI_JOB_JWT', value: jwt, public: false, masked: true) variables.append(key: 'CI_JOB_JWT_V1', value: jwt, public: false, masked: true) variables.append(key: 'CI_JOB_JWT_V2', value: jwt_v2, public: false, masked: true) + + id_tokens.map do |token_variable_key, token_config| + token_jwt = Gitlab::Ci::JwtV2.for_build(self, aud: token_config.dig('id_token', 'aud')) + + variables.append(key: token_variable_key, value: token_jwt, public: false, masked: true) + end rescue OpenSSL::PKey::RSAError, Gitlab::Ci::Jwt::NoSigningKeyError => e Gitlab::ErrorTracking.track_exception(e) end diff --git a/ee/app/models/concerns/ee/ci/metadatable.rb b/ee/app/models/concerns/ee/ci/metadatable.rb index 77fc2ba264cfed..b516cd8c77f9f1 100644 --- a/ee/app/models/concerns/ee/ci/metadatable.rb +++ b/ee/app/models/concerns/ee/ci/metadatable.rb @@ -17,6 +17,14 @@ def secrets? def secrets=(value) ensure_metadata.secrets = value end + + def id_tokens? + !!metadata&.id_tokens? + end + + def id_tokens=(value) + ensure_metadata.id_tokens = value + end end end end diff --git a/ee/app/models/ee/ci/build.rb b/ee/app/models/ee/ci/build.rb index 3fe621862e698a..48d6c53a85b6d0 100644 --- a/ee/app/models/ee/ci/build.rb +++ b/ee/app/models/ee/ci/build.rb @@ -67,7 +67,7 @@ def clone_accessors end def extra_accessors - (super + %i[secrets]).freeze + (super + %i[secrets id_tokens]).freeze end end diff --git a/ee/app/presenters/ee/ci/build_runner_presenter.rb b/ee/app/presenters/ee/ci/build_runner_presenter.rb index ab03c6cc98d3a3..a2bf0e6edad436 100644 --- a/ee/app/presenters/ee/ci/build_runner_presenter.rb +++ b/ee/app/presenters/ee/ci/build_runner_presenter.rb @@ -22,12 +22,22 @@ def vault_server 'name' => 'jwt', 'path' => variable_value('VAULT_AUTH_PATH', 'jwt'), 'data' => { - 'jwt' => '${CI_JOB_JWT}', + 'jwt' => "${#{jwt_var_key}}", 'role' => variable_value('VAULT_AUTH_ROLE') }.compact } } end + + def jwt_var_key + id_token_var_key.presence || 'CI_JOB_JWT_V2' + end + + def id_token_var_key + id_tokens.keys.find do |key| + variable_value(key).present? + end + end end end end diff --git a/ee/lib/ee/gitlab/ci/config/entry/job.rb b/ee/lib/ee/gitlab/ci/config/entry/job.rb index e6cb8de013fd73..7f4045716a7058 100644 --- a/ee/lib/ee/gitlab/ci/config/entry/job.rb +++ b/ee/lib/ee/gitlab/ci/config/entry/job.rb @@ -18,7 +18,8 @@ module Job entry :secrets, ::Gitlab::Ci::Config::Entry::Secrets, description: 'Configured secrets for this job', - inherit: false + inherit: false, + default: {} end EE_ALLOWED_KEYS = %i[dast_configuration secrets].freeze diff --git a/ee/lib/ee/gitlab/ci/yaml_processor/result.rb b/ee/lib/ee/gitlab/ci/yaml_processor/result.rb index 3e4fc72e90a863..a8ea7634f4ef89 100644 --- a/ee/lib/ee/gitlab/ci/yaml_processor/result.rb +++ b/ee/lib/ee/gitlab/ci/yaml_processor/result.rb @@ -14,7 +14,8 @@ def build_attributes(name) super.deep_merge( { options: { dast_configuration: job[:dast_configuration] }.compact, - secrets: job[:secrets] + secrets: job[:secrets], + id_tokens: job[:id_tokens] }.compact ) end diff --git a/lib/gitlab/ci/jwt.rb b/lib/gitlab/ci/jwt.rb index d3e7210b820747..dbc86ef058c1c3 100644 --- a/lib/gitlab/ci/jwt.rb +++ b/lib/gitlab/ci/jwt.rb @@ -12,7 +12,7 @@ def self.for_build(build) self.new(build, ttl: build.metadata_timeout).encoded end - def initialize(build, ttl: nil) + def initialize(build, ttl: nil, aud: nil) @build = build @ttl = ttl end diff --git a/lib/gitlab/ci/jwt_v2.rb b/lib/gitlab/ci/jwt_v2.rb index 4e01688a955b77..bd5e0ad3aa0616 100644 --- a/lib/gitlab/ci/jwt_v2.rb +++ b/lib/gitlab/ci/jwt_v2.rb @@ -3,13 +3,25 @@ module Gitlab module Ci class JwtV2 < Jwt + def self.for_build(build, aud: Settings.gitlab.base_url) + new(build, ttl: build.metadata_timeout, aud: aud).encoded + end + + def initialize(build, ttl: nil, aud:) + super + + @aud = aud + end + private + attr_reader :aud + def reserved_claims super.merge( iss: Settings.gitlab.base_url, sub: "project_path:#{project.full_path}:ref_type:#{ref_type}:ref:#{source_ref}", - aud: Settings.gitlab.base_url + aud: aud ) end end -- GitLab