NHAS / wag

Simple Wireguard 2FA
BSD 3-Clause "New" or "Revised" License
499 stars 27 forks source link

Redirect http to https on mfa portal #55

Closed bluecraank closed 10 months ago

bluecraank commented 1 year ago
NHAS commented 1 year ago

Would be nice to have a button to download a wireguard config again

Cant do this as wag does not store the wireguard private keys it generates (and the client can optionally supply a public key thus the server never knows the private key). The way you'd do this now would be to generate a new registration token. So thus, cant do it.

Maybe beside of group rules also user specific rules

I dont know what this means 100% but the Rules page can make rules for individual users as well as groups. Just dont prefix with group: and it targets a user.

Redirect http to https on mfa portal

This is a good idea!

Show only users, which do not have downloaded the config yet / did not Registration

This is already a thing. If you look under "registration tokens" that shows you the current active registration tokens. I,e the users who have not downloaded the configuration files.

atm there are also users who not have verified the session

No idea what this means. That just means for your instance of Wag you have users who havent completed MFA. This is not an issue or a bug?

on Apple iOS, sometimes there are weird behaviours on MFA Portal (Constantly redirecting, No connection established)

Without more information I cant raise this as a bug. I've never observed this myself.

Redirect http traffic for MFA routes to specific url if MFA was not completed

Not technically possible without implementing a TCP stack within eBPF which is far outside of this project.

bluecraank commented 1 year ago
  1. Okay...
  2. Oh! We didnt tried without "group:" thanks!
  3. Perfect :)
  4. This was related to the image, so there are 8 Users who are not verified right now (only public routes) but the Panel says 10 (because 2 of them need to scan qr-code and register mfa) But the label says "users have not completed MFA registration" So it should says two users
  5. Related to the image
  6. Its hard to diagnostic cause we dont find the error. It keeps reloading the mfa portal. We changed username, new config... Is there any check which reload page if some error is happening?
  7. This would be a huge feature. There are ways with iptables (https://serverfault.com/questions/586486/how-to-do-the-port-forwarding-from-one-ip-to-another-ip-in-same-network) but it cant tell you how well you can implement it
NHAS commented 1 year ago
  1. If you have 10 users that havent completed setting up MFA.... then you have 10 users who havent set up MFA and thus the web portal is going to report that. It doesnt matter if they dont have any MFA routes.

  2. This really isnt possible with how wag works. XDP/eBPF which is what wag uses to do the firewall part of what it does intercepts packets before they ever get entered into the regular kernel packet processing stack. As such it never touches iptables. To use iptables for this, you'd just have to blanket allow all traffic in wag rendering it useless.

bluecraank commented 1 year ago
  1. Yes thats clear, but if you enable wireguard and use it, you get reported as "users have not completed MFA registration" MFA "Registration" should mean: User was never before on mfa portal and submitted the code (so user will see the qr code) If you are using wireguard but you not entered the mfa code, then u should not be counted.

Hopefully understandable :)

NHAS commented 1 year ago

Sorry I dont think Im understanding what you concern is. If you're using wireguard with wag you must set up MFA regardless of whether you're using it or not.

From my understanding that MFA warning occurs when you have users that have not registered MFA. I.e have not completed the MFA flow of scanning the QR code.

NHAS commented 1 year ago

Howdy, this is now on unstable if you wanted to give it a little poke. Not sure how well made it is

mestara commented 12 months ago

I believe this is related.

Seems to cause a conflict and wont start/skip this if the port is already in use, which I have in place in this case for Lets Encrypt as an example use case (and if the port is already in use we can do this within function Apache/nginx etc). It would also cause a conflict if using the standalone certbot implementation i'd imagine.

Error: Redirect to TLS webserver public listener failed: listen tcp 172.16.27.254:80: bind: address already in use

NHAS commented 12 months ago

Yeah fair dinkum. I'll change that, thanks for the feedback!

mestara commented 12 months ago

Think this one too. The unstable build is a just running a VERY basic configuration/ports. essentially defaults with personalised info, but with no TLS cert specified in the testing environment

"Error: tunnel does not support TLS (no cert/key given)"

If I link it to config.json used for stable to start it, it loads however: Browsing to the management URL without adding /login after the port returns a "to many redirects" error With /login it brings up the login page, and allows details to be entered but then gives: "Sorry, a server error has occurred. please contact the admins if this problem persists"

(These 2 issues above may be unrelated and may need to be verified by someone else, as I've only just started testing with the unstable build)

NHAS commented 12 months ago

The unstable build is a just running a VERY basic configuration/ports. essentially defaults with personalised info, but with no TLS cert specified in the testing environment "Error: tunnel does not support TLS (no cert/key given)" If I link it to config.json used for stable to start it, it loads however:

This sounds like a configuration error on your behalf then, that error only happens if you enable webauthn and not https (as the js needs it)


        case "webauthn":
...
            if !c.Webserver.Tunnel.SupportsTLS() && !c.Proxied {
                return c, errors.New("tunnel does not support TLS (no cert/key given)")
            }
NHAS commented 12 months ago

Browsing to the management URL without adding /login after the port returns a "to many redirects" error With /login it brings up the login page, and allows details to be entered but then gives: "Sorry, a server error has occurred. please contact the admins if this problem persists"

Can you give me the server logs for this? As thats pretty odd and may be a bug. I cant currently replicate it on my deployment and that code hasnt changed on unstable.

Could you perhaps share where its directing to/from as well? (Just in the network table, but dont share the cookie)

mestara commented 12 months ago

The unstable build is a just running a VERY basic configuration/ports. essentially defaults with personalised info, but with no TLS cert specified in the testing environment "Error: tunnel does not support TLS (no cert/key given)" If I link it to config.json used for stable to start it, it loads however:

This sounds like a configuration error on your behalf then, that error only happens if you enable webauthn and not https (as the js needs it)

      case "webauthn":
...
          if !c.Webserver.Tunnel.SupportsTLS() && !c.Proxied {
              return c, errors.New("tunnel does not support TLS (no cert/key given)")
          }

that would certianly be it then! I hadnt disabled that and couldnt see what was requiring TLS, thanks :) I've put a little more effort into the testing environment now rather than being lazy :D

when running unstable undeamonised for testing this is the log output:

My use case for the management interface is its bound on the real world IP, with a gateway firewall in front of it restricting access through to that port to a specific set of IPs belonging to the network team

2023/09/13 07:05:30 Wag started successfully, Ctrl + C to stop 2023/09/13 07:05:56 myusername myip:13704 admin logged in 2023/09/13 07:05:56 token wasnt found in form

NHAS commented 12 months ago

It's not lazy! It's just a bad error message which I've updated to be clearer.

Also with that log I now know what's wrong.

I recently moved to using a session library I've built it does some csrf protections and apparently it's being a little bit aggressive here.

Thanks!

NHAS commented 12 months ago

Sweet, fixed the csrf issues and have pushed that to unstable

mestara commented 12 months ago

me again!

2023/09/13 10:32:03 Creating redirection from 80/tcp to TLS webserver public listener failed: listen tcp 103.169.14.100:80: bind: cannot assign requested address 2023/09/13 10:32:03 Redirect to TLS webserver public listener failed: listen tcp 172.16.27.254:80: bind: address already in use

git pull

Already up to date.

On a slightly related note (I figured this was minor but if you want it as a seperate issue let me know and I can split it out), which updating reminded me about. Debian 12 environment, make complains about line 3 of go.mod in unstable, specifically the .0. It works as 1.21 but not as 1.21.0

go.mod:3: invalid go version '1.21.0': must match format 1.23 make: *** [Makefile:18: .generate_ebpf] Error 1

NHAS commented 12 months ago

Im not supporting people building their own binaries. Try using the docker builder, or the provided binaries. Its just a bit finicky.

Cool I've updated that now

mestara commented 12 months ago

Im not supporting people building their own binaries. Try using the docker builder, or the provided binaries. Its just a bit finicky.

All good, I'm using the binaries in general, just messing around with unstable for testing purposes and noticed that. Easy enough to fix it as I need to for the time being :)

Would it take much to expand this feature out to the public server? as i'm testing and copying the register_device URL i inevitably find myself entering it without https and getting an error page, redirection would be nice there too

NHAS commented 12 months ago

It is on the public endpoint?

mestara commented 12 months ago

It is on the public endpoint?

Oh wow, thats terrible of me. I must have been had stable running when I was trying to test this. Yes.. yes it is, my apologies.