You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by Stefano Bagnara <ap...@bago.org> on 2008/02/23 14:49:06 UTC
Re: svn commit: r630214 - in /james/jspf/trunk/src/main/java/org/apache/james/jspf:
core/SPFSession.java impl/SPF.java
Norman Maurer ha scritto:
> Hi Stefano,
>
> by my understanding the mailserver should take care about valid format
> of the given data. So I think throwing the IllegalArgumentException is
> the best we can do.
I don't think so. We should do what the SPF RFC tell us to do.
Furthermore we currently throw an error when email address contains more
than one "@" [1] that is not correct because its perfectly valid to have
the @ in the local part if it is correctly excaped:
The following addresses are valid per RFC:
"test@foo"@example.com
test\@foo@example.com
For the SPF concerns I would say that we should take everyrhing we find
before the last @ and consider it localpart with no parsing or validation.
The most important thing is that the SPF library have to return ALWAYS
an error defined in the SPF RFC because returning Unchecked Exceptions
would leave the library user in doubt about what to do. We are the RFC
implementors and we are the ones that should no what is appropriate when
invalid data is given in input.
RFC4408 4.3 (Initial Processing) tells us:
-----
If the <domain> is malformed (label longer than 63 characters,
zero-length label not at the end, etc.) or is not a fully qualified
domain name, or if the DNS lookup returns "domain does not exist" (RCODE
3), check_host() immediately returns the result "None".
If the <sender> has no localpart, substitute the string "postmaster" for
the localpart.
------
RFC4408 2.4 tells us:
-----------
Implementations must take care to correctly extract the <domain> from
the data given with the SMTP MAIL FROM command as many MTAs will still
accept such things as source routes (see RFC 2821, Appendix C), the
%-hack (see RFC 1123), and bang paths (see RFC 1983). These archaic
features have been maliciously used to bypass security systems.
So we have to check the domain (and we do this because we pass the
testsuite) and check if the localpart is empty.
-------------
So they basically say that we MUST be able to check SPF for similar
addresses: "@ONE,@TWO:JOE@THREE" (this is an example from "Appendix C
Source Routes" in RFC2821)
Stefano
[1] --------------------------
String[] fromParts = mailFrom.split("@");
// should never be bigger as 2 !
if (fromParts.length > 2) {
throw new IllegalArgumentException("Not a valid email address " +
mailFrom);
} else if (fromParts.length == 2) {
---------------------------------
> bye
> Norman
>
> Am Samstag, den 23.02.2008, 13:55 +0100 schrieb Stefano Bagnara:
>> Norman Maurer ha scritto:
>>> What you guy think should we maybe use a checked exception ?
>> As far as I can tell the SPF check MUST return one of the return codes
>> defined by the RFC, so PermErrorException or NoneException seems to be
>> the best answer.. but I have to reread the SPF RFC to see if they
>> specify what to do with malformed email addresses.
>>
>> We should leave all the None/PermError that was there before this change
>> and simply catch the NPE reported by the user and rethrow a
>> None/PermError (depending on the RFC).
>>
>> Stefano
>>
>>> Cheers
>>> Norman
>>>
>>> Am Freitag, den 22.02.2008, 14:17 +0000 schrieb norman@apache.org:
>>>> Author: norman
>>>> Date: Fri Feb 22 06:17:19 2008
>>>> New Revision: 630214
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=630214&view=rev
>>>> Log:
>>>> Throw IllegalArgumentException on invalid data given for SPFSession. See JSPF-60
>>>>
>>>> Modified:
>>>> james/jspf/trunk/src/main/java/org/apache/james/jspf/core/SPFSession.java
>>>> james/jspf/trunk/src/main/java/org/apache/james/jspf/impl/SPF.java
>>>>
>>>> Modified: james/jspf/trunk/src/main/java/org/apache/james/jspf/core/SPFSession.java
>>>> URL: http://svn.apache.org/viewvc/james/jspf/trunk/src/main/java/org/apache/james/jspf/core/SPFSession.java?rev=630214&r1=630213&r2=630214&view=diff
>>>> ==============================================================================
>>>> --- james/jspf/trunk/src/main/java/org/apache/james/jspf/core/SPFSession.java (original)
>>>> +++ james/jspf/trunk/src/main/java/org/apache/james/jspf/core/SPFSession.java Fri Feb 22 06:17:19 2008
>>>> @@ -86,23 +86,22 @@
>>>> * The helo provided by the sender
>>>> * @param clientIP
>>>> * The ipaddress of the client
>>>> - * @throws PermErrorException
>>>> + * @throws IllegalArgumentException
>>>> * Get thrown if invalid data get passed
>>>> - * @throws NoneException
>>>> - * Get thrown if no valid emailaddress get passed
>>>> + *
>>>> */
>>>> - public SPFSession(String mailFrom, String heloDomain, String clientIP) throws PermErrorException, NoneException {
>>>> + public SPFSession(String mailFrom, String heloDomain, String clientIP) {
>>>> super();
>>>> this.mailFrom = mailFrom.trim();
>>>> this.hostName = heloDomain.trim();
>>>> - this.ipAddress = IPAddr.getProperIpAddress(clientIP.trim());
>>>> -
>>>> +
>>>> try {
>>>> + this.ipAddress = IPAddr.getProperIpAddress(clientIP.trim());
>>>> // get the in Address
>>>> this.inAddress = IPAddr.getInAddress(clientIP);
>>>> } catch (PermErrorException e) {
>>>> // throw an exception cause the ip was not rfc conform
>>>> - throw new PermErrorException(e.getMessage());
>>>> + throw new IllegalArgumentException(e.getMessage());
>>>> }
>>>>
>>>> // setup the data!
>>>> @@ -119,7 +118,7 @@
>>>> * @throws NoneException
>>>> * Get thrown if an invalid emailaddress get passed
>>>> */
>>>> - private void setupData(String mailFrom, String helo) throws NoneException {
>>>> + private void setupData(String mailFrom, String helo) {
>>>>
>>>> // if nullsender is used postmaster@helo will be used as email
>>>> if (mailFrom.equals("")) {
>>>> @@ -131,7 +130,7 @@
>>>>
>>>> // should never be bigger as 2 !
>>>> if (fromParts.length > 2) {
>>>> - throw new NoneException("Not a valid email address " + mailFrom);
>>>> + throw new IllegalArgumentException("Not a valid email address " + mailFrom);
>>>> } else if (fromParts.length == 2) {
>>>> this.currentSenderPart = fromParts[0];
>>>> this.senderDomain = fromParts[1];
>>>>
>>>> Modified: james/jspf/trunk/src/main/java/org/apache/james/jspf/impl/SPF.java
>>>> URL: http://svn.apache.org/viewvc/james/jspf/trunk/src/main/java/org/apache/james/jspf/impl/SPF.java?rev=630214&r1=630213&r2=630214&view=diff
>>>> ==============================================================================
>>>> --- james/jspf/trunk/src/main/java/org/apache/james/jspf/impl/SPF.java (original)
>>>> +++ james/jspf/trunk/src/main/java/org/apache/james/jspf/impl/SPF.java Fri Feb 22 06:17:19 2008
>>>> @@ -313,13 +313,8 @@
>>>> SPFSession spfData = null;
>>>>
>>>> // Setup the data
>>>> - try {
>>>> - spfData = new SPFSession(mailFrom, hostName, ipAddress);
>>>> - } catch (PermErrorException e1) {
>>>> - spfData.setCurrentResultExpanded(e1.getResult());
>>>> - } catch (NoneException e1) {
>>>> - spfData.setCurrentResultExpanded(e1.getResult());
>>>> - }
>>>> + spfData = new SPFSession(mailFrom, hostName, ipAddress);
>>>> +
>>>>
>>>> SPFChecker resultHandler = new DefaultSPFChecker();
>>>>
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>>>> For additional commands, e-mail: server-dev-help@james.apache.org
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>>> For additional commands, e-mail: server-dev-help@james.apache.org
>>>
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>> For additional commands, e-mail: server-dev-help@james.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: svn commit:
r630214 - in /james/jspf/trunk/src/main/java/org/apache/james/jspf:
core/SPFSession.java impl/SPF.java
Posted by Norman Maurer <no...@apache.org>.
Am Samstag, den 23.02.2008, 15:22 +0100 schrieb Stefano Bagnara:
> Norman Maurer ha scritto:
> > Hi Stefano,
> >
> > after rereading I think you are right... What you think we should do ?
> > Just throw the exceptions ( NoneException, PermErrorException ) on
> > checkSPF method ?
>
> We probably have to change SPFSession so that it doesn't throw an
> exception on constructor but instead internally sets its (error) result.
>
> The DefaultSPFChecker should already skip the remaining processing if
> spfData.getCurrentResultExpanded() returns something not null.
>
> I have no time at the moment, I'll be back in few hours and if you don't
> already changed it I'll try a fix for review.
>
> Thank you,
> Stefano
I have to get some stuff done today too.. So i will have no time until
tomorrow. We will se who fix it ( you or me ).
Thx
Norman
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: svn commit: r630214 - in /james/jspf/trunk/src/main/java/org/apache/james/jspf:
core/SPFSession.java impl/SPF.java
Posted by Stefano Bagnara <ap...@bago.org>.
Norman Maurer ha scritto:
> Hi Stefano,
>
> after rereading I think you are right... What you think we should do ?
> Just throw the exceptions ( NoneException, PermErrorException ) on
> checkSPF method ?
We probably have to change SPFSession so that it doesn't throw an
exception on constructor but instead internally sets its (error) result.
The DefaultSPFChecker should already skip the remaining processing if
spfData.getCurrentResultExpanded() returns something not null.
I have no time at the moment, I'll be back in few hours and if you don't
already changed it I'll try a fix for review.
Thank you,
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: svn commit: r630214
- in /james/jspf/trunk/src/main/java/org/apache/james/jspf:
core/SPFSession.java impl/SPF.java
Posted by Norman Maurer <no...@apache.org>.
Hi Stefano,
after rereading I think you are right... What you think we should do ?
Just throw the exceptions ( NoneException, PermErrorException ) on
checkSPF method ?
bye
Norman
Am Samstag, den 23.02.2008, 14:49 +0100 schrieb Stefano Bagnara:
> Norman Maurer ha scritto:
> > Hi Stefano,
> >
> > by my understanding the mailserver should take care about valid format
> > of the given data. So I think throwing the IllegalArgumentException is
> > the best we can do.
>
> I don't think so. We should do what the SPF RFC tell us to do.
>
> Furthermore we currently throw an error when email address contains more
> than one "@" [1] that is not correct because its perfectly valid to have
> the @ in the local part if it is correctly excaped:
>
> The following addresses are valid per RFC:
> "test@foo"@example.com
> test\@foo@example.com
>
> For the SPF concerns I would say that we should take everyrhing we find
> before the last @ and consider it localpart with no parsing or validation.
>
> The most important thing is that the SPF library have to return ALWAYS
> an error defined in the SPF RFC because returning Unchecked Exceptions
> would leave the library user in doubt about what to do. We are the RFC
> implementors and we are the ones that should no what is appropriate when
> invalid data is given in input.
>
> RFC4408 4.3 (Initial Processing) tells us:
> -----
> If the <domain> is malformed (label longer than 63 characters,
> zero-length label not at the end, etc.) or is not a fully qualified
> domain name, or if the DNS lookup returns "domain does not exist" (RCODE
> 3), check_host() immediately returns the result "None".
>
> If the <sender> has no localpart, substitute the string "postmaster" for
> the localpart.
> ------
>
> RFC4408 2.4 tells us:
> -----------
> Implementations must take care to correctly extract the <domain> from
> the data given with the SMTP MAIL FROM command as many MTAs will still
> accept such things as source routes (see RFC 2821, Appendix C), the
> %-hack (see RFC 1123), and bang paths (see RFC 1983). These archaic
> features have been maliciously used to bypass security systems.
>
> So we have to check the domain (and we do this because we pass the
> testsuite) and check if the localpart is empty.
> -------------
> So they basically say that we MUST be able to check SPF for similar
> addresses: "@ONE,@TWO:JOE@THREE" (this is an example from "Appendix C
> Source Routes" in RFC2821)
>
> Stefano
>
> [1] --------------------------
> String[] fromParts = mailFrom.split("@");
>
> // should never be bigger as 2 !
> if (fromParts.length > 2) {
> throw new IllegalArgumentException("Not a valid email address " +
> mailFrom);
> } else if (fromParts.length == 2) {
> ---------------------------------
>
> > bye
> > Norman
> >
> > Am Samstag, den 23.02.2008, 13:55 +0100 schrieb Stefano Bagnara:
> >> Norman Maurer ha scritto:
> >>> What you guy think should we maybe use a checked exception ?
> >> As far as I can tell the SPF check MUST return one of the return codes
> >> defined by the RFC, so PermErrorException or NoneException seems to be
> >> the best answer.. but I have to reread the SPF RFC to see if they
> >> specify what to do with malformed email addresses.
> >>
> >> We should leave all the None/PermError that was there before this change
> >> and simply catch the NPE reported by the user and rethrow a
> >> None/PermError (depending on the RFC).
> >>
> >> Stefano
> >>
> >>> Cheers
> >>> Norman
> >>>
> >>> Am Freitag, den 22.02.2008, 14:17 +0000 schrieb norman@apache.org:
> >>>> Author: norman
> >>>> Date: Fri Feb 22 06:17:19 2008
> >>>> New Revision: 630214
> >>>>
> >>>> URL: http://svn.apache.org/viewvc?rev=630214&view=rev
> >>>> Log:
> >>>> Throw IllegalArgumentException on invalid data given for SPFSession. See JSPF-60
> >>>>
> >>>> Modified:
> >>>> james/jspf/trunk/src/main/java/org/apache/james/jspf/core/SPFSession.java
> >>>> james/jspf/trunk/src/main/java/org/apache/james/jspf/impl/SPF.java
> >>>>
> >>>> Modified: james/jspf/trunk/src/main/java/org/apache/james/jspf/core/SPFSession.java
> >>>> URL: http://svn.apache.org/viewvc/james/jspf/trunk/src/main/java/org/apache/james/jspf/core/SPFSession.java?rev=630214&r1=630213&r2=630214&view=diff
> >>>> ==============================================================================
> >>>> --- james/jspf/trunk/src/main/java/org/apache/james/jspf/core/SPFSession.java (original)
> >>>> +++ james/jspf/trunk/src/main/java/org/apache/james/jspf/core/SPFSession.java Fri Feb 22 06:17:19 2008
> >>>> @@ -86,23 +86,22 @@
> >>>> * The helo provided by the sender
> >>>> * @param clientIP
> >>>> * The ipaddress of the client
> >>>> - * @throws PermErrorException
> >>>> + * @throws IllegalArgumentException
> >>>> * Get thrown if invalid data get passed
> >>>> - * @throws NoneException
> >>>> - * Get thrown if no valid emailaddress get passed
> >>>> + *
> >>>> */
> >>>> - public SPFSession(String mailFrom, String heloDomain, String clientIP) throws PermErrorException, NoneException {
> >>>> + public SPFSession(String mailFrom, String heloDomain, String clientIP) {
> >>>> super();
> >>>> this.mailFrom = mailFrom.trim();
> >>>> this.hostName = heloDomain.trim();
> >>>> - this.ipAddress = IPAddr.getProperIpAddress(clientIP.trim());
> >>>> -
> >>>> +
> >>>> try {
> >>>> + this.ipAddress = IPAddr.getProperIpAddress(clientIP.trim());
> >>>> // get the in Address
> >>>> this.inAddress = IPAddr.getInAddress(clientIP);
> >>>> } catch (PermErrorException e) {
> >>>> // throw an exception cause the ip was not rfc conform
> >>>> - throw new PermErrorException(e.getMessage());
> >>>> + throw new IllegalArgumentException(e.getMessage());
> >>>> }
> >>>>
> >>>> // setup the data!
> >>>> @@ -119,7 +118,7 @@
> >>>> * @throws NoneException
> >>>> * Get thrown if an invalid emailaddress get passed
> >>>> */
> >>>> - private void setupData(String mailFrom, String helo) throws NoneException {
> >>>> + private void setupData(String mailFrom, String helo) {
> >>>>
> >>>> // if nullsender is used postmaster@helo will be used as email
> >>>> if (mailFrom.equals("")) {
> >>>> @@ -131,7 +130,7 @@
> >>>>
> >>>> // should never be bigger as 2 !
> >>>> if (fromParts.length > 2) {
> >>>> - throw new NoneException("Not a valid email address " + mailFrom);
> >>>> + throw new IllegalArgumentException("Not a valid email address " + mailFrom);
> >>>> } else if (fromParts.length == 2) {
> >>>> this.currentSenderPart = fromParts[0];
> >>>> this.senderDomain = fromParts[1];
> >>>>
> >>>> Modified: james/jspf/trunk/src/main/java/org/apache/james/jspf/impl/SPF.java
> >>>> URL: http://svn.apache.org/viewvc/james/jspf/trunk/src/main/java/org/apache/james/jspf/impl/SPF.java?rev=630214&r1=630213&r2=630214&view=diff
> >>>> ==============================================================================
> >>>> --- james/jspf/trunk/src/main/java/org/apache/james/jspf/impl/SPF.java (original)
> >>>> +++ james/jspf/trunk/src/main/java/org/apache/james/jspf/impl/SPF.java Fri Feb 22 06:17:19 2008
> >>>> @@ -313,13 +313,8 @@
> >>>> SPFSession spfData = null;
> >>>>
> >>>> // Setup the data
> >>>> - try {
> >>>> - spfData = new SPFSession(mailFrom, hostName, ipAddress);
> >>>> - } catch (PermErrorException e1) {
> >>>> - spfData.setCurrentResultExpanded(e1.getResult());
> >>>> - } catch (NoneException e1) {
> >>>> - spfData.setCurrentResultExpanded(e1.getResult());
> >>>> - }
> >>>> + spfData = new SPFSession(mailFrom, hostName, ipAddress);
> >>>> +
> >>>>
> >>>> SPFChecker resultHandler = new DefaultSPFChecker();
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> >>>> For additional commands, e-mail: server-dev-help@james.apache.org
> >>>>
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> >>> For additional commands, e-mail: server-dev-help@james.apache.org
> >>>
> >>>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> >> For additional commands, e-mail: server-dev-help@james.apache.org
> >>
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> > For additional commands, e-mail: server-dev-help@james.apache.org
> >
> >
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org