Mailing List Archive

Patches from Robert Millan
tag 430508 upstream
tag 431239 upstream
thanks

Any comments or objections to these patches?
http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;att=1;bug=430508
http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;att=1;bug=431239

--
Magnus Holmgren holmgren@lysator.liu.se
(No Cc of list mail needed, thanks)

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=21821003-61be35
Powered by Listbox: http://www.listbox.com
Re: Patches from Robert Millan [ In reply to ]
On Friday 13 July 2007 16:20, Magnus Holmgren wrote:

> Any comments or objections to these patches?
> http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;att=1;bug=430508

I'm not a C coder, so I'm only looking at this from a design/requirements
perspective.

spfquery segfault #430508 looks reasonable to me. Setting the fallback is
almost certainly uncommon, so it's not suprising this didn't show up before.
It's also functionality that's not required (or even mentioned) in RFC 4408,
so if it doesn't work it's not a high priority from my perspective.


Scott K

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=21968009-933000
Powered by Listbox: http://www.listbox.com
Re: Patches from Robert Millan [ In reply to ]
On Friday 13 July 2007 16:20, Magnus Holmgren wrote:
> tag 430508 upstream
> tag 431239 upstream
> thanks
>
> Any comments or objections to these patches?
> http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;att=1;bug=430508
> http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;att=1;bug=431239


RFC conformance issues #431239 is a little more complex:

The patch carries an LGPL copyright notification. Libspf2 is currently dual
licensed under BSD and GPL version 2.

I don't recall the exact rules on combining LGPL and GPL code (it hasn't come
up in any packages I've done), but that needs to be considered.

More important is that is LGPL code is incorporated, the package is no longer
distributable under the BSD license. The patch submitter is, of course, free
to license their work however they want, but the intent of the original
author was clearly (I've discussed it with him) to have libspf2 be BSD
distributable so it could be incorporated into closed products in order to
better evangelize and spread the SPF technology. I'd recommend asking the
submitter if they would relicense their patch under BSD/GPL v2. The Debian
libpsf2 package is the most up to date currently and we have been pointing
even non-Debian people to it, so this would have implications larger than
Debian. It would be nice to be able to keep things the way the author
wanted.

From a technical perspective:

Adding the Null Mail From HELO check is useful. The approach used here is
consistent with the approach that many long standing SPF libraries
(Mail::SPF::Query and pyspf for two) take. So this is good.

The second part of the change, changing the SPF None response string from

"%s is neither permitted nor denied by %s",

to

"%s doesn't provide an SPF record"

is a wording improvement, but given that it's been this way for several years,
I don't know if there are programs that are dependent on that string. So,
I'd suggest this is an improvement, but not entirely without risk. I'd tend
not to want to make it, but I could understand either way.

Scott K

P.S. Thanks for the chance to review the changes.

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=21969402-eee89b
Powered by Listbox: http://www.listbox.com
Re: Patches from Robert Millan [ In reply to ]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Magnus Holmgren wrote:
> Any comments or objections to these patches?
> http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;att=1;bug=430508

Thanks for your patch!

Robert Millan wrote in bugs.debian.org #430508:
> spfquery segfaults when passing it a -guess argument:
>
> $ spfquery -ip 1.2.3.4 -sender foo@bar.org -helo foo -guess blah
>
> This is caused by spf_response_2mx being a null pointer in
> APPEND_RESULT(SPF_response_result(spf_response_2mx));
>
> I *think* that usage of this variable is all wrong in this routine,
> since we're checking for fallback spf record not 2nd rcpt mx. See
> attached patch for a proposed fix.

I think the use of the "spf_response_2mx" variable instead of an
additional, separate "spf_response_fallback" for the fallback branch in
spfquery.c was laziness on the spfquery author's part. However, the
initialization of "spf_response" instead of "spf_response_2mx" was clearly
erroneous. Overall, I think Robert's patch for bdo #430508 is sound.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFGl/pRwL7PKlBZWjsRAnpuAKC7QdzqLZaA/anMRjEDWU+DywcOPQCdGUpo
H01G+BtYJPDSJGo1tNhA0rg=
=xWK1
-----END PGP SIGNATURE-----

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=21958012-a4a348
Powered by Listbox: http://www.listbox.com
Re: Patches from Robert Millan [ In reply to ]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Scott Kitterman wrote:
> On Friday 13 July 2007 16:20, Magnus Holmgren wrote:
> > Any comments or objections to these patches?
> > http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;att=1;bug=431239
>
> RFC conformance issues #431239 is a little more complex:
>
> The patch carries an LGPL copyright notification. Libspf2 is currently
> dual licensed under BSD and GPL version 2.

This seems to be a mistake in debian/copyright. libspf2's LICENSES file
says:

| The code in the libspf2 distribution is Copyright 2005 by Shevek
| and Wayne Schlitt, all rights reserved. Copyright retained for the
| purpose of protecting free software redistribution.
|
| This program is free software; you can redistribute it and/or modify
| it under the terms of either:
|
| a) the GNU Lesser General Public License as published by the Free
| Software Foundation; either version 2.1, or (at your option) any
| later version, or
|
| OR
|
| b) The two-clause BSD license.

I just reported this as a separate bug (bdo #433047[1]).

> I don't recall the exact rules on combining LGPL and GPL code (it hasn't
> come up in any packages I've done), but that needs to be considered.
>
> More important is that is LGPL code is incorporated, the package is no
> longer distributable under the BSD license.

This is true even considering that libspf2's actual license is LGPL/BSD
rather than GPL/BSD. Robert's patch needs to be dual-licensed under LGPL
and BSD just like libspf2 in order to allow the patched libspf2 to be
distributed under the BSD license in the future. Robert, would you
consider resubmitting your patch with the license note amended to that
effect?

> From a technical perspective:
>
> Adding the Null Mail From HELO check is useful. The approach used here
> is consistent with the approach that many long standing SPF libraries
> (Mail::SPF::Query and pyspf for two) take. So this is good.

Agreed. (Do not construe this as a retreat from the newer Mail::SPF Perl
module's more formal approach, whose author I am. Whereas I prefer the
more formal approach, the less formal one used by Mail::SPF::Query, pyspf,
and now libspf2, is certainly legitimate.)

> The second part of the change, changing the SPF None response string
> from
>
> "%s is neither permitted nor denied by %s",
>
> to
>
> "%s doesn't provide an SPF record"
>
> is a wording improvement, but given that it's been this way for several
> years, I don't know if there are programs that are dependent on that
> string.

If there was any code depending on the explanatory result text, then that
would be a design bug in such code.

> So, I'd suggest this is an improvement, but not entirely without risk.
> I'd tend not to want to make it, but I could understand either way.

I say go ahead with the change. I wouldn't... er... would not use a
contraction, though. Say "%s does not provide an SPF record" instead.

References:
1. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=433047

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFGmAnmwL7PKlBZWjsRAt/mAKDCQGnf80lNqAnUyxxFpWAxNrrtTwCghnUc
PIPXpUOYbXPIKLlSQaef9Tg=
=oZf7
-----END PGP SIGNATURE-----

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=21980995-b603d9
Powered by Listbox: http://www.listbox.com
Re: Re: Patches from Robert Millan [ In reply to ]
On Friday 13 July 2007 19:25, Julian Mehnle wrote:
> Scott Kitterman wrote:

> > The second part of the change, changing the SPF None response string
> > from
> >
> > "%s is neither permitted nor denied by %s",
> >
> > to
> >
> > "%s doesn't provide an SPF record"
> >
> > is a wording improvement, but given that it's been this way for several
> > years, I don't know if there are programs that are dependent on that
> > string.
>
> If there was any code depending on the explanatory result text, then that
> would be a design bug in such code.
>
> > So, I'd suggest this is an improvement, but not entirely without risk.
> > I'd tend not to want to make it, but I could understand either way.
>
> I say go ahead with the change. I wouldn't... er... would not use a
> contraction, though. Say "%s does not provide an SPF record" instead.

Upon reflection, I agree with Julian. Go for it.

Scott K

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=22113860-9beb83
Powered by Listbox: http://www.listbox.com
Re: Patches from Robert Millan [ In reply to ]
Hi!

On Fri, Jul 13, 2007 at 11:25:26PM +0000, Julian Mehnle wrote:
> > I don't recall the exact rules on combining LGPL and GPL code (it hasn't
> > come up in any packages I've done), but that needs to be considered.
> >
> > More important is that is LGPL code is incorporated, the package is no
> > longer distributable under the BSD license.
>
> This is true even considering that libspf2's actual license is LGPL/BSD
> rather than GPL/BSD. Robert's patch needs to be dual-licensed under LGPL
> and BSD just like libspf2 in order to allow the patched libspf2 to be
> distributed under the BSD license in the future. Robert, would you
> consider resubmitting your patch with the license note amended to that
> effect?

To be honest, I have to say that I don't like the possibility of my code
becoming non-free. That said, I always find it a reasonable compromise to
allow this for the sake of merging the code upstream when the upstream
maintainers require it.

However, the upstream maintainer didn't reply to my emails, and the last
release is more than 2 years old. In this situation it's really up to Debian.
My code is DFSG-free and it can't bring incompatibility problems with
other packages using it. It is really up to Magnus wether to include them.

If there's ever someone actively maintaining libspf2 again, and that person
wants to stick with an unprotected license, I'll gladly license it in these
terms.

> > The second part of the change, changing the SPF None response string
> > from
> >
> > "%s is neither permitted nor denied by %s",
> >
> > to
> >
> > "%s doesn't provide an SPF record"
> >
> > is a wording improvement, but given that it's been this way for several
> > years, I don't know if there are programs that are dependent on that
> > string.
>
> If there was any code depending on the explanatory result text, then that
> would be a design bug in such code.

For the record, this puzzled me when debugging, since I was delluded into
thinking I was getting an SPF_RESULT_NEUTRAL when it was actually
SPF_RESULT_NONE. I think it's important to distinguish.

> > So, I'd suggest this is an improvement, but not entirely without risk.
> > I'd tend not to want to make it, but I could understand either way.
>
> I say go ahead with the change. I wouldn't... er... would not use a
> contraction, though. Say "%s does not provide an SPF record" instead.

Fine with that.

--
Robert Millan

My spam trap is honeypot@aybabtu.com. Note: this address is only intended
for spam harvesters. Writing to it will get you added to my black list.

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=22113590-1401e9
Powered by Listbox: http://www.listbox.com
Re: Patches from Robert Millan [ In reply to ]
On Fri, Jul 13, 2007 at 11:25:26PM +0000, Julian Mehnle wrote:
> >
> > Adding the Null Mail From HELO check is useful. The approach used here
> > is consistent with the approach that many long standing SPF libraries
> > (Mail::SPF::Query and pyspf for two) take. So this is good.
>
> Agreed. (Do not construe this as a retreat from the newer Mail::SPF Perl
> module's more formal approach, whose author I am. Whereas I prefer the
> more formal approach, the less formal one used by Mail::SPF::Query, pyspf,
> and now libspf2, is certainly legitimate.)

What is the other approach?

--
Robert Millan

My spam trap is honeypot@aybabtu.com. Note: this address is only intended
for spam harvesters. Writing to it will get you added to my black list.

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=22114121-c5c5e8
Powered by Listbox: http://www.listbox.com
Re: Patches from Robert Millan [ In reply to ]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Millan wrote:
> On Fri, Jul 13, 2007 at 11:25:26PM +0000, Julian Mehnle wrote:
> > > Adding the Null Mail From HELO check is useful. The approach used
> > > here is consistent with the approach that many long standing SPF
> > > libraries (Mail::SPF::Query and pyspf for two) take. So this is
> > > good.
> >
> > Agreed. (Do not construe this as a retreat from the newer Mail::SPF
> > Perl module's more formal approach, whose author I am. Whereas I
> > prefer the more formal approach, the less formal one used by
> > Mail::SPF::Query, pyspf, and now libspf2, is certainly legitimate.)
>
> What is the other approach?

Mail::SPF's approach is more formal in that it does not automatically
switch over to checking the HELO identity in case of the MAIL FROM identity
being empty. Rather, there is only one identity argument (not "mfrom" +
"helo"), and Mail::SPF requires you to check for yourself whether MAIL FROM
is empty and then pass the HELO identity to make a "postmaster@<HELO>"
check. (Note in particular that RFC 4408 does not require implementations
to automate this. I think it's cleaner if they don't.)

See <http://search.cpan.org/dist/Mail-SPF/lib/Mail/SPF/Request.pm#mfrom>
and <http://search.cpan.org/dist/Mail-SPF/lib/Mail/SPF/Request.pm#identity>.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFGmNCJwL7PKlBZWjsRAtnTAKDxDxJotgP6Pi9Q7Buzypi7c4Xj0ACg8z2s
PvEa/tyFpVwDFUvQ0EIOrCI=
=KX+o
-----END PGP SIGNATURE-----

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=22117239-08e291
Powered by Listbox: http://www.listbox.com
Re: Re: Patches from Robert Millan [ In reply to ]
On Saturday 14 July 2007 15:32, Julian Mehnle wrote:
> Mail::SPF's approach is more formal in that it does not automatically
> switch over to checking the HELO identity in case of the MAIL FROM identity
> being empty. Rather, there is only one identity argument (not "mfrom" +
> "helo"), and Mail::SPF requires you to check for yourself whether MAIL FROM
> is empty and then pass the HELO identity to make a "postmaster@<HELO>"
> check.

The RFC specifies that the MAIL FROM identity MUST be checked, and that the
MAIL FROM identity is defined as postmaster@(HELO-id) when the return path is
null (<>). It'd seem convenient to me if libspf2 automatically did what was
required.

> (Note in particular that RFC 4408 does not require implementations
> to automate this. I think it's cleaner if they don't.)

Can you point out precisely where it doesn't require that? :-)

Seriously though, it talks about "implementations" and "SPF clients" that MUST
or SHOULD do things in specified ways, but that doesn't say anything about
what should be done in the library and what should be done in the
applications.

However, I don't quite understand the difference between a HELO id check and
MAIL FROM check with a null return path. In the first case, which is
optional, check_host() is passed the HELO identity, which IIUC is just the
HELO string and looks like "foo.example.com" (in the ideal case), and
check_host() should prepend "postmaster@" to it. In the second case the
check_host() caller must itself prepend "postmaster@" to the HELO id. In this
light it seems appropriate for the application to follow the
Mail::SPF::Request instructions.

> See <http://search.cpan.org/dist/Mail-SPF/lib/Mail/SPF/Request.pm#mfrom>
> and
> <http://search.cpan.org/dist/Mail-SPF/lib/Mail/SPF/Request.pm#identity>.

--
Magnus Holmgren holmgren@lysator.liu.se
(No Cc of list mail needed, thanks)

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=22162956-2b8bb5
Powered by Listbox: http://www.listbox.com
Re: Patches from Robert Millan [ In reply to ]
On Friday 13 July 2007 22:20, Magnus Holmgren wrote:
> Any comments or objections to these patches?
> http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;att=1;bug=430508
> http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;att=1;bug=431239

I've looked closer at the code and wonder, Robert, if you have an example
where libspf2 gives the wrong result, in your opinion.

SPF_request_set_helo_dom() calls SPF_request_set_env_from(), with the HELO
domain unmodified, if the helo_dom field hasn't been set yet, which causes
env_from to be filled in with "postmaster@" plus the HELO domain, and
env_from_lp and env_from_dp accordingly. This *should* (I think) cause the
HELO domain to be used in all cases where it should, *provided* that
SPF_request_set_env_from() isn't called with an empty envelope sender. Hence
the bug can be seen as the fact that the documentation doesn't mention that
fact and/or that SPF_request_set_env_from() doesn't complain against, or
ignore, an empty envelope sender.

--
Magnus Holmgren holmgren@lysator.liu.se
(No Cc of list mail needed, thanks)

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=24426550-5dc12a
Powered by Listbox: http://www.listbox.com
Re: Patches from Robert Millan [ In reply to ]
On Monday 23 July 2007 15:55, Magnus Holmgren wrote:
> SPF_request_set_helo_dom() calls SPF_request_set_env_from(), with the HELO
> domain unmodified, if the helo_dom field hasn't been set yet, which causes
> env_from to be filled in with "postmaster@" plus the HELO domain, and
> env_from_lp and env_from_dp accordingly. This *should* (I think) cause the
> HELO domain to be used in all cases where it should, *provided* that
> SPF_request_set_env_from() isn't called with an empty envelope sender.
> Hence the bug can be seen as the fact that the documentation doesn't
> mention that fact and/or that SPF_request_set_env_from() doesn't complain
> against, or ignore, an empty envelope sender.

I've tested this and indeed, calling SPF_request_set_helo_dom() but not
SPF_request_set_env_from() gives the desired result. However, that approach
doesn't work if one wants to reuse the same SPF_request_t for several
queries, where the empty sender is not the first one.

I suggest that this logic

if (sr->env_from == NULL)
SPF_request_set_env_from(sr, dom);

be moved from SPF_request_set_helo_dom() to SPF_request_set_env_from().

--
Magnus Holmgren holmgren@lysator.liu.se
(No Cc of list mail needed, thanks)

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=26453240-8cdbd1
Powered by Listbox: http://www.listbox.com
Re: Patches from Robert Millan [ In reply to ]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

DISCLAIMER: I am not a libspf2 expert at all.

Magnus Holmgren wrote:
> I've tested this and indeed, calling SPF_request_set_helo_dom() but not
> SPF_request_set_env_from() gives the desired result. However, that
> approach doesn't work if one wants to reuse the same SPF_request_t for
> several queries, where the empty sender is not the first one.
>
> I suggest that this logic
>
> if (sr->env_from == NULL)
> SPF_request_set_env_from(sr, dom);
>
> be moved from SPF_request_set_helo_dom() to SPF_request_set_env_from().

But then, within SPF_request_set_env_from(), where would you get "dom" (the
HELO domain) from? Right, you'd use the value set by a prior call to
SPF_request_set_helo_dom(). But then users would be required to call
SPF_request_set_helo_dom() _first_, and SPF_request_set_env_from()
_second_.

However, existing software may not be calling these functions in this
order, despite <http://www.libspf2.org/docs/api.html> recommending it, and
would thus fail.

Perhaps the above logic could be _copied_ to SPF_request_set_env_from()
rather than _moved_?

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (GNU/Linux)

iD8DBQFGrdexwL7PKlBZWjsRAnfYAJ9H7vgdiw7RssD50zxetj7NX+PejgCdH4AL
XMN7HHFL6UDTNagGK9ChDzc=
=pC3C
-----END PGP SIGNATURE-----

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=26479642-e9ef92
Powered by Listbox: http://www.listbox.com
Re: Re: Patches from Robert Millan [ In reply to ]
On Monday 30 July 2007 14:20, Julian Mehnle wrote:
> Magnus Holmgren wrote:
> > I suggest that this logic
> >
> > if (sr->env_from == NULL)
> > SPF_request_set_env_from(sr, dom);
> >
> > be moved from SPF_request_set_helo_dom() to SPF_request_set_env_from().
>
> But then, within SPF_request_set_env_from(), where would you get "dom" (the
> HELO domain) from? Right, you'd use the value set by a prior call to
> SPF_request_set_helo_dom(). But then users would be required to call
> SPF_request_set_helo_dom() _first_, and SPF_request_set_env_from()
> _second_.
>
> Perhaps the above logic could be _copied_ to SPF_request_set_env_from()
> rather than _moved_?

Perhaps. It's would still be more localized that way than if checking for an
empty domain in numerous places, and localized is a Good Thing.

--
Magnus Holmgren holmgren@lysator.liu.se
(No Cc of list mail needed, thanks)

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=26480228-4a6244
Powered by Listbox: http://www.listbox.com
Re: Patches from Robert Millan [ In reply to ]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Magnus Holmgren wrote:
> On Monday 30 July 2007 14:20, Julian Mehnle wrote:
> > [...]
> >
> > However, existing software may not be calling these functions in this
> > order, despite <http://www.libspf2.org/docs/api.html> recommending it,
> > and would thus fail.
> >
> > Perhaps the above logic could be _copied_ to
> > SPF_request_set_env_from() rather than _moved_?
>
> Perhaps. It's would still be more localized that way than if checking
> for an empty domain in numerous places, and localized is a Good Thing.

But breaking compatibility with unknown amounts of software is about the
worst thing a library can do.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (GNU/Linux)

iD8DBQFGrgzswL7PKlBZWjsRAkHpAKDMX9W5jMYDr58xJtUypfz37ttF1wCg/Bna
riXdBr2syR7zNKspYPo3YGA=
=lixW
-----END PGP SIGNATURE-----

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=26537418-0c32a5
Powered by Listbox: http://www.listbox.com
Re: Re: Patches from Robert Millan [ In reply to ]
On Monday 30 July 2007 18:08, Julian Mehnle wrote:
> Magnus Holmgren wrote:
> > On Monday 30 July 2007 14:20, Julian Mehnle wrote:
> > > [...]
> > >
> > > However, existing software may not be calling these functions in this
> > > order, despite <http://www.libspf2.org/docs/api.html> recommending it,
> > > and would thus fail.
> > >
> > > Perhaps the above logic could be _copied_ to
> > > SPF_request_set_env_from() rather than _moved_?
> >
> > Perhaps. It's would still be more localized that way than if checking
> > for an empty domain in numerous places, and localized is a Good Thing.
>
> But breaking compatibility with unknown amounts of software is about the
> worst thing a library can do.

Absolutely. To to that we would have to bump the soname. Defining previously
undefined behaviour is a different matter, though. So what do we have here?

Some applications, including spfquery, call SPF_request_set_env_from() with an
empty 'from' argument. That sets the env_from field to "@postmaster" and the
env_from_dp field to "" and causes libspf2 to go on and look up SPF records
for "", which can't possibly be intended behaviour. IIUC, correct usage is to
call SPF_request_set_env_from() with the HELO string, which is done
automatically from SPF_request_set_helo_dom() already, or to do nothing if a
HELO check, which is essentially the same thing, has already been carried
out.

AFAICS, helo_dom is not used for anything else (outside all the legacy code
still there, enclosed in "#if 0 ... #endif" - aaargh!) except display
purposes (there is no SPF_request_query_helo() yet, and
SPF_request_t.use_helo is not used; in SPF_i_set_header_comment() we have

sender_dom = spf_request->env_from_dp;
if (sender_dom == NULL)
sender_dom = spf_request->helo_dom;

but spf_request->env_from_dp can't be NULL unless spf_request->helo_dom is
also NULL). Perhaps this is planned to change, though.

In short, my proposed patch lets "" stand for to "postmaster@"+<HELO string>
in the call to SPF_request_set_env_from(), nothing else. The problem is of
course that even if this patch is applied to the version of libspf2 in Debian
and Ubuntu, no software outside Debian and Ubuntu can depend on it.

(The patch also fixes a minor bug; that a sender address of "@domain.example"
would become "postmaster@@domain.example", with env_from_dp incorrect.

--
Magnus Holmgren holmgren@lysator.liu.se
(No Cc of list mail needed, thanks)

"Exim is better at being younger, whereas sendmail is better for
Scrabble (50 point bonus for clearing your rack)" -- Dave Evans

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=26608590-c35acd
Powered by Listbox: http://www.listbox.com
Re: Re: Patches from Robert Millan [ In reply to ]
On Monday 30 July 2007 22:26, Magnus Holmgren wrote:
> In short, my proposed patch lets "" stand for to "postmaster@"+<HELO
> string> in the call to SPF_request_set_env_from(), nothing else. The
> problem is of course that even if this patch is applied to the version of
> libspf2 in Debian and Ubuntu, no software outside Debian and Ubuntu can
> depend on it.

Since there were no further comments, I've prepared a new version including
this patch, and intend to have it uploaded shortly, unless wild protests
ensue.

The package can be found in
ftp://ftp.kibibyte.se/debian/pool/main/libs/libspf2, and the debian
subdirectory can also be browsed at http://svn.kibibyte.se/libspf2/trunk.

I've included a README.Debian containing the following note:

[...] This should
improve behaviour of applications that (incorrectly) call
SPF_request_set_env_sender() with an empty envelope sender address.
However, applications SHOULD NOT RELY ON THIS, but should check if
the envelope sender address is empty, and in that case pass the HELO
identity instead, or use the result of an earlier HELO check.

--
Magnus Holmgren holmgren@lysator.liu.se
(No Cc of list mail needed, thanks)

"Exim is better at being younger, whereas sendmail is better for
Scrabble (50 point bonus for clearing your rack)" -- Dave Evans

-------------------------------------------
-----------------------------------------------------------------------
To unsubscribe, change your address, or temporarily deactivate your
subscription,
please go to http://v2.listbox.com/member/?member_id=1311533&id_secret=34534977-1afc4c
Powered by Listbox: http://www.listbox.com