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.

Sign up for GitHub

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

Jump to bottom

The “header” processor populates “Received:” headers wrongly. #198

Closed
issuefiler opened this issue Nov 22, 2019 · 3 comments
Closed

The “header” processor populates “Received:” headers wrongly. #198

issuefiler opened this issue Nov 22, 2019 · 3 comments
Labels
lgtm

Comments

@issuefiler
Copy link

issuefiler commented Nov 22, 2019 •
edited
Loading

RFC 5321

   From-domain    = "FROM" FWS Extended-Domain

   By-domain      = CFWS "BY" FWS Extended-Domain

   Extended-Domain  = Domain /
                    ( Domain FWS "(" TCP-info ")" ) /
                    ( address-literal FWS "(" TCP-info ")" )

   TCP-info       = address-literal / ( Domain FWS address-literal )
                  ; Information derived by server from TCP connection
                  ; not client EHLO.

go-guerrilla/backends/p_header.go

Line 59 in 51f7dda

addHead += "Received: from " + e.Helo + " (" + e.Helo + " [" + e.RemoteIP + "])\n"

A Domain in a TCP-info should not be from the HELO command. Would better omit it as go-guerrilla doesn’t do rDNS lookup.

And replacing from with FROM would better comply with the RFC. (Unsure if case-insensitive.)

go-guerrilla/backends/p_header.go

Line 61 in 51f7dda

addHead += " by " + e.RcptTo[0].Host + " with SMTP id " + hash + "@" + e.RcptTo[0].Host + ";\n"

I can’t say it is wrong for this one, just want to know your opinion on changing with SMTP to WITH ESMTPS, as go-guerrilla supports both EHLO and STARTTLS (which might be a poor reason to back it).
@flashmob
Copy link
Owner

flashmob commented Nov 22, 2019 •
edited
Loading

Hi, thanks for the bug reports - they're excellently written!
Sorry, no time to respond them yet, will soon.

@flashmob
Copy link
Owner

Thanks.

TODO

  • Remove e.Helo from the tcp-info header, only include the source IP in the tcp info
  • Change with SMTP to with ESMTPS' / with ESMTP`, depending on what protocol was used

As for, FROM - in the wild, eg. Postfix, Gmail always leave it all lowercase.

@flashmob flashmob added the todo label Nov 27, 2019
flashmob added a commit that referenced this issue Dec 12, 2019
@flashmob
issue #198 - remove helo/ehlo from the population on headers
a32e269
@flashmob
Copy link
Owner

flashmob commented Dec 13, 2019 •
edited
Loading

Implemented in #202

  • Remove e.Helo from the tcp-info header, only include the source IP in the tcp info
  • Change with SMTP to with ESMTPS/ with ESMTP, depending on what protocol was used

@flashmob flashmob added testing and removed todo labels Dec 13, 2019
@flashmob flashmob mentioned this issue Dec 13, 2019
Merged
flashmob added a commit that referenced this issue Dec 27, 2019
@flashmob
integration test for issue #198
54640d5
@flashmob flashmob added lgtm and removed testing labels Dec 27, 2019
flashmob added a commit that referenced this issue Dec 27, 2019
@flashmob
Issues #195 to #201 (#202)
d8ea544 
- Parser captures quoted local-parts without the escape characters
- mail.Address.String() know when to quote local-part, 
- server's `allowsHost` function is ipv6 address aware (addresses specified in the config will get normalized to their ipv6 simplest form, addresses parsed from RCPT and MAIL commands will have ipv6 normalized)
- if `<postmaster>` is used in the RCPT TO (without a host), then new functionality was added to assume that the host is assumed to be the Hostname setting for the Server
- HELO/EHLO argument validation. #200
- The “header” processor populates “Received:” headers wrongly. #198
-  tiny bug in “p_redis.go”. #196
- “MimeHeaderDecode” (envelope.go) returns an incorrectly-spaced string. #195
- go-guerrilla cannot properly handle some valid addresses. #199
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
lgtm
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@flashmob @issuefiler