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

SASL passwords are created with the container's hostname as domain #192

Closed
thielj opened this issue Apr 14, 2024 · 10 comments
Closed

SASL passwords are created with the container's hostname as domain #192

thielj opened this issue Apr 14, 2024 · 10 comments

Comments

@thielj
Copy link

thielj commented Apr 14, 2024

Current boky/postfix:latest-alpine, postfix v3.8.4.

I've been trying to force authentication using SMTPD_SASL_USERS="user:pass". However, it didn't work as expected. Long story short, this is what ended up in sasldb

51686ed8d2b4:/tmp# sasldblistusers2
user@51686ed8d2b4: userPassword

When I use user@<containername> it all works as expected - except that the mail is bounced by Gmail and probably others ('From' header has non compliant domain name). I tracked this down to postfix_setup_smtpd_sasl_auth() in common-run.sh. It might need changing like this (see SASL_README):

    echo $_pwd | saslpasswd2 -p -c -u `postconf -h mydomain` $_user

When used in conjunction with the below, postfix would accept both user and [email protected] as valid user names.

    POSTFIX_smtpd_sasl_local_domain:          "$$mydomain"

Another option would be to specify SMTPD_SASL_USERS='[email protected]:password,...', which I've started using now. If the behaviour on other platforms is the same, it might be worth adding this to the README where it mentions SMTPD_SASL_USERS.


Speaking of other platforms, there's probably another issue in this function. When using the Debian based image boky/postfix:latest with SMTPD_SASL_USERS, the container keeps restarting:

--------8<-------------
‣ INFO  Enable smtpd sasl auth.
ln: failed to create symbolic link '/etc/sasl2/smtpd.conf': File exists
★★★★★ POSTFIX STARTING UP (debian) ★★★★★
‣ NOTE  Setting container timezone to: Etc/UTC
--------8<-------------

A simple check if this file exists or using ln -s -f might fix it. They both look identical, I haven't investigated any further than this.


Many thanks for providing this image, by the way!

bokysan added a commit that referenced this issue Apr 16, 2024
So, according to the documentation, usernames must always include a
domain for SASL.

In other words. User cannot be `johhny` but `[email protected]`.
Further info can be found on this ticket: #192

This commit will automatically append domain if one is not provided in
`SMTPD_SASL_USERS`.
@bokysan
Copy link
Owner

bokysan commented Apr 16, 2024

Hi,

thanks for this detailed report. You are, in fact, correct. The documentation was not clean enough on that part.

The latest commit to master should have this fixed. Please do kindly test and let me know if you run into any additional issues.

@thielj
Copy link
Author

thielj commented Apr 17, 2024

Hi, I haven't build this yet but had a quick look through the source and will test next weekend.

I think the bad users handling is unnecessary, i.e. you can always pass a sensible domain default, and the user can override this if desired:

/tmp # echo test | saslpasswd2 -p -c -u example.com bokysan
/tmp # echo test | saslpasswd2 -p -c -u example.com [email protected]
/tmp # sasldblistusers2
[email protected]: userPassword
[email protected]: userPassword

Also, it would be helpful to call sasldblistusers2 at the end to log the added user accounts.

@bokysan
Copy link
Owner

bokysan commented Apr 17, 2024

I think the bad users handling is unnecessary, i.e. you can always pass a sensible domain default, and the user can override this if desired:

The problem is legacy. If users are already using it as-is they won't notice the change and will complain why authentication stopped working all of a sudden.

@bokysan
Copy link
Owner

bokysan commented Apr 17, 2024

Also, it would be helpful to call sasldblistusers2 at the end to log the added user accounts.

Generally not the best idea to log the passwords on the console. True, they are provided as environment variables, but in Kubernetes, for example, they can read from Secrets so they are semi-hidden, at least.

Logging them directly into console at startup seems really bad to me.

@thielj
Copy link
Author

thielj commented Apr 17, 2024

sasldblistusers2 doesn't print the actual passwords, only the literal string userPassword. It took me over an hour to figure out that SASL authentication wasn't expecting just user, but something very different.

And I don't see how your current solution is any different with regards to backwards compatibility. If someone currently relies on SMTPD_SASL_USERS="user:pass" becoming user@<containername>, your change will break this, too. And they don't deserve any better...

With SMTPD_SASL_USERS="[email protected]:pass", the result will be identical to previous releases, ie. it doesn't matter what you specify with -u, or if you use -u at all.

You can test both with the example I gave you earlier today ;)

@thielj
Copy link
Author

thielj commented Apr 17, 2024

root@6eafbb376f55:/tmp# sasldblistusers2
root@6eafbb376f55:/tmp# echo test | saslpasswd2 -p -c bokysan_without_domain
root@6eafbb376f55:/tmp# echo test | saslpasswd2 -p -c -u example.com bokysan_without_domain
root@6eafbb376f55:/tmp# echo test | saslpasswd2 -p -c -u example.com [email protected]
root@6eafbb376f55:/tmp# sasldblistusers2
[email protected]: userPassword
[email protected]: userPassword
bokysan_without_domain@6eafbb376f55: userPassword

@thielj
Copy link
Author

thielj commented Apr 17, 2024

Re: the Debian image, the latest commit made it work again with SMTPD_SASL_USERS now.

There's another issue though:

cp: '/etc/localtime' and '/var/spool/postfix/etc/localtime' are the same file

I think it's the repeated copy here:

[[ -e /etc/localtime ]] && cp -fpv /etc/localtime $POSTFIXD_ETC || true
)


(ignore the original post, /var/spool/postfix/etc was a file in my mounted volume!?).

@thielj
Copy link
Author

thielj commented Apr 17, 2024

I've also rebuild master using alpine:latest - no major issues. Occasionally the following appears in the log file, but I can't reproduce this.

postfix/postfix-script[301]: warning: group or other writable: /etc/postfix/./makedefs.out

Another thought: switching between base images and keeping the same volume for /var/spool/postfix might be a source of spurious errors. I've seen this in other issues and experienced them myself. Wouldn't it be a good idea to zap any existing etc and usr directories when setting up the chroot? Or at least putting a warning in the docs and examples?

@thielj thielj closed this as completed Apr 17, 2024
@bokysan
Copy link
Owner

bokysan commented Apr 17, 2024

Do you want a pull request? ;)

That would be nice, yes. Thank you. 🙂

@bokysan
Copy link
Owner

bokysan commented Apr 17, 2024

Another thought: switching between base images and keeping the same volume for /var/spool/postfix might be a source of spurious errors. I've seen this in other issues and experienced them myself. Wouldn't it be a good idea to zap any existing etc and usr directories when setting up the chroot? Or at least putting a warning in the docs and examples?

Agreed. When I created alternate images I did not imagine that people would be switching between them back and forth. But your tool does get used in ways you've never expected. When starting you do get a warning but it would be prudent to add to docs that you should be doing this only if you really know what you're doing.

I'm against deleting anything, though - this is not something you'd expect an image to do and can come back and bite you in the a**. If anything I'd opt for killing the image and letting the user deal with this on his own.

I'll open another ticket on the topic if I just don't fix it in one commit.

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

No branches or pull requests

2 participants