Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pyspf allows invalid ALPHA/DIGIT in alphanum #12

Open
kitterma opened this issue Mar 1, 2019 · 4 comments
Open

pyspf allows invalid ALPHA/DIGIT in alphanum #12

kitterma opened this issue Mar 1, 2019 · 4 comments

Comments

@kitterma
Copy link
Collaborator

kitterma commented Mar 1, 2019

I believe that the example below should error out due to the '|' in the included domain before attempting a DNS lookup:

$ pyspf "v=spf1 include:a.b.com|c.d.com|e.f.com ~all" 1.1.1.1 [email protected] test.kitterman.com
result: ('permerror', 550, 'SPF Permanent Error: No valid SPF record for included domain: a.b.com|c.d.com|e.f.com: include:a.b.com|c.d.com|e.f.com') ~all

Since neither ALPHA nor DIGIT have "|", I think it's invalid per the ABNF.

@sdgathman
Copy link
Owner

sdgathman commented Apr 1, 2019

That at least needs a test case for now. But how will the test suite framework check how many lookups were done? I think the framework needs to provide that information to the tests. For now, checking for a different Permerror explanation might be sufficient.

@sdgathman
Copy link
Owner

RFC 7208 says:

   Note: This document and its predecessors make no provisions for
   defining correct handling of a syntactically invalid <domain-spec>
   (which might be the result of macro expansion), per [RFC1035].

So the result will be undefined, like the invalid-domain-long test case.

The goal seems to be to prevent useless DNS lookups.

@sdgathman
Copy link
Owner

RFC 7208 7.1/2 says:

   domain-spec      = macro-string domain-end
   domain-end       = ( "." toplabel [ "." ] ) / macro-expand
   macro-string     = *( macro-expand / macro-literal )
   macro-expand     = ( "%{" macro-letter transformers *delimiter "}" )
                      / "%%" / "%_" / "%-"
   macro-literal    = %x21-24 / %x26-7E
                      ; visible characters except "%"

We even have a test case with '/' in the domain-spec. So, I'm afraid the '|' is valid per SPF syntax. BUT, since invalid hostname per RFC 1035 is undefined, maybe we could avoid the lookup without breaking anything. This issue is on hold while RFC lawyers weigh in.

@sdgathman
Copy link
Owner

@kitterma Since the result is undefined, current behavior is correct. If you have a reliable and efficient test for a valid domain-spec per RFC 1035 that we could do after macro expansion, then if might be a win to avoid the DNS lookup. Since the result is undefined per RFC 7208 - we can make the result of an invalid domain-spec PermErr since we think the policy owner needs to fix their policy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants