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

Get IP from "standard" HTTP headers #3287

Open
S7evinK opened this issue Dec 15, 2023 · 4 comments
Open

Get IP from "standard" HTTP headers #3287

S7evinK opened this issue Dec 15, 2023 · 4 comments
Labels
good first issue Want to help with Dendrite? These are the issues to start with! help wanted More difficult than good-first-issue but not impossible! T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@S7evinK
Copy link
Contributor

S7evinK commented Dec 15, 2023

...

But, if I may, do you think Dendrite should perhaps auto-try any of the standard "this is the client's real IP" headers automatically? Do you see any downsides in doing that? For what it's worth, from my limited self-hosting experience, many applications do it automatically, given the current trends in hosting stuff (everything behind reverse proxy or ingress or whatever)

Best regards
Zbig

Originally posted by @zbig-t in #3286 (comment)

@S7evinK S7evinK added good first issue Want to help with Dendrite? These are the issues to start with! help wanted More difficult than good-first-issue but not impossible! T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. labels Dec 15, 2023
@S7evinK
Copy link
Contributor Author

S7evinK commented Dec 15, 2023

Change somewhere here (or in this file):

func (rp *RequestPool) updateLastSeen(req *http.Request, device *userapi.Device) {
if _, ok := rp.lastseen.LoadOrStore(device.UserID+device.ID, struct{}{}); ok {
return
}
remoteAddr := req.RemoteAddr
if rp.cfg.RealIPHeader != "" {
if header := req.Header.Get(rp.cfg.RealIPHeader); header != "" {
// TODO: Maybe this isn't great but it will satisfy both X-Real-IP
// and X-Forwarded-For (which can be a list where the real client
// address is the first listed address). Make more intelligent?
addresses := strings.Split(header, ",")
if ip := net.ParseIP(addresses[0]); ip != nil {
remoteAddr = addresses[0]
}
}
}
lsreq := &userapi.PerformLastSeenUpdateRequest{
UserID: device.UserID,
DeviceID: device.ID,
RemoteAddr: remoteAddr,
UserAgent: req.UserAgent(),
}
lsres := &userapi.PerformLastSeenUpdateResponse{}
go rp.userAPI.PerformLastSeenUpdate(req.Context(), lsreq, lsres) // nolint:errcheck
rp.lastseen.Store(device.UserID+device.ID, time.Now())
}

@zbig-t
Copy link

zbig-t commented Dec 15, 2023

Thanks, I think (hope) that's not beyond my abilities. Will create a PR once I mange to take care of that.

@Curious-r
Copy link

I think it's necessary.
Now I'm use X-Forwarded-For instead of X-Real-ip, because the former exists in a lot of reverse proxies as a standard header。

@bones-was-here
Copy link

None of these headers are safe to trust in the default configuration, unless Dendrite will never use the information for anything important.

To be trustworthy the IP header must be set by a trusted reverse proxy that also discards any (potentially spoofed) information it receives in these headers. The various proxy implementations have different default behaviours, might not be using their defaults, or the admin might not be using a proxy at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Want to help with Dendrite? These are the issues to start with! help wanted More difficult than good-first-issue but not impossible! T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

No branches or pull requests

4 participants