Skip to content
Snippets Groups Projects

feat: x509 signed commits using openssl

Merged Roger Meier requested to merge siemens/gitlab:feat/x509-signed-commits into master

What does this MR do?

This MR provides basic support for x509 signed commits as proposed within #24512 (closed) and #29782 (closed).

Main difference in comparision to gpg:

  • trust anchor is the certificate authority not a verified key
  • no specific key uploaded by the user
  • verification status could be one of the following
    • verified: email within x509 certificate equals committer email and ca is trusted
    • unverified: email within x509 certificate does not match committer email or ca is not trusted or signature invalid
  • verification can be done using plain OpenSSL functionality or via gpgsm
    • my initial approach was using gpgsm, however the formats and concepts of x509 and gpg keys differ heavily and requires a lot of workarounds when using gpgsm, beside of the additional complexity at the code level gpp does require a lot of local configuration. This will make it error prone and reduce the operatability significantly
    • the openssl native approach is much simpler from an implementation, maintenance and operations point of view

Approach:

Further reading:

The MR we made to introduce GPG signed commits within GitLab:

Samples of SMIME signed commits:

:wrench: with :heart: at Siemens

/cc @dlouzan @henning-schild

Screenshots

Screenshot_2020-01-25_at_20.59.11

Screenshot_2020-01-25_at_20.58.55

Database

Does this MR meet the acceptance criteria?

Conformity

Performance 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 🤖 GitLab Bot 🤖

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Roger Meier added 1 commit

    added 1 commit

    • 6cf3bbaf - fix(commit-model): load signature_data only if project is defined

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • 5eb4dfa2 - fix(commit-model): load signature_data only if project is defined

    Compare with previous version

  • Roger Meier resolved all threads

    resolved all threads

  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
    • Resolved by Roger Meier

      Thanks for submitting the MR!

      I skimmed over the HAML when I realized my knowledge of it is sadly lacking; we should probably get a frontend reviewer to take a look at that.

      One thing that I didn't see included here were any spec changes or additions; we'll want to add tests for the frontend additions as well as the new X509Signatures class.

  • Roger Meier added 1 commit

    added 1 commit

    • 0333f5a8 - refactor: change several sections based on reviewer input

    Compare with previous version

  • Roger Meier mentioned in merge request gitlab-test!40 (closed)

    mentioned in merge request gitlab-test!40 (closed)

  • Roger Meier changed the description

    changed the description

  • Roger Meier added 1 commit

    added 1 commit

    • 2cb92ed9 - docs(repository): add x509 signed commits doc

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • 122e9c4f - refactor: add user_id and cert issuer, display cert subject/issuer

    Compare with previous version

  • Roger Meier changed the description

    changed the description

  • Roger Meier added 1 commit

    added 1 commit

    • 4fce6ed9 - refactor: strong_memoize signature_type within commit model

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • c2d8fdea - fix: ensure safe navigation within signature_text and signed_text

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • 9bc0823c - docs: add changelog file for x509 signed commits

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • 1518ec14 - docs: add link text to get rid of bare url

    Compare with previous version

  • Roger Meier marked the checklist item Changelog entry as completed

    marked the checklist item Changelog entry as completed

  • Roger Meier added 1 commit

    added 1 commit

    • cb43488a - refactor: do not show fingerprint, limit to CN,OU,O attributes

    Compare with previous version

  • Roger Meier changed the description

    changed the description

  • Roger Meier added 1353 commits

    added 1353 commits

    • cb43488a...1bea9238 - 1336 commits from branch gitlab-org:master
    • 7fa3b8c8 - feat: x509 signed commits using openssl
    • 52910dd6 - chore: use system certificate store
    • e7ffeda7 - refactor: use dedicated haml templates for x509
    • a7a9f6ac - chore: update db/schema.rb
    • 199543d1 - refactor: remove unused code section from x509 badge
    • 4a37db1c - fix(commit-model): load signature_data only if project is defined
    • e91ea436 - refactor: change several sections based on reviewer input
    • 9369c8d3 - docs(repository): add x509 signed commits doc
    • 0973dee5 - refactor: add user_id and cert issuer, display cert subject/issuer
    • 4bd59490 - refactor: strong_memoize signature_type within commit model
    • e3df076c - fix: ensure safe navigation within signature_text and signed_text
    • 6478b296 - docs: add changelog file for x509 signed commits
    • 1afc3390 - docs: add link text to get rid of bare url
    • de56fd53 - refactor: do not show fingerprint, limit to CN,OU,O attributes
    • 65a61ab6 - fix: issues from rebase
    • b7fa3da7 - fix: x509 documentation link text
    • 5c070896 - feat: add x509 related text to gitlab.pot

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • 583af232 - refactor: remove keyUsage==Digital Signature enforcement

    Compare with previous version

  • Roger Meier changed the description

    changed the description

  • Henning Schild
  • Roger Meier added 1 commit

    added 1 commit

    • 681a2123 - fix: add second variant of pgp signature header

    Compare with previous version

  • Roger Meier changed the description

    changed the description

  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Roger Meier added 2 commits

    added 2 commits

    • 6a24591e - chore: move rubocop to individual line
    • e74b8c93 - style: add extra new line for better readability

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • 532b6a6f - fix: add return for :none case within signature_type

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • 7147e190 - refactor: simplify access to signature_text and signed_text

    Compare with previous version

  • Roger Meier added 2 commits

    added 2 commits

    • 6771f71b - fix: use break instead of return within signature_type
    • 70731f13 - refactor: use belongs_to :user instead of user method

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • 19f2132f - refactor: avoid & operator within initializer

    Compare with previous version

  • Roger Meier added 3 commits

    added 3 commits

    • 27bfa21f - fix: change from 5.1 to 5.2
    • af6f8bf7 - refactor: use next instead of break within strong_memoize
    • a27b956f - refactor: use belongs_to instead of a commit function

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • ef6868b2 - docs: use global instead of local, minor changes

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • 4d3c00f6 - refactor: introduce signed commit parent class

    Compare with previous version

  • Roger Meier changed the description

    changed the description

  • Roger Meier added 1 commit

    added 1 commit

    • 6e2b35e0 - refactor(smime): do not include OpenSSL to avoid name conflicts

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • 62563127 - refactor(smime): do not include OpenSSL to avoid name conflicts within specs

    Compare with previous version

  • Diego Louzán added 1 commit

    added 1 commit

    • bfb56cb3 - fix: smime specs fail because of new Gitlab::X509 module scoping

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • c96c462f - fix: only load signature if commit.project is defined

    Compare with previous version

  • Roger Meier added 8 commits

    added 8 commits

    • b2b52852 - feat: show user for unverified x509 signatures
    • 653511b3 - refactor: x509 class to be inline with gpg
    • fbb036a3 - fix: corner case for signature handling
    • 1457af47 - test: add initial x509 signed commit spec
    • 88534d67 - refactor: cleanup x509_signature
    • 75708ac2 - test: load trusted cert from x509 helper
    • 8a600f91 - feat: update x509 signature worker
    • b04a3e58 - test: add update x509 signature worker spec

    Compare with previous version

    • Author Contributor
      Resolved by Roger Meier

      @kerrizor I'm very sorry for the delay! However, I was able to work a bit on this and would appriciate if you could take a look at the current state. I've addeed specs and an update worker including specs now. Beside of this I did several refactorings.

  • Roger Meier added 1361 commits

    added 1361 commits

    • b04a3e58...80bd6f78 - 1318 commits from branch gitlab-org:master
    • 3cb89232 - feat: x509 signed commits using openssl
    • ad84091d - chore: use system certificate store
    • e6f81cf0 - refactor: use dedicated haml templates for x509
    • ad07afb7 - chore: update db/schema.rb
    • 9e40e85d - refactor: remove unused code section from x509 badge
    • 21bc6bf2 - fix(commit-model): load signature_data only if project is defined
    • 87b7d630 - refactor: change several sections based on reviewer input
    • 811846f5 - docs(repository): add x509 signed commits doc
    • 1dcbc9dd - refactor: add user_id and cert issuer, display cert subject/issuer
    • 1231a008 - refactor: strong_memoize signature_type within commit model
    • 4b6d0dbb - fix: ensure safe navigation within signature_text and signed_text
    • 33316d13 - docs: add changelog file for x509 signed commits
    • 894b7d73 - docs: add link text to get rid of bare url
    • c1c9894e - refactor: do not show fingerprint, limit to CN,OU,O attributes
    • 64716140 - fix: issues from rebase
    • 531bccc6 - fix: x509 documentation link text
    • 041924f8 - feat: add x509 related text to gitlab.pot
    • 117fdea0 - refactor: remove keyUsage==Digital Signature enforcement
    • 93c43b5e - fix: add second variant of pgp signature header
    • 6fecff4c - chore: move rubocop to individual line
    • b2aba906 - style: add extra new line for better readability
    • 77e9c32e - fix: add return for :none case within signature_type
    • 992a249e - refactor: simplify access to signature_text and signed_text
    • a2cb9386 - fix: use break instead of return within signature_type
    • 133d5fa6 - refactor: use belongs_to :user instead of user method
    • fecfe781 - refactor: avoid & operator within initializer
    • 531a0ee5 - fix: change from 5.1 to 5.2
    • fc23c92e - refactor: use next instead of break within strong_memoize
    • a97a1040 - refactor: use belongs_to instead of a commit function
    • 5c28e312 - docs: use global instead of local, minor changes
    • 8c185bf7 - refactor: introduce signed commit parent class
    • 5866a7ca - refactor(smime): do not include OpenSSL to avoid name conflicts
    • b3fdfefc - refactor(smime): do not include OpenSSL to avoid name conflicts within specs
    • 3f4aff32 - fix: smime specs fail because of new Gitlab::X509 module scoping
    • 5fca5e54 - fix: only load signature if commit.project is defined
    • 03dc5afe - feat: show user for unverified x509 signatures
    • c4270fb7 - refactor: x509 class to be inline with gpg
    • 29844f38 - fix: corner case for signature handling
    • 1ec1e859 - test: add initial x509 signed commit spec
    • dbf4878d - refactor: cleanup x509_signature
    • 2bbdb8fa - test: load trusted cert from x509 helper
    • 80c25f87 - feat: update x509 signature worker
    • 455bedf8 - test: add update x509 signature worker spec

    Compare with previous version

  • Author Contributor

    @kerrizor just rebased as well

  • Roger Meier added 1 commit

    added 1 commit

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    Compare with previous version

  • Kerri Miller
  • Kerri Miller
  • Kerri Miller
  • Roger Meier added 1 commit

    added 1 commit

    Compare with previous version

  • Roger Meier added 29 commits

    added 29 commits

    • 1007e4ee - fix: issues from rebase
    • 97f66642 - fix: x509 documentation link text
    • de5013e4 - feat: add x509 related text to gitlab.pot
    • 60e0dbc7 - refactor: remove keyUsage==Digital Signature enforcement
    • b2c05b13 - fix: add second variant of pgp signature header
    • 73f6fd4f - chore: move rubocop to individual line
    • 11d31295 - style: add extra new line for better readability
    • 2cf3fc43 - fix: add return for :none case within signature_type
    • 01d270f0 - refactor: simplify access to signature_text and signed_text
    • 791d21cc - fix: use break instead of return within signature_type
    • 1fd16f62 - refactor: use belongs_to :user instead of user method
    • a9929746 - refactor: avoid & operator within initializer
    • 2763ac71 - fix: change from 5.1 to 5.2
    • 029db131 - refactor: use next instead of break within strong_memoize
    • f48f287e - refactor: use belongs_to instead of a commit function
    • c90a65b1 - docs: use global instead of local, minor changes
    • 24c955c4 - refactor: introduce signed commit parent class
    • 59b81394 - refactor(smime): do not include OpenSSL to avoid name conflicts
    • 655af0b8 - refactor(smime): do not include OpenSSL to avoid name conflicts within specs
    • 6ac1fb34 - fix: smime specs fail because of new Gitlab::X509 module scoping
    • 58fb21e2 - fix: only load signature if commit.project is defined
    • 7af0cc4d - feat: show user for unverified x509 signatures
    • b5f7b8e3 - refactor: x509 class to be inline with gpg
    • fa8459cf - fix: corner case for signature handling
    • d6cc9a42 - test: add initial x509 signed commit spec
    • b552c1e1 - refactor: cleanup x509_signature
    • bf256691 - test: load trusted cert from x509 helper
    • 5b775484 - feat: update x509 signature worker
    • 2f512985 - test: add update x509 signature worker spec

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    • 5148b957 - refactor: move delegate to common place

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    Compare with previous version

  • Roger Meier added 1 commit

    added 1 commit

    Compare with previous version

  • Kerri Miller
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading