Created attachment 163485 [details] svn diff security/amavisd-milter As I said to maintainer by private mail some months ago without response: On two different mail servers, amavisd-milter gives me: postfix/cleanup[10153]: 060685525: milter-reject: END-OF-MESSAGE from mel0.serv.int[10.3.0.88]: 4.5.0 Failure: Suspicious temporary directory name '/var/run/amavis/tmp/afcokTMBJlIs' This line comes from amavisd l.21082, where tempdir is checked against amavis's TEMPBASE and MYHOME (by default in /var/amavis). amavisd-milter use /var/run/amavis/tmp by default (--with-working-dir=tmp) => I don't understand how it can work ? (by overriding amavis's TEMPBASE ?) Attached patch fixes this by using ${AMAVISDIR}/tmp as working dir I'm also using an 'inet' socket (maybe that changes something ?). I also add a simple test in rc script to NOT TRY chmod'ing socket if socket is not on filesystem. 'poudriere testport'ed, 'port test'ed, production-tested since 2015-05. not fixing portlint here (I may propose a patch later after further testing)
Make this an individual port PR.
Created attachment 163557 [details] svn diff security/amavisd-milter ensure exit status==0 if socket is not a file
(In reply to Mark Linimon from comment #1) sorry, all work and no play made me go too fast ;)
It's OK. Thanks. P.
Comment on attachment 163557 [details] svn diff security/amavisd-milter It's OK. Thanks. P.
(In reply to Petr Rehor from comment #5) I think you should approve with 'maintainer-feedback'=>'+' both pr and patch… About merge-quarterly, not sure about policy but for my usage it's broken without this patch: is 'un-break port' a sufficient reason to go there ? I'd say yes.
Comment on attachment 163557 [details] svn diff security/amavisd-milter TLDR: I'll set maintainer-approval as per comment 5 There's currently a permission bug precluding people who's account names are listed in the ? value of a flag, to modify the value of that flag. Currently, the only people who can change an attachments flag value is the attachment creator. This will be fixed in the upcoming Bugzilla 5.x upgrade.
(In reply to geoffroy desvernay from comment #6) Yes this is a good example of an MFH candidate. Merge Quarterly rules of thumb: * Anything other than version updates. Primarily: bugs, fixes, improvements * Version updates that provide security fixes
Created attachment 164135 [details] svn diff security/amavisd-milter One line less: @dir line in plist is not sufficient in some case (I had poudriere fail without mkdir in Makefile (but not when I testport'ed ?!?).
(In reply to Kubilay Kocak from comment #8) Ok, thank you for your clarifications :)
Comment on attachment 164135 [details] svn diff security/amavisd-milter Patch is OK.
(In reply to geoffroy desvernay from comment #6) I can set maintainer-feedback => + on PR but setting maintainer-approval => on attachment doesn't have any effect. P.
A commit references this bug: Author: junovitch Date: Sun Dec 27 15:49:02 UTC 2015 New revision: 404578 URL: https://svnweb.freebsd.org/changeset/ports/404578 Log: security/amavisd-milter: resolve runtime bug with temp directory handling - Use ${AMAVISDIR}/tmp as working directory to resolve runtime bug: Failure: Suspicious temporary directory name '/var/run/amavis/tmp/afcokTMBJlIs' PR: 204787 Submitted by: Geoffroy Desvernay <dgeo@centrale-marseille.fr> Approved by: Petr Rehor <rx@rx.cz> (maintainer) MFH: 2015Q4 Changes: head/security/amavisd-milter/Makefile head/security/amavisd-milter/files/amavisd-milter.in head/security/amavisd-milter/pkg-plist
Sorry for the delay and thank you both for the collaboration on this. I'll MFH the update as soon as I get portmgr/ports-secteam approval for the commit to quarterly.
What permissions is the socket file created with in the rc script? Why do we need to chmod it during the postcmd function?
I can't check exactly now, If I remembrer well it lacks g+w perm by default to let MTA use it.
(In reply to geoffroy desvernay from comment #16) If there is something that we missed then please let me know. I'll go ahead and close this in the meantime as it's been 3 months with no issues raised against the port. Set merge-quartery- as it was never approved and became irrelvant after 2016Q1 was branched.