Roundup Tracker - Issues

Issue 2550747

classification
Feature request - Send mail using external program
Type: rfe Severity: normal
Components: Mail interface Versions:
process
Status: new
:
: : ber, dgatwood, jherazo-beverly, pefu, rouilj, rrt
Priority: : patch

Created on 2012-03-01 16:33 by jherazo-beverly, last changed 2013-11-19 22:46 by rrt.

Files
File name Uploaded Description Edit Remove
use_sendmail.patch dgatwood, 2012-10-26 05:52
sendmail.second.patch rouilj, 2013-06-09 03:47
Messages
msg4505 Author: [hidden] (jherazo-beverly) Date: 2012-03-01 16:33
Roundup seems to lack the ability to send mail using an external
program, like /usr/sbin/sendmail, ssmtp or similar ones. Considering the
fact that this is a rather common setup in many places it would be good
if Roundup supported this.
msg4506 Author: [hidden] (ber) Date: 2012-03-02 08:26
Hi Jamie,
thanks for your suggestion and for using roundup!

Can you tell us more about it?
Is this something you need for an installation somewhere?
Why is installing a simple SMPT server like nullmailer or esmtp 
not solving your problem?

Best,
Bernhard
msg4507 Author: [hidden] (ber) Date: 2012-03-02 08:29
Ah, okay, there is also a discussion about it on roundup-users,
subject: How to use a command to send email?
http://sourceforge.net/mailarchive/forum.php?thread_name=4F4FA69A.2090203
%40beverlydata.com&forum_name=roundup-users
msg4658 Author: [hidden] (dgatwood) Date: 2012-10-26 05:52
This bug was pretty much a showstopper for me.  My ISP won't allow me to send email using 
their SMTP server over port 25 (even connecting to localhost) without providing a username 
and password, and there's *no* *way* I'm hard-coding a cleartext account password into any 
script, period, much less on a shared server.  :-)

Patch attached.  This patch adds a "sendmail = " option.  After applying this patch, you can 
add:

    sendmail = /usr/sbin/sendmail

(or whatever) to your config file, and instead of using SMTP directly, it will pass the message 
to the sendmail binary.
msg4659 Author: [hidden] (ber) Date: 2012-10-26 07:33
Hi David,
thanks for the patch and the message.

I am curious: how does the sendmail binary that is called now
send out the email then? It probably also will need to use the
credentials somehow?

Best,
Bernhard
msg4660 Author: [hidden] (dgatwood) Date: 2012-10-29 18:29
If memory serves, the sendmail binary creates a randomly named data file and a 
corresponding lock file inside the outgoing mail spool directory, writes the message into the 
main file, then deletes the lock file.

Meanwhile, the sendmail daemon (running as root) watches that queue directory and when it 
sees a new file, it waits for the corresponding lock file to disappear (if it even still exists at 
that point).  Then it reads the file, relays its contents, and deletes the file from disk.

That said, I've never done any coding under the hood of sendmail—the closest I've ever come 
is writing sendmail.cf rules—so I'm not certain about that.
msg4661 Author: [hidden] (ber) Date: 2012-10-30 08:26
David,

first thanks for commenting, my aim is to understand your
need better in order to help creating a cool solution. ;)

The question I had was: 
How does sendmail get this email away from your machine.
I mean: If sendmail can send out emails to another machine without need
to authenticate itself, then roundup would be able, too.
My conclusion is that sendmail has the necessary credentials.

So you are calling sendmail which probably is a suid root or sgid root
application and then uses it higher access rights to read the saved
credentials. 

If this is the case, you could configure your sendmail to just listen
to a local port without authentification and accept emails from roundup
from there. 

Best,
Bernhard
msg4662 Author: [hidden] (dgatwood) Date: 2012-10-31 18:01
You're conflating two different things that have the same name.  The shared server has:

1.  sendmail—a daemon running on the local host.  This daemon does two things:
a.  Receives incoming mail on port 25.  If the delivery address is local, it delivers it.  
Otherwise, it either relays it or refuses depending on whether credentials were provided by 
the connecting host.
b.  Looks for email in an outgoing spool folder.  It delivers this without asking for credentials.

2.  sendmail—a binary on the local machine that drops files into the mail spool.
msg4663 Author: [hidden] (dgatwood) Date: 2012-10-31 22:00
Just to further clarify, this is on a shared server.  If I were running the server, I could trivially 
reconfigure the sendmail daemon to accept unauthenticated connections from localhost, but this 
is running on a shared hosting server, so I have no control over sendmail's configuration.
msg4664 Author: [hidden] (ber) Date: 2012-11-01 10:38
David,

so now I do understand your issue, I believe.
The running sendmail allows local process to inject emails
without further credentials via the binary, but not via TCP.

Thanks for explaning it.

I check if we can integrate your patch. Do you think it would need
some more documentation somewhere? What happens in case that the
sendmail binary return an error? Is this case handled well enought?

Bernhard
msg4665 Author: [hidden] (dgatwood) Date: 2012-11-01 20:52
As far as I know, there are only three possible results of sendmail running:

    0: It happened.

    255: The sendmail binary failed to execute. (This value may be OS-dependent.)

    other nonzero value: You failed to pass in a recipient parameter.

So it might be worth checking the exit status to catch mistakes caused by somebody entering 
the wrong path for the sendmail binary, or passing a sendmail binary that won't execute for 
some reason or whatever.

Beyond that, though, sendmail does basically zero validity checking.  You can pass in 
complete gibberish (and possibly even an empty string) for the recipient address, and 
complete gibberish for the message, and it still exits successfully, as far as I can tell.  So for 
the most part, it's a "hand it off and hope" technique.  If it fails, you'll know it when you get a 
bounce message in the inbox of the person listed in the From: field.  :-)


As for documentation, presumably the new key in the config file would need to be explained 
in the config file, e.g.

"sendmail: The path to a local sendmail binary.  If nonempty, the specified binary is used for 
handling all mail delivery, and any SMTP hostname and port settings are ignored."

Or the equivalent info in a form that conforms to your house style.

Also, it would probably be worth adding a section to the "Installing Roundup" document to 
cover setting up outgoing email.  Right now, I'm not seeing that mentioned at all, and if you 
don't do that, you can easily find yourself locked out of accounts because it can't send out 
password change emails.
msg4667 Author: [hidden] (ber) Date: 2012-11-02 08:13
David: 
again thanks for the contribution.
If you feel like adding a patch to help with any of this,
it is for course highly welcome. :)
Bernhard
msg4905 Author: [hidden] (rouilj) Date: 2013-06-09 03:47
I attached a new sendmail patch. This one adds:

  explicit close of stdin after writing the message to sendmail

  wait for the sendmail process to finish

  check the return code. If it's 0 or 75 things are ok (75 indicates
      the email 1s queued according to the man page for sendmail
      I have). If it's any other exit code, raise MessageSendError
      with exitcode and stderr text.

Do we want to try getting this in the next release?
msg4906 Author: [hidden] (rouilj) Date: 2013-06-17 21:09
Don't apply the second patch. Comments:

In message <20130617183108.GC7600@runtux.com>,
Ralf Schlatterbeck writes:
>On Mon, Jun 17, 2013 at 02:01:05PM -0400, John P. Rouillard wrote:
>> >
>> >Anything that should be in the current release?
>>
>> Can somebody comment on whether the new patch I added to:
>>
>>    http://issues.roundup-tracker.org/issue2550747
>>
>> looks reasonable? I used the original requestors patch as a base and
>> updated it with a bit of error detection etc. If it looks ok, I'll
>> commit it.
>>
>> It would be nice if we could get through the backlog of tickets with
>> patches since people contributed them and this is a good way to get
>> additional contributers.
>
>Did you test this?

Yeah minimally. I had a mock and tested the output of the mock with a
real sendmail (not postgres/qmail in sendmail emulation
mode). However,[q I just checked my test logs and realized that the
test used the -t flag for sendmail which is not what my patch
implements 8-(. Which explains how my tests passed.

>To me it looks like the sending only works for a single recipient,
>sendmail (according to my manpage for the sendmail command coming with
>postfix) wants each recipient as a separate command-line parameter. Not
>a comma separated list. Note that postfix' sendmail doesn't have that
>weird exitcode.

Exit code 75 may exit only in a true sendmail(tm).

Does the -t flag work with postfix's sendmail interface?

   -t     Read  message  for recipients.  To:, Cc:, and Bcc: lines will be
          scanned for recipient addresses.  The Bcc: line will be  deleted
          before transmission.

>The stderr= setting of Popen also looks wrong to me (we don't read back
>from a pipe to sendmail).

Well my patch certainly does if there is an error exit.

+            print >>mailer.stdin, message
+            mailer.stdin.close()
+            if mailer.wait():
+                if mailer.returncode != 75:
+                    mailer_err = mailer.stderr.read(2048)
+                    raise MessageSendError("Error: couldn't send email: "
+                          "%s returned %s - %s"%(sendmail,
+                                    mailer.returncode, mailer_err))

>So the correct call would probably be
>
>args = [sendmail] + to
>mailer = subprocess.Popen(args, stdin=subprocess.PIPE)

At least it should be a sanitized set of space separated addresses (or
more properly an array of addresses since the shell would have split
the space separated addresses into separate elements of argv[]).

Also despite using popen, those addresses should never be exposed to
an external process (and possible shell) without sanitizing them. At
the very least shell metachars |, `, & should be considered to make
sure they won't trick popen into starting a shell.

>maybe we should add a test before committing this?

Sigh yeah. Well there's another timesink.

>Also note that one reason people wanted that feature is that sendmail
>doesn't require authorisation -- but roundup *can* be configured with a
>user and password for smtp authentication, so a normal authenticated
>service of the provider *can* be used.

>What do you think?

I still think it's a useful patch. Especially when you don't want to
embed passwords for your upstream email provider in roundup. Roundup
has a larger attack surface than a binary dedicated to schlepping
email around (or it should).

Using sendmail locally like this means that you get messages queued
and retried automatically. AFAIK roundup will never try to resend an
email that fails because the smtp server is down, inaccessible etc.

But yes this patch is crap, good thing I don't work as a developer.
I won't be committing it.
msg4951 Author: [hidden] (rrt) Date: 2013-11-19 22:46
By changing one line in the current (second) version of the patch, I was
able to get it working with roundup 1.4.18 (I'm using an old version as
shipped with Ubuntu precise, but I presume the patch applies cleanly to
later versions too):

+            mailer = subprocess.Popen([sendmail, "-t"],
stdin=subprocess.PIPE, stderr=subprocess.PIPE)

In other words, use the -t parameter to sendmail (in my case, sendmail
was esmtp) as specified in msg4906.
History
Date User Action Args
2013-11-19 22:46:55rrtsetnosy: + rrt
messages: + msg4951
2013-06-17 21:09:26rouiljsetmessages: + msg4906
2013-06-09 03:47:42rouiljsetfiles: + sendmail.second.patch
nosy: + rouilj
messages: + msg4905
2012-11-02 08:13:44bersetmessages: + msg4667
2012-11-01 20:52:43dgatwoodsetmessages: + msg4665
2012-11-01 10:38:05bersetmessages: + msg4664
2012-10-31 22:00:37dgatwoodsetmessages: + msg4663
2012-10-31 18:01:42dgatwoodsetmessages: + msg4662
2012-10-30 08:27:00bersetmessages: + msg4661
2012-10-29 18:29:43dgatwoodsetmessages: + msg4660
2012-10-26 07:33:27bersetmessages: + msg4659
2012-10-26 05:52:40dgatwoodsetfiles: + use_sendmail.patch
keywords: + patch
messages: + msg4658
nosy: + dgatwood
2012-03-02 08:29:32bersetmessages: + msg4507
2012-03-02 08:26:30bersetnosy: + ber
messages: + msg4506
2012-03-02 07:40:58pefusetnosy: + pefu
components: + Mail interface, - None
2012-03-01 16:33:43jherazo-beverlycreate