Skip to content

[Mailer] Add configuration for dkim and smime signers #58501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Feb 10, 2025
Merged

[Mailer] Add configuration for dkim and smime signers #58501

merged 1 commit into from
Feb 10, 2025

Conversation

eliasfernandez
Copy link
Contributor

@eliasfernandez eliasfernandez commented Oct 8, 2024 •
edited
Loading

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #53941
License MIT

DKIM is becoming a must have for emails but the current workflow to sign messages on symfony requires to do it in the controller or the application and it causes failures when the email is signed before some other listener needs to get it.

This PR adds the needed configuration as part of the mailer configuration and sign all the messages if configured at the mailer level. By instance, we can add these two new blocks under mailer.yml:

framework:
    mailer:
        dsn: '%env(MAILER_DSN)%'
        dkim_signer:
            key: 'file://private.key'
            domain: 'symfony.com'
            select: 's1'
            passphrase: ''
            options:

or

framework:
    mailer:
        dsn: '%env(MAILER_DSN)%'
        smime_signer:
            key: '/path/to/certificate-private-key.key'
            certificate: '/path/to/certificate.crt'
            passphrase: ''
            extra_certificates: '/path/to/certificate.crt'
            sign_options: 1

@carsonbot carsonbot added this to the 7.2 milestone Oct 8, 2024
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 7.2 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@eliasfernandez eliasfernandez force-pushed the bugfix/53941-dkim-and-smime-signer-as-configuration branch 4 times, most recently from d749ca4 to a1242f1 Compare October 8, 2024 16:38
stof
stof reviewed Oct 8, 2024
View reviewed changes
@eliasfernandez eliasfernandez force-pushed the bugfix/53941-dkim-and-smime-signer-as-configuration branch from a1242f1 to 0765515 Compare October 9, 2024 13:18
nicolas-grekas
nicolas-grekas reviewed Oct 9, 2024
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't miss updating the XSD file also.

@nicolas-grekas nicolas-grekas changed the title Bugfix/53941 dkim and smime signer as configuration [FrameworkBundle][Mailer] Add configuration for dkim and smime signers Oct 9, 2024
@eliasfernandez eliasfernandez force-pushed the bugfix/53941-dkim-and-smime-signer-as-configuration branch from 0765515 to 0a0611c Compare October 9, 2024 14:31
@eliasfernandez eliasfernandez force-pushed the bugfix/53941-dkim-and-smime-signer-as-configuration branch 9 times, most recently from ead8485 to f514909 Compare October 10, 2024 07:28
stof
stof reviewed Oct 10, 2024
View reviewed changes
@fabpot fabpot added this to the 7.3 milestone Nov 20, 2024
@stof stof mentioned this pull request Jan 7, 2025
Open
@stof stof mentioned this pull request Feb 4, 2025
Open
stof
stof reviewed Feb 4, 2025
View reviewed changes
@eliasfernandez eliasfernandez force-pushed the bugfix/53941-dkim-and-smime-signer-as-configuration branch 2 times, most recently from c7f41d7 to 005a990 Compare February 5, 2025 19:40
@eliasfernandez eliasfernandez requested a review from stof February 5, 2025 19:45
@eliasfernandez eliasfernandez force-pushed the bugfix/53941-dkim-and-smime-signer-as-configuration branch from 005a990 to eea246e Compare February 5, 2025 19:47
fabpot
fabpot requested changes Feb 7, 2025
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
I've made some rewording suggestions.

@carsonbot carsonbot changed the title [FrameworkBundle][Mailer] Add configuration for dkim and smime signers Add configuration for dkim and smime signers Feb 7, 2025
@carsonbot carsonbot changed the title Add configuration for dkim and smime signers [Mailer] Add configuration for dkim and smime signers Feb 7, 2025
@eliasfernandez eliasfernandez requested a review from fabpot February 7, 2025 12:36
@eliasfernandez eliasfernandez force-pushed the bugfix/53941-dkim-and-smime-signer-as-configuration branch 2 times, most recently from f8df5f6 to eb6c711 Compare February 7, 2025 12:40
@fabpot fabpot force-pushed the bugfix/53941-dkim-and-smime-signer-as-configuration branch from eb6c711 to 244cbf1 Compare February 10, 2025 09:33
@fabpot
Copy link
Member

fabpot commented Feb 10, 2025

Thank you @eliasfernandez.

@fabpot fabpot merged commit 3918524 into symfony:7.3 Feb 10, 2025
4 of 5 checks passed
@fabpot fabpot mentioned this pull request Feb 10, 2025
Closed
@Spomky Spomky mentioned this pull request Feb 21, 2025
Merged
fabpot added a commit that referenced this pull request Mar 26, 2025
@fabpot
feature #59831 [Mailer][Mime] Refactor S/MIME encryption handling in …
7100c7b 
…`SMimeEncryptionListener` (Spomky)

This PR was merged into the 7.3 branch.

Discussion
----------

[Mailer][Mime] Refactor S/MIME encryption handling in `SMimeEncryptionListener`

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | yes
| New feature?  | yes
| Deprecations? | no
| Issues        |
| License       | MIT

It appears that the smime_encrypter introduced in #58501 is incorrect, as the email is encrypted only for the sender instead of being encrypted per recipient.
This PR introduces a new `SmimeCertificateRepositoryInterface`, responsible for retrieving recipient certificates.

An email is encrypted under the following conditions:
* A certificate is found for all recipients.
* The custom header `X-SMime-Encrypt` is present.
If either of these conditions is not met, the email is sent unencrypted.

Commits
-------

7c76c54 Refactor S/MIME encrypter to use certificate repository
@fabpot fabpot mentioned this pull request May 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas Awaiting requested review from nicolas-grekas

@alexandre-daubois alexandre-daubois Awaiting requested review from alexandre-daubois

@OskarStark OskarStark Awaiting requested review from OskarStark

@stof stof Awaiting requested review from stof

Assignees
No one assigned
Labels
Feature Mailer Status: Reviewed
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

[RFC] Email signing (and encryption) workflow
8 participants
@eliasfernandez @carsonbot @fabpot @nicolas-grekas @stof @OskarStark @alexandre-daubois @elias-playfinder