Skip to content

Commit

Permalink
added support for retrieving correct origin ip for requests coming fr…
Browse files Browse the repository at this point in the history 
…om behind a proxy (nginx/apache2/cloudflare/azure)
  • Loading branch information
@kgretzky
kgretzky committed Mar 1, 2024
1 parent e7a6866 commit 1b9cb59
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions core/http_proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,16 @@ func NewHttpProxy(hostname string, port int, cfg *Config, crt_db *CertDb, db *da
hiblue := color.New(color.FgHiBlue)

// handle ip blacklist
from_ip := req.RemoteAddr
if strings.Contains(from_ip, ":") {
from_ip = strings.SplitN(from_ip, ":", 2)[0]
from_ip := strings.SplitN(req.RemoteAddr, ":", 2)[0]

// handle proxy headers
proxyHeaders := []string{"X-Forwarded-For", "X-Real-IP", "X-Client-IP", "Connecting-IP", "True-Client-IP", "Client-IP"}
for _, h := range proxyHeaders {
origin_ip := req.Header.Get(h)
if origin_ip != "" {
from_ip = strings.SplitN(origin_ip, ":", 2)[0]
break
}
}

if p.cfg.GetBlacklistMode() != "off" {
Expand Down

8 comments on commit 1b9cb59

@M41KL-N41TT
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this is an actually faulty commit.
You shouldn't blindly trust the client's relayed IP header as this is a security vulnerability in itself.
I imagine this might break this or potentially cause some exploit where a random user could spoof the header value and cause the evilginx2 session to close down or shut completely...

You are also missing the CF-Connecting-IP header value (that one specifically is Cloudflare's header)...

https://developers.cloudflare.com/fundamentals/reference/http-request-headers/

@M41KL-N41TT
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest a total rollback of this commit @kgretzky

I can work out a patch and submit it via PR

@wick3dhub
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest a total rollback of this commit @kgretzky

I can work out a patch and submit it via PR

I saw this and rolled back, Have you been able to work around this ? i don't trust X-Forwarded-For", "X-Real-IP", "X-Client-IP", "Connecting-IP", "True-Client-IP", "Client-IP" as i think it'll be used to detect evilginx host ip

@wick3dhub
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kgretzky please take a look at this approach again

@M41KL-N41TT
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wick3dhub

I only use Cloudflare proxying (not azure/aws/etc.) so my implementation just grabs CF-Connecting-IP and if the request is coming from one of Cloudflare's IPs, I use CF-Connecting-IP as the visitor's IP address.

@M41KL-N41TT
Copy link

@M41KL-N41TT M41KL-N41TT commented on 1b9cb59 Apr 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems kgretzky isn't paying attention here.
Hey @wick3dhub. Do you code? Want I can open a repo

Im thinking...
Maybe if we make a scanner to detect all the evilginx2 sessions (technically all from this version forward),
since now those evilginx2 machines can be detected as they blindly trust ALL those headers by default.
What if we make a scanner to auto report as phishing all running evilginx2 instances to those blacklists lol...

(/s lol, joke, im aware he's only on X)

@wick3dhub
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @M41KL-N41TT , How do you use cloudflare proxying, paste logic

@kgretzky
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, just reading this.
Appreciate the comments. You are right that this can result in client spoofing the IP address by adding a bogus HTTP header with a spoofed IP value.
All in all, phished user aware and doing this would not get successfully phished, but it leaves room for malicious behaviour and I agree this should be rolled back and made optional with Evilginx user being able to specify the name of the HTTP header they want to use in their configuration. It should not be automated with HTTP header guessing.

Please sign in to comment.