Fix mail sendmail

[themancalledjakob]

Problem

• To fix email sending #228

Solution

• [x] adjust .env to use sendmail / postfix for sending emails
• [x] report the upstream security issue with sendmail

PR Status

• [x] Code finished and ready to be reviewed/tested
• [ ] The fix/enhancement were manually tested and worked on all tested machines

Comment

Tested with yunohost 11.2.2

[lapineige]

!testme

[yunohost-bot]

:stuck_out_tongue_winking_eye:

[lapineige]

What's your feedback on your instance ?

[themancalledjakob]

hey @lapineige, sorry for the late response. I am on vacation and can only rarely sneak in some computer time.

What's your feedback on your instance ?

I am a single user instance :) I did successfully try it with 2 different accounts and email addresses (one mailbox.org, the other yunohost mail). As there are no other users, I simply installed from scratch multiple times and it worked out of the box. Also, fixing a non-working config like this and reloading it with artisan worked immediately.

So, for me, it just worked. Pixelfed was able to send the email, and I received it properly on my mail provider (mailbox.org).

Reading through the internet and the manpage, I get the idea that sendmail (postfix) is actually a good way to solve this.

pixelfed's underlying sendmail command is '/usr/sbin/sendmail -bs' and therefore doing SMTP for us :)

-bs    Stand-alone SMTP server mode. Read SMTP commands  from  standard
              input,  and  write responses to standard output.  In stand-alone
              SMTP server mode, mail relaying and other  access  controls  are
              disabled  by  default.  To  enable  them, run the process as the
              [mail_owner](https://www.postfix.org/postconf.5.html#mail_owner) user.

So, technically seems legit to use sendmail. However, the command should possibly better be '/usr/sbin/sendmail -bs --' (notice the last dashes). There is a possibly scenario, where a user could create an email-address that starts with '-' and therefore inject commands (under SECURITY mentioned in the manpage). Adding the last two dashes prevent this. As Yunohost has afaik the premise to trust users, this is perhaps less of a critical issue? Though, as registrations are open by default, this could leave someone vulnerable out of the box. But my experience here is minimal. I'll investigate this, and if this is really a thing, perhaps a PR to upstream pixelfed could be an idea.

Alternatively, as this is using smtp... perhaps it may be possible to somehow figure out which commands are sent, and use smtp directly after all?

Am I overthinking this?

[lapineige]

sorry for the late response. I am on vacation and can only rarely sneak in some computer time.

Who said there were any hurry ? :wink:

I did successfully try it with 2 different accounts and email addresses (one mailbox.org, the other yunohost mail)

Great to know it works with Yunohost mail too :)

Thank you for the detailed answer ! Good to know it's working well :)

So, technically seems legit to use sendmail. However, the command should possibly better be '/usr/sbin/sendmail -bs --' (notice the last dashes). There is a possibly scenario, where a user could create an email-address that starts with '-' and therefore inject commands (under SECURITY mentioned in the manpage). Adding the last two dashes prevent this. As Yunohost has afaik the premise to trust users, this is perhaps less of a critical issue?

Do these last dashes changes anything in the behaviour ? If it's just working right, let's use it :)

Though, as registrations are open by default, this could leave someone vulnerable out of the box.

Yeah, let's avoid that risk :)

[themancalledjakob]

nice, that makes sense :)

The additional dashes should not change behaviour. I'm not sure how to add them in the sendmail command in pixelfed without adjusting the github repository. is there maybe a config we could set? For testing, I quickly made a fork and adjusted the default setting: https://github.com/themancalledjakob/pixelfed This is not ideal, as I will very likely never update this fork in the future.

But, good news: when I use this url in the manifest.toml and do a fresh install, everything still works!

[lapineige]

I'm not sure how to add them in the sendmail command in pixelfed without adjusting the github repository.

I don't understand what you mean. Can you explain it again please ? :sweat_smile:

Oh wait… you mean this has to be patched in Pixelfed itself ? https://github.com/themancalledjakob/pixelfed/commit/4311891fe4dcf694fb1ec9439db51298ce9f23cf#diff-af6ac21d51218ff8f16692c0c8a9bf2fb420d3cc0de12458049dd1ed9c5ec244 We could patch it, but if that's a security issue, we should probably report this upstream.

[themancalledjakob]

Oh wait… you mean this has to be patched in Pixelfed itself ?

exactly :) I am not sure how quick they will patch it though, so we would need a temporary solution. It would be great if we could define a custom sendmail-command, without having to fork the whole pixelfed repository. otherwise we would need to update the fork constantly. Sorry, I find this super hard to put into words clearly. :upside_down_face: I'll try again: basically, we can set all kind of settings by simply providing a custom .env file, right? ... sooo.. what I'm looking for, is a similar way to provide a custom sendmail-command and still use the upstream pixelfed repository. afaik there is no setting in the .env for this, but they seem to suggest that their sendmail command is a default command, and it sounds as if it can be adjusted by us... but how :detective:

We could patch it, but if that's a security issue, we should probably report this upstream.

yep, I did send them an email to here :)

[lapineige]

I am not sure how quick they will patch it though, so we would need a temporary solution.

It's easy to patch a line in a file :slightly_smiling_face:

[themancalledjakob]

aaaah, of course. you mean simply doing something like:

sed -i -e "s#'/usr/sbin/sendmail -bs'#'/usr/sbin/sendmail -bs --'#g" $install_dir/config/mail.php

in the install + upgrade script?

[edit]: I see, there is already a yunohost function which uses sed under the hood

ynh_replace_string --match_string="'/usr/sbin/sendmail -bs'" --replace_string="'/usr/sbin/sendmail -bs --'" --target_file=$install_dir/config/mail.php

[lapineige]

Indeed :)

[themancalledjakob]

Amazing, I added it to install, upgrade and restore now.

Could you have a look if it is fine?

[lapineige]

Looks good to me !

[lapineige]

!testme

[yunohost-bot]

:sunflower:

[lapineige]

Once continuous integration is happy, feel free to merge :slightly_smiling_face:

[lapineige]

I tried it (with password reset), I have a something went wrong :( And I can't get the details, I get an error code but I don't remember in which Pixelfed site it's possible to translate it…

[themancalledjakob]

I tried it (with password reset), I have a something went wrong :( And I can't get the details, I get an error code but I don't remember in which Pixelfed site it's possible to translate it…

ah damn. so that means this PR isn't ready yet, I guess.

You can decrypt the error message like this on your yunohost server in the shell:

$ sudo yunohost app shell pixelfed
$ php artisan tinker
Psy Shell v0.11.18 (PHP 8.2.8 — cli) by Justin Hileman
> decrypt('payload here')

haven't tried it yet myself actually, but can't wait to get an error message to try it out :-D

you can also do it in the admin panel, like the dev explains here. make sure your pixelfed user has admin rights, and go to: https://my.pixelfed.domain/i/admin/diagnostics/home scroll to the very bottom and there should be a field to enter the payload

[lapineige]

Great, thanks 🙂

I tried this version:

Unable to connect with STARTTLS: stream_socket_enable_crypto(): Peer certificate CN=`maindomain.tld' did not match expected CN=`localhost'

The thing is it's using the wrong domain, it's using the default domain of my server, not Pixelfed domain.

MAIL_FROM_ADDRESS= use the right domain.

[lapineige]

Strangely, if I force upgrade to this version, the env file is not updated. That might mean it's only done on new installs ? :thinking:

[lapineige]

I changed the file manually. This is still happening :

The thing is it's using the wrong domain, it's using the default domain of my server, not Pixelfed domain.

[lapineige]

So.... I restarted everything (the new parameters in the .env were not taken into account apparently).

And it works !!

So now we need to handle upgrades to fix this for existing installs.

[lapineige]

Done ! 😃

[lapineige]

!testme

[yunohost-bot]

May the CI gods be with you!

[lapineige]

Thanks a lot for your work !!

[FatherMcGruder]

Will email just work with the next update, or will I have to subsequently adjust the .env and/or other system settings?

[lapineige]

The .env should be overridden during the upgrade (a backup a is created in the same folder), so it should just work.