ix-ai / smtp

This is a SMTP docker container for sending emails. You can also relay emails to gmail and amazon SES.
MIT License
92 stars 14 forks source link

Document DKIM support and fix permissions bug #27

Closed yanokwa closed 1 year ago

yanokwa commented 1 year ago

Closes #26.

I've confirmed that the instructions and bash if block works on my container, but I have not tested a fresh end-to-end configuration.

tlex commented 1 year ago

Thank you for the pull request.

I particularly appreciate the enrichment of the documentation.

I would prefer however, instead of changing permissions on the fly, to have the file directly with the right permissions.

For this to work, the file should have group id 101 and permissions 640 (assuming userns is disabled):

root@4230594df42e:/# id Debian-exim
uid=101(Debian-exim) gid=101(Debian-exim) groups=101(Debian-exim)

My suggestion, therefore, is to add two extra steps to the documentation (after generating the keys):

chown :101 rsa.private
chmod 640 rsa.private
yanokwa commented 1 year ago

Thanks for the fast review and for maintaining this useful container!

I had considered the approach you suggested and decided against it because openssl writes rsa.private with 600 (as recommended at http://linuxcommand.org/lc3_man_pages/ssh1.html). Given that, weakening the permissions outside the container seems unnecessarily less secure, no?

tlex commented 1 year ago

Hi again,

Thinking about this:

Given that, weakening the permissions outside the container seems unnecessarily less secure, no?

The approach with the bind mount will actually change the permissions of the file on disk, if implemented like this.

I'll make a suggestion to the code, to handle this differently. Basically, if a certain file exists at start, to copy it with the right permissions inside the container.

yanokwa commented 1 year ago

I've force pushed some further changes to make it easier for users to implement. Just one mount and one ENV variable.

I don't love appending .temp to the name, but it at least gives users a sense that the file isn't used as is.

tlex commented 1 year ago

Thank you again, I'm quite happy with the solution.

tlex commented 1 year ago

Released in v0.5.0