'setPublicKey' dies on broken cert, causes spindle to resend mails indefinitely

[manuelschneider]

The setPublicKey method in the c implementation of the smime module uses a croak(). If I understand it correctly this causes the whole script to die():

https://github.com/sympa-community/sympa/blob/sympa-6.2/src/lib/Sympa/Message.pm#L1120

This line should probably have some handling for this, like

https://github.com/sympa-community/sympa/blob/sympa-6.2/src/lib/Sympa/Message.pm#L1134

Probably this should become another issue: I think as a fallback, maybe you could remember recipients that have already been sent emails. That could avoid sending the mail to them over and over again, in case some other part of the code has a bug too. Maybe you could update the rcpts (I guess $message is some kind of ORM object?), removing the processed recipient when mailer->store() was successful?

https://github.com/sympa-community/sympa/blob/sympa-6.2/src/lib/Sympa/Spindle/ProcessOutgoing.pm#L421

[ikedas]

Hi @manuelschneider,

Could you please apply this patch and check if Sympa will no longer crash?

By this change, if a user had contributed broken certificate, Sympa will send a message telling "It was not possible to send you the message" instead of sending encrypted message.

Probably this should become another issue: I think as a fallback, maybe you could remember recipients that have already been sent emails. That could avoid sending the mail to them over and over again, in case some other part of the code has a bug too. Maybe you could update the rcpts (I guess $message is some kind of ORM object?), removing the processed recipient when mailer->store() was successful?

List of all recepients (rcpts) is split into (generally more than one) "packets", and message distribution is done by each packet. If Sympa has crashed during distribution and has restarted, recipients in aborted packet would receive the message again. If total of subscribers in a list is not larger than size of a packet (10 or so by default), most of subscribers would receive message again.
If Sympa won't crash, packet will be removed and subscribers won't receive messages again.

[manuelschneider]

Hi @ikedas ,

thank you very much for the quick response!

Could you please apply this patch and check if Sympa will no longer crash?

From looking at it, as far as I understand perl and this part of sympa, this should solve the problem.
Unfortunately I cannot test it, as we don't have a test-instance. I won't risk another hickup in production by reverting the offending certificate back to its broken stage.

If total of subscribers in a list is not larger than size of a packet (10 or so by default), most of subscribers would receive message again.

Is it possible to configure the packetsize to 1? In terms of public perception its pretty much a complete desaster, even more if your own addresses aren't in that package and you have no log monitoring.

[ikedas]

Hi @ikedas ,

thank you very much for the quick response!

Could you please apply this patch and check if Sympa will no longer crash?

From looking at it, as far as I understand perl and this part of sympa, this should solve the problem.
Unfortunately I cannot test it, as we don't have a test-instance. I won't risk another hickup in production by reverting the offending certificate back to its broken stage.

Ok, I'll merge that patch and check it during beta period in the next week.

If total of subscribers in a list is not larger than size of a packet (10 or so by default), most of subscribers would receive message again.

Is it possible to configure the packetsize to 1? In terms of public perception its pretty much a complete desaster, even more if your own addresses aren't in that package and you have no log monitoring.

There are no parameters setting packet size exactly, but adding

nrcpt 1
avg 1

in sympa.conf may almost always limit the size to 1 or a few. However, please note that this setting will make message distribution slower.

[manuelschneider]

Thank you very much!

I think slower is acceptable if it prevents spamming :)

[ikedas]

Merged. Thanks for reporting bug and suggesting fix!