Skip to content
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.

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

Custom DKIM selector #2348

Open
bilogic opened this issue Jan 4, 2024 · 7 comments
Open

Custom DKIM selector #2348

bilogic opened this issue Jan 4, 2024 · 7 comments

Comments

@bilogic
Copy link
Contributor

bilogic commented Jan 4, 2024

Hi!

Is it possible to specify another DKIM selector other than mail?

@bilogic
Copy link
Contributor Author

bilogic commented Jan 4, 2024

I had a quick look over the source code, seems easy. However, some questions as I'm not familiar with python

  1. Can the python code already read values from etc/mailinabox.conf? Seems to be the env variable.
  2. I think adding a DKIM_SELECTOR=mail as default
  3. The PR will likely touch 2 scripts, test_dns.py and dns_update.py, ok?

@bilogic
Copy link
Contributor Author

bilogic commented Jan 4, 2024

I have some commits here main...bilogic:mailinabox:custom-dkim-selector, apologies that it is mixed in with a configurable TTL feature.

The main change is on line 792 of dns_update.py where I replaced mail with {selector}

My problem is, why does the /admin#external_dns still show mail._domainkey...? Where is the code to make the page reflect the custom selector?

@myfirstnameispaul
Copy link
Contributor

#2220 replaces OpenDKIM with dkimpy. My recommendation would be to work with @kiekerjan as your changes seem reasonable to me as I have issues with using mail as a selector because there are commercial services using the same selector so there is a potential to be in conflict. Better would be either configurable selector or at least something unique.

Then, ideally, this stimulates getting the PR merged.

@kiekerjan
Copy link
Contributor

kiekerjan commented Jan 4, 2024

Look at the dkim.sh script in the setup directory. There the default dns entry is generated.
Because this is done only at initial setup, you need to include some logic to generate a new mail.txt on every run of the setup, or include generating the dns record in dns_update.py

Also, in start.sh you might want to use something like DKIM_SELECTOR=${DEFAULT_DKIM_SELECTOR:-mail} Your current code will always use mail as selector (and overwrite anything else)

@bilogic
Copy link
Contributor Author

bilogic commented Jan 4, 2024

Just finished a hardware migration. Thank you for the info, let me find some time to review them.

But 1 quick qn first, is the maintainer open to accepting dkimpy first?

@bilogic
Copy link
Contributor Author

bilogic commented Jan 5, 2024

python and bash are like my 5th language 🤣

@myfirstnameispaul

Definitely configurable (and not just something different). Since we are going to change it, let's provide an end-all solution. From the looks of it, this is not a very big change here, so if and when we switch to dkimpy, I would be happy to help.

@kiekerjan

  1. I'm thinking of asking for a selector at initial setup and it should not be touched again, trying to accomplish this with minimum code rather than introduce the same logic in different parts of the code, or do you have some ideas? Do share! The use of DEFAULT_DKIM_SELECTOR feels sufficiently configurable yet simple to me (aligned with aims of MIAB). What do you think?
  2. I made the change in start.sh, but where does $DEFAULT_DKIM_SELECTOR come from? All lines in an existing mailinbox.conf are prefixed with DEFAULT_

Thanks!

@bilogic
Copy link
Contributor Author

bilogic commented Jan 5, 2024

My new branch with the key bits here b51550d

Please help me to review. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants