Skip to content

Add Trusted Origins to CSRF Handler - #2301

Merged
glennzw merged 3 commits into
gophish:masterfrom
mcab:mcab/add-trusted-origins
Sep 6, 2022
Merged

Add Trusted Origins to CSRF Handler#2301
glennzw merged 3 commits into
gophish:masterfrom
mcab:mcab/add-trusted-origins

Conversation

@mcab

@mcab mcab commented Sep 5, 2021

Copy link
Copy Markdown
Contributor

With a load balancer acting as TLS-termination for the application, there have been issues where the underlying application runs into a CSRF issue:

In particular, it comes from the CSRF handler seeing the Referer header passed through as an invalid origin. This ultimately depends on how your load balancer does its negotiation. Since the application is unaware of the load balancer's existence, the application sees the forwarded request from a host it doesn't understand, and returns a 403.

To fix this, explicitly state that there are origins that we will expect requests to come from. It's assumed to be a comma-delimited string, because I didn't know how to translate multiple environment variables into a string array using jq.

Happy to work a bit more (tests, docs, etc.) to get this in.

@mcab

mcab commented Sep 5, 2021

Copy link
Copy Markdown
Contributor Author

I found that this worked for me in this setup:

gophish#2301 (2)

with the follow environment variables:

ADMIN_LISTEN_URL=0.0.0.0:8443
ADMIN_USE_TLS=false
PHISH_LISTEN_URL=0.0.0.0:8080
ADMIN_TRUSTED_ORIGINS=TARGET_UPSTREAM_DOMAIN.com

@gillescoolen

Copy link
Copy Markdown

Worked for me as well, using Traefik as a reverse proxy. Thanks!

@CiraciNicolo

Copy link
Copy Markdown

Worked for me as well, using Traefik as a reverse proxy. Thanks!

How did you get to get this working with Traefik? Is gophish served as a subdomain?

@gillescoolen

gillescoolen commented Oct 5, 2021

Copy link
Copy Markdown

Yep, I took the following steps:

  1. Set TLS to false in config.json.
  2. Add your domains to the trusted domains key in config.json
  3. Build the image from mcab:mcab/add-trusted-origins.
  4. Push the image to your registry.

Afterwards, I deploy using Docker Swarm with a Traefik reverse proxy. My docker compose file looks like this:

version: '3.2'

services:
 gophish:
  container_name: gophish
  # Use custom registry
  image: private.registry/gophish
  restart: unless-stopped
  networks:
    - proxy
  deploy:
    placement:
      constraints: [node.hostname == phish]
    labels:
      - "traefik.enable=true"
      - "traefik.docker.network=proxy"
      
      # Regular
      - "traefik.http.routers.gophish.entrypoints=http"
      - "traefik.http.routers.gophish.rule=Host(`gophish.domain.com`)"
      - "traefik.http.middlewares.gophish-https-redirect.redirectscheme.scheme=https"
      - "traefik.http.routers.gophish.middlewares=gophish-https-redirect"
      - "traefik.http.routers.gophish-secure.entrypoints=https"
      - "traefik.http.routers.gophish-secure.rule=Host(`gophish.domain.com`)"
      - "traefik.http.routers.gophish-secure.tls=true"
      - "traefik.http.routers.gophish-secure.tls.certresolver=http"
      - "traefik.http.routers.gophish-secure.service=gophish"
      - "traefik.http.services.gophish.loadbalancer.server.port=80"
      # Admin
      - "traefik.http.routers.gophish-admin.entrypoints=http"
      - "traefik.http.routers.gophish-admin.rule=Host(`admin.gophish.domain.com`)"
      - "traefik.http.middlewares.gophish-admin-https-redirect.redirectscheme.scheme=https"
      - "traefik.http.routers.gophish-admin.middlewares=gophish-admin-https-redirect"
      - "traefik.http.routers.gophish-admin-secure.entrypoints=https"
      - "traefik.http.routers.gophish-admin-secure.rule=Host(`admin.gophish.domain.com`)"
      - "traefik.http.routers.gophish-admin-secure.tls=true"
      - "traefik.http.routers.gophish-admin-secure.tls.certresolver=http"
      - "traefik.http.routers.gophish-admin-secure.service=gophish-admin"
      - "traefik.http.services.gophish-admin.loadbalancer.server.port=3333"
      
networks:
  proxy:
    external:
      name: proxy

@glennzw glennzw mentioned this pull request Dec 20, 2021
@tarrinho

tarrinho commented Dec 20, 2021

Copy link
Copy Markdown

I took the following steps:

  • git clone -b mcab/add-trusted-origins https://github.com/mcab/gophish.git
  • cd gophish
  • docker build --no-cache -t gophish -f Dockerfile .
  • docker tag gophish tarrinho/gophish-csrf-solution:gophish
  • docker push tarrinho/gophish-csrf-solution:gophish

I'm using an Apache reverse proxy: issue created about this

No TLS configured for any of the endpoints.

Still getting: "Forbidden - CSRF token invalid"
What am I missing? Please advise.

Thanks

@mcab

mcab commented Dec 21, 2021

Copy link
Copy Markdown
Contributor Author

I'm using an Apache reverse proxy: issue created about this

"trusted_origins": "https://>admin site url<",

Are you sure you specified just the domain part? From the linked issue, it seems you specified the protocol as well.

From the csrf library, it seems like it will check against the Host portion of the parsed Referer.

for _, trustedOrigin := range cs.opts.TrustedOrigins {
	if referer.Host == trustedOrigin {
		valid = true
		break
	}
}

@tarrinho

Copy link
Copy Markdown

Hi @mcab ,

I did tried it without the https:// , just -> admin.domain.com . I even tried the "master domain", without admin, just domain.com. Still the same error.

I realized that with the referer wasn't arriving in the logs of gophish, so I even changed my apache configuration:
Before in Apache configuration:
RequestHeader set Referer ""
Log:
time="2021-12-21T08:55:13Z" level=info msg=">my public IP< - - [21/Dec/2021:09:01:01 +0000] "GET / HTTP/1.1" 307 51 "" "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:95.0) Gecko/20100101 Firefox/95.0""

After in Apache configuration:
RequestHeader set Referer "https://admin.domain.com"
Log:
time="2021-12-21T09:01:44Z" level=info msg=">my public IP< - - [21/Dec/2021:09:01:44 +0000] "POST /login?next=%2F HTTP/1.1" 403 31 "https://admin.domain.com/\" "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:95.0) Gecko/20100101 Firefox/95.0""

Still the same error.

How can I do some extra analysis in the gophish side? (extra logs)

Thanks

@glennzw

glennzw commented Dec 23, 2021

Copy link
Copy Markdown
Collaborator

@tarrinho I'm not sure if you'll be able to get extra verbosity in the gophish logs, but you could insert a webproxy to output all requests + headers. Or run a tcpdump or tshark session perhaps?

@tekkamanendless

Copy link
Copy Markdown

I almost forked the repo to do this myself; glad I found this first.

I'd really like to have this feature. It would make my nginx ingress controller almost trivially-easy to support. Currently, I have to use v0.9.0 due to the addition of adminHandler = handlers.ProxyHeaders(adminHandler) in later commits, since I simply cannot make the nginx ingress strip the proxy headers, which trick the csrf code into thinking that the admin server is over HTTPS (thanks to X-Forwarded-Proto always being set to HTTPS by the ingress).

Please update config.go to use an array:

TrustedOrigins       []string   `json:"trusted_origins"`

And route.go with a simpler, smaller change:

	csrfHandler := csrf.Protect(csrfKey,
		csrf.FieldName("csrf_token"),
		csrf.Secure(as.config.UseTLS),
                csrf.TrustedOrigins(as.config.TrustedOrigins))

(You don't need to check for the TrustedOrigins value; if it's nil or [], it'll be just fine in csrf, as the default is nil there anyway.)

And finally, here's how to transform your comma-separated environment variable into a proper array in jq in run.sh:

if [ -n "${ADMIN_TRUSTED_ORIGINS+set}" ] ; then
    jq -r \
        --arg ADMIN_TRUSTED_ORIGINS "${ADMIN_TRUSTED_ORIGINS}" \
        '.admin_server.trusted_origins = ($ADMIN_TRUSTED_ORIGINS|split(","))' config.json > config.json.tmp && \
        cat config.json.tmp > config.json
fi

@tarrinho

Copy link
Copy Markdown

@tarrinho I'm not sure if you'll be able to get extra verbosity in the gophish logs, but you could insert a webproxy to output all requests + headers. Or run a tcpdump or tshark session perhaps?

The problem has to do with Apache!!!

I just added a nginx in the communication flow with the following conf and it worked:
location / {
proxy_set_header Host $host;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;

    proxy_pass              https://localhost:3000;
    proxy_read_timeout      90;
}

But I wanted to do this without the nginx in the equation! Any ideas on the Apache conf?

@sam-skinner-notarize

Copy link
Copy Markdown

@tarrinho I'm running into the same issue. Where did you add this?

@tarrinho

tarrinho commented Jul 2, 2022

Copy link
Copy Markdown

I added a Nginx server in front of the docker. I mean, your browser will connect to the Nginx server that will proxy to the docker port. With this setup, it worked for me.

I had an Apache previously doing this proxying, but I didn't manage to get it to work.

@NyaMeeEain

Copy link
Copy Markdown

I added a Nginx server in front of the docker. I mean, your browser will connect to the Nginx server that will proxy to the docker port. With this setup, it worked for me.

I had an Apache previously doing this proxying, but I didn't manage to get it to work.

can you please share your Nginx rule ?

@Spritekin

Copy link
Copy Markdown

Will this one be merged anytime?

@Spritekin

Spritekin commented Aug 10, 2022

Copy link
Copy Markdown

@mcab While I wait for this one to be merged, I pulled all the latest gophish code then added in your changes, configured and all worked flawlessly. Thank you so much for this!!!
Built a new docker image spritekin/gophish:latest

@fonsecas72

fonsecas72 commented Aug 10, 2022

Copy link
Copy Markdown

I'm trying to use gophish in a k8s cluster and i'm finding it hard to figure out how to make it work with an nginx ingress in front

I hope this PR makes it easier 🙏

Are there any plans on merging it and update the docker image ?

PS: if anyone made it to make it working with a nginx ingress in front please post the solution. 🙏

@glennzw

glennzw commented Sep 2, 2022

Copy link
Copy Markdown
Collaborator

Thanks all for your work on this and patience in getting it merged in. @mcab what are your thoughts on the comment from @tekkamanendless (#2301 (comment)) with the three small modifications?

@glennzw

glennzw commented Sep 2, 2022

Copy link
Copy Markdown
Collaborator

I've tested the PR with ngrok and it works well:

ngrok http -bind-tls=true https://localhost:3333

amending config.json:

"trusted_origins": "0a29-188-241-83-115.ngrok.io"

and we no longer get the CSRF error.

Of interest wildcards don't work (*.ngork.io), but I think that's to be expected.

@mcab

mcab commented Sep 3, 2022

Copy link
Copy Markdown
Contributor Author

Thanks all for your work on this and patience in getting it merged in. @mcab what are your thoughts on the comment from @tekkamanendless (#2301 (comment)) with the three small modifications?

Happy to get to this by the end of tomorrow. 👍

@mcab

mcab commented Sep 3, 2022

Copy link
Copy Markdown
Contributor Author

Ah. Codespaces plus vigilant mode on my account is making those commits show up as Unverified, but it incorporates the feedback from @tekkamanendless (thank you, by the way!)

This should be ready to merge.

mcab added 2 commits September 3, 2022 05:12
Enables the user to add addresses that they expect incoming connections
to come from. Helpful in cases where TLS termination is handled by a
load balancer upstream, rather than the application itself.

This does not support wildcard hosts.
If admin_server.trusted_origins is set, have the CSRF handler allow
those origins to serve up requests.
@Spritekin

Copy link
Copy Markdown

merge merge merge!

Comment thread config.json Outdated
"cert_path": "gophish_admin.crt",
"key_path": "gophish_admin.key"
"key_path": "gophish_admin.key",
"trusted_origins": ""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This should be:

"trusted_origins": []

to match the structure in config/config.go.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oof

Yep, I should have handled that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

@glennzw
glennzw merged commit 78e9a51 into gophish:master Sep 6, 2022
@glennzw

glennzw commented Sep 6, 2022

Copy link
Copy Markdown
Collaborator

Merged into master, thanks for the great new feature everyone.

@mcab

mcab commented Sep 6, 2022

Copy link
Copy Markdown
Contributor Author

Nice! Are there any documentation tidbits to update in parallel?

@mcab
mcab deleted the mcab/add-trusted-origins branch September 6, 2022 17:41
@glennzw

glennzw commented Sep 6, 2022

Copy link
Copy Markdown
Collaborator

@mcab I added some info here: https://docs.getgophish.com/user-guide/installation#understanding-the-config.json

Let me know if you think it needs anything more to better explain your contribution.

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

Successfully merging this pull request may close these issues.