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

Add function to get connection RemoteAddr #15

Closed
webfrank opened this issue Sep 10, 2018 · 5 comments
Closed

Add function to get connection RemoteAddr #15

webfrank opened this issue Sep 10, 2018 · 5 comments

Comments

@webfrank
Copy link

Hi,
it would be useful to have the remote address of the connection to implement a filter based on sender IP.
I think it could be implemented defining User struct (UserData as User is already ad interface) with RemoteAddr field filled during init of connection.

This change will impact how User is defined as at this moment the Login functions are responsible of User creation. With this change the User object is created by the connection filling all relevant data and passing it to Login functions. Is this doable?

@emersion
Copy link
Owner

I've been thinking about this. Having access to RemoteAddr is indeed required for filters and SPF/DMARC.

I guess the best way to do this would be to add a new parameter to Login and AnonymousLogin. We could also add in a TLS *tls.ConnectionState field for TLS information. I'm not sure about the name though, maybe LoginRequest would be a better one? (For consistency with http.Request)

I'm okay with breaking the API, the package is explicitly marked as unstable. Are you willing to send a PR to add this feature?

@webfrank
Copy link
Author

webfrank commented Sep 17, 2018 via email

@ksinica
Copy link

ksinica commented Oct 10, 2018

Another way might be to make dataReader "hijackable". What I mean is to add Hijack() method, that will return underlying connection (like in net/http):

type dataReader struct {
    r.   io.Reader
    conn net.Conn
		
    limited bool
    n       int64
 }
		
 func (d *dataReader) Hijack() net.Conn {
     return d.conn
 }

It would be possible to get for ex. client remote address without breaking API:

func (u *user) Send(from string, to []string, r io.Reader) error {
    type hijacker interface {
        Hijack() net.Conn
    }

    conn := r.(hijacker).Hijack()

    fmt.Println("Addr:", conn.RemoteAddr().String())
    
    // ...
}

What do you guys think?

@emersion
Copy link
Owner

That's an interesting proposal, but the semantics are a little different from net/http from what I've understood. An hijacked connection becomes unmanaged by the server. If we did the same it wouldn't (so maybe pick another name?).

Another issue is that you can't access the connection prior to the Send handler.

@emersion emersion changed the title [Feature Request] Connection RemoteAddr Add function to get connection RemoteAddr Jan 10, 2019
@foxcpp
Copy link
Collaborator

foxcpp commented Mar 18, 2019

It would also be nice to have access to the hostname reported in EHLO/HELO command. For server-server communication, we may want to check it against rDNS and actual IP of the peer.

IMO, it would be best to define a structure that contains all useful information about connection state and pass it to AnonymousLogin/Login as a third argument.
Say...

type ConnInfo struct {
  Hostname string
  RemoteAddr net.Addr
  TLS *tls.ConnectionState
}

foxcpp added a commit to foxcpp/go-smtp that referenced this issue Mar 18, 2019
foxcpp added a commit to foxcpp/go-smtp that referenced this issue Mar 18, 2019
foxcpp added a commit to foxcpp/go-smtp that referenced this issue Mar 18, 2019
foxcpp added a commit to foxcpp/go-smtp that referenced this issue Mar 18, 2019
foxcpp added a commit to foxcpp/go-smtp that referenced this issue Mar 18, 2019
foxcpp added a commit to foxcpp/go-smtp that referenced this issue Mar 18, 2019
foxcpp added a commit to foxcpp/go-smtp that referenced this issue Mar 18, 2019
emersion pushed a commit that referenced this issue Mar 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants