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

Chmod and chown whole opendkim key directory #15

Closed
wants to merge 1 commit into from

Conversation

wader
Copy link
Owner

@wader wader commented Mar 15, 2020

Looking at the source https://github.com/trusteddomainproject/OpenDKIM/blob/5c539587561785a66c1f67f720f2fb741f320785/opendkim/opendkim.c#L4695
it seems it will make sure all parents are secure which make sense.

Maybe fixes #14

@wader
Copy link
Owner Author

wader commented Mar 15, 2020

@hemberger should be ok? any idea why you didnt see this?

@hemberger
Copy link
Contributor

The comment in the code that you linked is:

 	/*
	**  Check each node in the tree to ensure that:
	**  1) The file itself is read-write only by the executing user and the
	**  	super-user;
	**  2) No directory above the file is writeable by anyone other than
	**  	the executing user and the super-user.
	*/

When I look in a container (using the current version on the master branch), the permissions already satisfy these criteria:

# cd /etc/opendkim/keys
# ls -lha
total 12K
drwxr-xr-x 3 root     root     4.0K Dec 16  2018 .
drwxr-xr-x 1 root     root     4.0K Mar 11 07:22 ..
drwxr-xr-x 2 opendkim opendkim 4.0K Dec 16  2018 <domain>

As the current image is working for me without any DKIM permission errors, I do not think this change is necessary. It would be somewhat disruptive for my use case because it would make the volume-mounted directory unreadable by my user (and, therefore, would interfere with git tracking).

If we have no other ways to solve #14, then I am fine with this solution (if it is tested and working), but let me comment on that Issue, because I have an idea what the problem may be.

@Hamsterman
Copy link

Let me propose another solution - I noticed in another project that the DKIM keys were in another folder. What if you do this

  1. Map a "temp" folder with keys to a volume
  2. At container start - copy the content to the /etc/opendkim/keys folder

This way the rights does not need to be changed in the /etc/opendkim/keys and thus opendkim won't complain. But you can still reuse the keys when the container is recreated.

Note you have to copy the keys when they are generated to the "temp" folder.

@wader
Copy link
Owner Author

wader commented Mar 16, 2020

Yeah that is an option, but then we have crippled the safety check for ppl who actually would like to know that their private keys are read or writable. Not sure what ppl expect hmm.

How about make it possible to explicitly configure RequireSafeKeys=no easily?

@Hamsterman
Copy link

How about make it possible to explicitly configure RequireSafeKeys=no easily?

That would work.

@wader
Copy link
Owner Author

wader commented Mar 16, 2020

@hemberger what do you think?

@hemberger
Copy link
Contributor

Is this a change you'd want to make even if we can solve #14 in another way? I would think we'd want to avoid encouraging people to use RequireSafeKeys=no (especially since it seems to be a common way to "solve" permission problems -- I know I used this as a hack when I was first fiddling around with DKIM). That said, I can imagine some rare scenario where someone legitimately wants to use this option...

@Hamsterman
Copy link

@hemberger In my world this should work out of the box - with no need for extra parameters.

@hemberger hemberger mentioned this pull request Mar 17, 2020
@wader
Copy link
Owner Author

wader commented Jun 14, 2020

Workaround possible with #18

@wader wader closed this Jun 14, 2020
@wader wader deleted the chown-chmod-opendkim-keys branch June 14, 2020 16:37
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 this pull request may close these issues.

Can't make DKIM work
3 participants