Skip to content
Snippets Groups Projects

feat: smime signed notification emails

All threads resolved!

What does this MR do?

Add a new optional ActionMailer interceptor that signs all outgoing emails with SMIME

  • I checked this configuration working against a dummy gmail account with actual SMTP delivery in gdk; Gmail web and native Outlook on macOS were ok with the SMIME signing.
  • We have identified that letter_opener_web does not play nice with the signed emails, so they cannot currently be checked in development. Opened https://github.com/fgrehm/letter_opener_web/issues/96 to track this (check the attached screenshots in there).
  • Per guidelines, this MR will require another Omnibus MR to add the new configuration options (I'd also say they need to be added to gdk? though this is not documented in https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/doc/settings/gitlab.yml.md)

Corresponding MR on omnibus for the new smime configuration params: omnibus-gitlab!3514 (merged)

Issue for adding new config parameters to k8s chart: https://gitlab.com/charts/gitlab/issues/1533

Issue for supporting eliptic curve keys: https://gitlab.com/gitlab-org/gitlab-ce/issues/66439

The development of this MR is sponsored by siemens (/cc @bufferoverflow).

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 Diego Louzán

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
  • Diego Louzán added 1 commit

    added 1 commit

    • 2d6975c6 - add specs for certificate and interceptor

    Compare with previous version

  • Diego Louzán
  • @dlouzan thanks! Left some comments. This is not something I'm very familiar with, so I'm not sure how useful they are! :sweat_smile:

  • assigned to @dlouzan

  • @dlouzan we may also want to wait until Product evaluates this, just in case this is found not to be useful for some reason (as there is no issue for it). cc @ebrinkman

  • Author Contributor

    @jameslopez Thanks, your suggestion about the kubernetes code was really helpful; I'll add some extra tests, and hopefully this will be checked soon.

  • Diego Louzán changed the description

    changed the description

  • Diego Louzán added 1 commit

    added 1 commit

    • 8ed2ad3c - refactor sign logic into own class and improve specs

    Compare with previous version

  • Diego Louzán added 1 commit

    added 1 commit

    • 81895fcb - refactor sign logic into own class and improve specs

    Compare with previous version

  • Author Contributor

    @ebrinkman Any news? Thanks!

  • Author Contributor

    @jameslopez Is there any way we can follow this up or raise its awareness? I'm sure there will be changes required and I'd prefer to do that while the code is fresh on my mind :-)

  • This is labelled as ~"group::team planning" so I'm gonna ping @gweaver here to see if this gets some attention.

    As there is no associated issue so this was never planned, it may not be a priority to look at, so I apologise in advance!

    Edited by James Lopez
  • Diego Louzán marked the checklist item Code review guidelines as completed

    marked the checklist item Code review guidelines as completed

  • Diego Louzán marked the checklist item Tested in all supported browsers N/A as completed

    marked the checklist item Tested in all supported browsers N/A as completed

  • Diego Louzán changed the description

    changed the description

  • Diego Louzán marked the checklist item Merge request performance guidelines as completed

    marked the checklist item Merge request performance guidelines as completed

  • Diego Louzán marked the checklist item Style guides as completed

    marked the checklist item Style guides as completed

  • Diego Louzán marked the checklist item Database guides n/a as completed

    marked the checklist item Database guides n/a as completed

  • Diego Louzán marked the checklist item Separation of EE specific content n/a as completed

    marked the checklist item Separation of EE specific content n/a as completed

  • Sean McGivern changed milestone to %12.2

    changed milestone to %12.2

  • Sean McGivern assigned to @jameslopez and unassigned @dlouzan

    assigned to @jameslopez and unassigned @dlouzan

  • Thanks @dlouzan - left some comments and suggestions :)

  • James Lopez assigned to @dlouzan and unassigned @jameslopez

    assigned to @dlouzan and unassigned @jameslopez

  • Diego Louzán added 1 commit

    added 1 commit

    Compare with previous version

  • Diego Louzán added 1 commit

    added 1 commit

    Compare with previous version

  • Diego Louzán added 1 commit

    added 1 commit

    Compare with previous version

  • Diego Louzán
  • Author Contributor

    @jameslopez I have addressed all your comments, and left some of my own, please review.

  • Diego Louzán added 1 commit

    added 1 commit

    Compare with previous version

  • Thanks @dlouzan! - left some more comments.

  • Diego Louzán changed the description

    changed the description

  • Diego Louzán added 1 commit

    added 1 commit

    Compare with previous version

  • Diego Louzán resolved all threads

    resolved all threads

  • Diego Louzán added 1 commit

    added 1 commit

    Compare with previous version

  • James Lopez resolved all threads

    resolved all threads

  • James Lopez assigned to @smcgivern and unassigned @dlouzan

    assigned to @smcgivern and unassigned @dlouzan

  • Sean McGivern assigned to @dlouzan and unassigned @smcgivern

    assigned to @dlouzan and unassigned @smcgivern

  • Diego Louzán added 1 commit

    added 1 commit

    Compare with previous version

  • Diego Louzán added 1 commit

    added 1 commit

    Compare with previous version

  • Diego Louzán added 1760 commits

    added 1760 commits

    Compare with previous version

  • Diego Louzán changed the description

    changed the description

  • Diego Louzán added 1 commit

    added 1 commit

    Compare with previous version

  • Diego Louzán added 1 commit

    added 1 commit

    • f6734ea8 - 1 commit from branch gitlab-org:master

    Compare with previous version

  • Diego Louzán added 1 commit

    added 1 commit

    • 6a70d792 - 1 commit from branch gitlab-org:master

    Compare with previous version

  • Diego Louzán added 1 commit

    added 1 commit

    Compare with previous version

  • Diego Louzán added 1 commit

    added 1 commit

    Compare with previous version

  • mentioned in merge request omnibus-gitlab!3514 (merged)

  • Diego Louzán changed the description

    changed the description

  • assigned to @estrike

  • Diego Louzán changed the description

    changed the description

  • Diego Louzán added 1 commit

    added 1 commit

    • 0dcb9d21 - feat: SMIME signed notification emails

    Compare with previous version

  • James Ritchey
  • James Ritchey approved this merge request

    approved this merge request

  • mentioned in issue #66439 (moved)

  • Diego Louzán resolved all threads

    resolved all threads

  • Author Contributor

    @smcgivern All threads resolved and got an approval, I think we are good to go :fire:

  • Sean McGivern changed milestone to %12.3

    changed milestone to %12.3

  • Sean McGivern assigned to @smcgivern and unassigned @dlouzan and @estrike

    assigned to @smcgivern and unassigned @dlouzan and @estrike

  • merged

  • Sean McGivern mentioned in commit 842b4d4a

    mentioned in commit 842b4d4a

  • Diego Louzán resolved all threads

    resolved all threads

  • Diego Louzán mentioned in merge request !32146 (merged)

    mentioned in merge request !32146 (merged)

  • Diego Louzán changed the description

    changed the description

  • julian mentioned in issue gitlab#5060

    mentioned in issue gitlab#5060

  • Diego Louzán mentioned in merge request gitlab!18021 (merged)

    mentioned in merge request gitlab!18021 (merged)

  • Diego Louzán mentioned in merge request gitlab!29352 (merged)

    mentioned in merge request gitlab!29352 (merged)

  • Diego Louzán mentioned in merge request gitlab!76233 (merged)

    mentioned in merge request gitlab!76233 (merged)

  • mentioned in issue gitlab#5816

  • Please register or sign in to reply
    Loading