From 89e48d71f4ca600fcd645792c26900f2f41ecff0 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Mon, 25 Nov 2024 16:37:37 +1100 Subject: [PATCH 1/4] Make note stand out a little more --- spec/support/helpers/auth/dpop_token_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/support/helpers/auth/dpop_token_helper.rb b/spec/support/helpers/auth/dpop_token_helper.rb index bb282fe3f86120..748b8b4ddd21a2 100644 --- a/spec/support/helpers/auth/dpop_token_helper.rb +++ b/spec/support/helpers/auth/dpop_token_helper.rb @@ -9,8 +9,8 @@ module DpopTokenHelper DpopProof = Struct.new(:ssh_public_key, :public_key_in_jwk, :openssl_private_key, :fingerprint, :proof) def generate_dpop_proof_for(user, alg: VALID_ALG, typ: VALID_TYP, kty: VALID_KTY, fingerprint: nil) - # `ssh_public_key` and `ssh_private_key` are not real secrets. They are a - # key pair generated solely for testing. + # NOTE: `ssh_public_key` and `ssh_private_key` are not real secrets. + # They are a key pair generated solely for testing. # ssh_public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC1ZgRbeixURy9/HxU5r5O3Xobnw1bmQx3dyFMkRLMFCy" \ "8aVkBvMw6CAc+81miOv+Sg/CZA2DKBAiEz0YwgPlD32o0q/OR5JdFAMH7e5IObm/4wr8dqm4JDE6eZ6f" \ -- GitLab From 866fb9204de7ca04a70a3fc4ff6922f28eb9c539 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Mon, 25 Nov 2024 16:38:17 +1100 Subject: [PATCH 2/4] Allow some test data to be injected --- spec/support/helpers/auth/dpop_token_helper.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/spec/support/helpers/auth/dpop_token_helper.rb b/spec/support/helpers/auth/dpop_token_helper.rb index 748b8b4ddd21a2..b5c576a63112b8 100644 --- a/spec/support/helpers/auth/dpop_token_helper.rb +++ b/spec/support/helpers/auth/dpop_token_helper.rb @@ -8,11 +8,13 @@ module DpopTokenHelper DpopProof = Struct.new(:ssh_public_key, :public_key_in_jwk, :openssl_private_key, :fingerprint, :proof) - def generate_dpop_proof_for(user, alg: VALID_ALG, typ: VALID_TYP, kty: VALID_KTY, fingerprint: nil) + def generate_dpop_proof_for( + user, ssh_public_key: nil, alg: VALID_ALG, typ: VALID_TYP, + kty: VALID_KTY, fingerprint: nil, ath: nil, public_key_in_jwk: nil) # NOTE: `ssh_public_key` and `ssh_private_key` are not real secrets. # They are a key pair generated solely for testing. # - ssh_public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC1ZgRbeixURy9/HxU5r5O3Xobnw1bmQx3dyFMkRLMFCy" \ + ssh_public_key ||= "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC1ZgRbeixURy9/HxU5r5O3Xobnw1bmQx3dyFMkRLMFCy" \ "8aVkBvMw6CAc+81miOv+Sg/CZA2DKBAiEz0YwgPlD32o0q/OR5JdFAMH7e5IObm/4wr8dqm4JDE6eZ6f" \ "eO+0tFwlrPnV8oiymw4SXeJLJf0n9f7HhH7xJJdWOOQZ2Ku/KMuNdf0aWYhbywUFWN4k5JtwCdEBxZYM" \ "NqRYv28i76j3rTm7hBMyor7B2+3lPfeUQpTJkW1UBwUDAYeKZAl6HgZPE9DmcaDSRVViLErp00/iaQSs" \ @@ -66,19 +68,21 @@ def generate_dpop_proof_for(user, alg: VALID_ALG, typ: VALID_TYP, kty: VALID_KTY openssl_private_key = OpenSSL::PKey::RSA.new(ssh_private_key) fingerprint ||= create_fingerprint(keys.find_by(key: ssh_public_key).key) - public_key_in_jwk = { + public_key_in_jwk ||= { kty: kty, n: Base64.urlsafe_encode64(openssl_private_key.n.to_s(2), padding: false), e: Base64.urlsafe_encode64(openssl_private_key.e.to_s(2), padding: false) } + ath ||= generate_ath(personal_access_token) + dpop_proof = create_dpop_proof( alg, typ, fingerprint, public_key_in_jwk, openssl_private_key, - ath: generate_ath(personal_access_token) + ath: ath ) DpopProof.new(ssh_public_key, public_key_in_jwk, openssl_private_key, fingerprint, dpop_proof) -- GitLab From a5577f98602ec1ba207043ed2666423f9d3a92f2 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Mon, 25 Nov 2024 16:38:36 +1100 Subject: [PATCH 3/4] New DpopTokenUser class --- lib/gitlab/auth/dpop_token_user.rb | 120 ++++++++++++++++++ spec/lib/gitlab/auth/dpop_token_user_spec.rb | 123 +++++++++++++++++++ 2 files changed, 243 insertions(+) create mode 100644 lib/gitlab/auth/dpop_token_user.rb create mode 100644 spec/lib/gitlab/auth/dpop_token_user_spec.rb diff --git a/lib/gitlab/auth/dpop_token_user.rb b/lib/gitlab/auth/dpop_token_user.rb new file mode 100644 index 00000000000000..dc1c8d25f00432 --- /dev/null +++ b/lib/gitlab/auth/dpop_token_user.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +# Demonstrated Proof of Possession (DPoP) is a mechanism to tie a user's +# Personal Access Token (PAT) to one of their signing keys. +# +# A DPoP Token is a signed JSON Web Token. This class implements +# the logic to ensure a provided DPoP Token is well-formed, cryptographically +# signed and belongs to the provided user. +# +module Gitlab + module Auth + class DpopTokenUser + SUPPORTED_JWS_ALGORITHMS = { 'ssh-rsa' => 'RS512' }.freeze + SUPPORTED_TYPES = ['dpop+jwt'].freeze + SUPPORTED_KEY_TYPES = ['RSA'].freeze + SUPPORTED_PROOF_KEY_ID_HASHING_ALGORITHMS = ['SHA256'].freeze + + def initialize(token:, user:, personal_access_token_plaintext:) + @token = token + @user = user + @personal_access_token_plaintext = personal_access_token_plaintext + end + + def validate! + token.validate! + pat_belongs_to_user! + valid_token_for_user! + valid_access_token_hash! + end + + private + + attr_reader :token, :user, :personal_access_token_plaintext + + def pat_belongs_to_user! + return if user.personal_access_tokens.active.find_by_token(personal_access_token_plaintext).present? + + raise Gitlab::Auth::DpopValidationError, 'Personal access token does not belong to the requesting user' + end + + # Check that the DPoP is signed with a SSH key belonging to the user + def valid_token_for_user! + user_public_key = signing_key_for_user! + algorithm = algorithm_for_dpop_validation(user_public_key) + openssh_public_key = convert_public_key_to_openssh_key!(user_public_key) + + # Decode the JSON token again, this time with the key, + # the expected algorthm, verifying all the timestamps, etc + # Overwrites the attrs, in case .decode returns a different result + # when verify is true. + payload, header = JWT.decode( + token.data, + openssh_public_key, + true, + { + required_claims: %w[exp ath iat], + algorithm: algorithm, + verify_iat: true + } + ) + + raise Gitlab::Auth::DpopValidationError, 'Unable to decode JWT' if payload.nil? || header.nil? + + jwk = header['jwk'] + + begin + unless openssh_public_key.to_s == OpenSSL::PKey.read(JWT::JWK::RSA.import(jwk).public_key.to_pem).to_s + raise 'Failed to parse JWK: invalid JWK' + end + rescue StandardError => e + raise Gitlab::Auth::DpopValidationError, e + end + end + + def signing_key_for_user! + # Gets a signing key from the user based on the fingerprint. + fingerprint = token.header['kid']&.delete_prefix('SHA256:') + + key = user.keys.signing.find_by_fingerprint_sha256(fingerprint)&.key + raise Gitlab::Auth::DpopValidationError, "No matching key found" unless key + + # Validate the signing key uses a supported algorithm. + algorithm = key.split(' ').first + + return key if algorithm.casecmp?('ssh-rsa') + + raise Gitlab::Auth::DpopValidationError, 'Currently only RSA keys are supported' + end + + # Finds the algorithm from the public key to decode the JWT in + # valid_for_user! + def algorithm_for_dpop_validation(key) + SUPPORTED_JWS_ALGORITHMS.each do |key_algorithm, jwt_algorithm| + return jwt_algorithm if key.start_with?(key_algorithm) + end + nil + end + + def convert_public_key_to_openssh_key!(key) + SSHData::PublicKey.parse_openssh(key).openssl + rescue SSHData::DecodeError => e + raise Gitlab::Auth::DpopValidationError, "Unable to parse public key. #{e.message}" + end + + # Check that the DPoP contains a hash of the PAT being used. + # Users can have multiple PATs, so we still need to check that + # they created this DPoP for this particular PAT. + def valid_access_token_hash! + expected_hash = Base64.urlsafe_encode64( + Digest::SHA256.digest(personal_access_token_plaintext), + padding: false + ) + + return if ActiveSupport::SecurityUtils.secure_compare(token.payload['ath'], expected_hash) + + raise Gitlab::Auth::DpopValidationError, 'Incorrect access token hash in JWT' + end + end + end +end diff --git a/spec/lib/gitlab/auth/dpop_token_user_spec.rb b/spec/lib/gitlab/auth/dpop_token_user_spec.rb new file mode 100644 index 00000000000000..e3638d1f8242cb --- /dev/null +++ b/spec/lib/gitlab/auth/dpop_token_user_spec.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Auth::DpopTokenUser, feature_category: :system_access do + include Auth::DpopTokenHelper + + let_it_be(:user, freeze: true) { create(:user) } + let_it_be(:personal_access_token, freeze: true) { create(:personal_access_token, user: user) } + + let(:personal_access_token_plaintext) { personal_access_token.token } + + let(:ssh_public_key) { nil } + let(:ath) { nil } + let(:public_key_in_jwk) { nil } + let(:dpop_proof) do + generate_dpop_proof_for(user, ssh_public_key: ssh_public_key, ath: ath, public_key_in_jwk: public_key_in_jwk) + end + + let(:dpop_token) do + Gitlab::Auth::DpopToken.new(data: dpop_proof.proof) + end + + describe '#validate!' do + subject(:validate!) do + described_class.new(token: dpop_token, user: user, + personal_access_token_plaintext: personal_access_token_plaintext).validate! + end + + context 'when the token is valid' do + it 'initializes with valid token' do + expect { validate! }.not_to raise_error + end + end + + context "when input isn't valid" do + context 'when the DPoP token is invalid' do + let(:dpop_token) { Gitlab::Auth::DpopToken.new(data: 'invalid') } + + it 'raises DpopValidationError' do + expect do + validate! + end.to raise_error(Gitlab::Auth::DpopValidationError, + /Malformed JWT, unable to decode. Not enough or too many segments/) + end + end + + context "when the PAT doesn't belong to the user" do + let(:personal_access_token_plaintext) { 'invalid' } + + it 'raises DpopValidationError' do + expect do + validate! + end.to raise_error(Gitlab::Auth::DpopValidationError, + /Personal access token does not belong to the requesting user/) + end + end + + context "when the DPoP token isn't valid for the user" do + context "when the jwk value is malformed" do + let(:public_key_in_jwk) { { kty: Auth::DpopTokenHelper::VALID_KTY } } + + it 'raises DpopValidationError' do + expect do + validate! + end.to raise_error(Gitlab::Auth::DpopValidationError, + /Key format is invalid for RSA/) + end + end + + context "when the jwk value is invalid" do + let(:public_key_in_jwk) { { kty: Auth::DpopTokenHelper::VALID_KTY, n: '', e: '' } } + + it 'raises DpopValidationError' do + expect do + validate! + end.to raise_error(Gitlab::Auth::DpopValidationError, + /Failed to parse JWK: invalid JWK/) + end + end + end + + context 'when the access token hash is incorrect' do + let(:ath) { 'incorrect' } + + it 'raises DpopValidationError' do + expect do + validate! + end.to raise_error(Gitlab::Auth::DpopValidationError, + /Incorrect access token hash in JWT/) + end + end + + context 'when the SSH public key is invalid' do + it 'raises DpopValidationError' do + allow(SSHData::PublicKey).to receive(:parse_openssh) + .with(dpop_proof.ssh_public_key) + .and_raise(SSHData::DecodeError) + + expect do + validate! + end.to raise_error(Gitlab::Auth::DpopValidationError, + /Unable to parse public key/) + end + end + + context 'when then SSH public key is unsupported' do + let(:ssh_public_key) do + # rubocop:disable Layout/LineLength -- Value is not important + 'ssh-dss AAAAB3NzaC1kc3MAAACBAIOcJlrYLOHfYxRY/i5nw2vcQZ8QpkNLObfjEBl5DsTWCXbKkkbNIkABpMjpH22nxJxzqolyiG8hwIRvtPmwUd3o4x+kvaWXmabPQhs6xDlHMV30ZXwliT5qjb04AkBtH1QH1wz6e9tEAMlUi7pCU76NREWjTjc30s+NmOIqItBhAAAAFQDhJ3PVkV2ytA24pZkr6QMppFzPxQAAAIBF4oicI5FBc0w4C8USL37NIa0uxrAPQ/Zkz3b0hdNNAmnNFjN9PihGrsTURa9zBgpV5tM2LfvS9qlAgnrbu7VY8NV0OSTxXeUfkn+k40DspHfY0sl8IKGvCYt0uO3tpfu0lZOJFgr/vFc4ODzE5QEm9eKMsWX8SJbRuXaGMn0myQAAAIBSzNgHW/lU/BgycBefJpe1NGnVVGRBPI9QHjxh/6HvyFHYc2N506wPDiRXyX03QREoUe4VXMWbOoFHpjZWX9dhwvvg3vBBQMeH7I5V0o5sEXbvdgtXBDtoiZlbZSiSC9wvw4c7rwSWsfm+iF1Ub1XPf2ALwh/BWHJzb93viCePcg==' + # rubocop:enable Layout/LineLength + end + + it 'raises DpopValidationError' do + expect do + validate! + end.to raise_error(Gitlab::Auth::DpopValidationError, + /Currently only RSA keys are supported/) + end + end + end + end +end -- GitLab From 1d75d942c5f79ea8f8abb0e635fbbd631006965d Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Mon, 9 Dec 2024 15:35:18 +1100 Subject: [PATCH 4/4] Streamline fingerprint generation --- spec/support/helpers/auth/dpop_token_helper.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/support/helpers/auth/dpop_token_helper.rb b/spec/support/helpers/auth/dpop_token_helper.rb index b5c576a63112b8..86ab29b9aa70bc 100644 --- a/spec/support/helpers/auth/dpop_token_helper.rb +++ b/spec/support/helpers/auth/dpop_token_helper.rb @@ -62,11 +62,9 @@ def generate_dpop_proof_for( "\ny1Y0tD9WVuVwFMEfkENQzOEJxVHwQpsxBRQ5snustS/HmrF5SIZyeg==" \ "\n-----END RSA PRIVATE KEY-----" - user.keys.create!(title: "Sample key #{user.id}", key: ssh_public_key) - - keys = user.keys.signing + key = user.keys.create!(title: "Sample key #{user.id}", key: ssh_public_key) + fingerprint ||= create_fingerprint(key.key) openssl_private_key = OpenSSL::PKey::RSA.new(ssh_private_key) - fingerprint ||= create_fingerprint(keys.find_by(key: ssh_public_key).key) public_key_in_jwk ||= { kty: kty, -- GitLab