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

Support Forwarded Authentication, Bugfix and Dockerfile Optimization #8

Merged
merged 5 commits into from
Sep 14, 2023

Conversation

ekrekeler
Copy link
Contributor

@ekrekeler ekrekeler commented Sep 12, 2023

I spent some time writing a handler for forwarded authentication into gns3-proxy, and it works pretty well so thought I'd contribute back. I also included a small bugfix and optimizations to the Dockerfile to speed up building after making small code changes.

The authentication section of gns3_proxy.py has the biggest rewrite, since I wrote functions for repetitive code. I tested this part extensively, making sure it still behaves the same as it did before for HTTP Basic Auth, while adding the new code to take authentication from headers passed from downstream proxies.

There is one part I amended that I'd like to bring up, lines 563 to 570 of the original code are setting password to None when the password in the HTTP Basic Auth is blank. There is also a comment that explains, it happens sometimes when the GNS3 client is doing server discovery. I tested with the latest versions of the client and server, 2.2.42 and did not find any behavior like this. Additionally, my understanding of the original code's behavior in the blank password situation would be to deny the request. I cleaned up this section while maintaining that behavior, with one small difference. If a user is defined without a password in the configuration file, this user can successfully authenticate without a password. I left a note in the README about setting strong passwords because of this.

The bugfix is the inactivity-timout in the configuration file. It wasn't correctly applied to the requests and always defaulted to 300 seconds. The reason for this is that it was not getting parsed from the configuration correctly as an integer, and the HTTP class was not passing it to the Proxy class presumably for this reason.

The changes to the Dockerfile are just like I said, it makes it faster to build with cache enabled after changing one or two lines in the source code. I also merged a few lines, and removed the line creating a user. Creating users is kind of pointless in containers; it's better to set the UID and GID at container runtime. Lastly I added a versionless requirements.txt file so there is one place for any python dependencies.

I tested the finished code with python 3.7.17 and 3.11.5. I think that is good enough for compatibility but let me know if there should be more testing.

Let me know if you want me to make any changes.

@ekrekeler
Copy link
Contributor Author

One other unrelated thing, mentioning it here instead of opening an issue. The images for gns3-proxy on Docker Hub are outdated compared to the code in the develop branch. Should the image be updated (after this is merged)?

@srieger1
Copy link
Owner

One other unrelated thing, mentioning it here instead of opening an issue. The images for gns3-proxy on Docker Hub are outdated compared to the code in the develop branch. Should the image be updated (after this is merged)?

Absolutely! Same applies to pypi... I already wanted to update both multiple times already... then I thought that maybe I will remove a lot of functionality from gns3-proxy and initiate a redesign as soon as GNS3 3.x and its RBAC support are finished. However, this seams to still take some time, as also other users expressed who reached out to me and are currently working on integrating gns3-proxy in their environment.

Maybe we could move to GHCR and automatically push images based on tags again, which was dropped (but maybe now is supported again?) for free DockerHub accounts?

@srieger1
Copy link
Owner

srieger1 commented Sep 12, 2023

Thank you for this interesting and extensive PR! I think this could even remove the need for other external authentication ideas that are on the todo list given in the comments in the header of the proxy code.

I'd be really interested to know how you use the external authentication in your setup? I suppose you use GNS3 web clients, right? Some other users of gns3-proxy also reported to use it exclusively with the web client. Maybe we can get in contact to share and discuss lab setups for the proxy!

Thanks again for your effort and the excellent PR! I will merge it as soon as the "self.cleint" review question that I asked back is clarified! Also, thanks for the bug fixes you also included together with your improvements!

@ekrekeler
Copy link
Contributor Author

ekrekeler commented Sep 12, 2023

The latest commit was to fix an oopsies I found today.

Maybe we could move to GHCR and automatically push images based on tags again, which was dropped (but maybe now is supported again?) for free DockerHub accounts?

I too am waiting on the stable GNS3 3.0 release but it's been a long time in the making. I am all for leveraging GHCR to host the latest images but to be honest with you I haven't used it myself and only have experience with Docker Hub. My understanding of it is that unless you automate the builds with GitHub Actions, it is pretty much the same as manually building and pushing images to Docker Hub.

I'd be really interested to know how you use the external authentication in your setup?

I have this test setup with gns3-proxy deployed alongside 2 GNS3 servers, with traefik2 as a reverse proxy, and authentik for handling authentication. Everything uses docker compose, except the GNS3 servers which are Proxmox VMs. A single application proxy provider is configured for gns3-proxy in authentik. The documentation is here if you are interested.
Personally, I use the GNS3 desktop client since it has more administrative functionality for creating appliance templates and whatnot. However this setup also works well for users of the webclient, especially because the browser can follow SSO workflows that incorporate 2FA. Much more secure than HTTP Basic Auth (even if it's encrypted over TLS).

I will merge it as soon as the "self.cleint" review question that I asked back is clarified!

Sorry I didn't see this question, can you elaborate?

@srieger1
Copy link
Owner

Sure - I opened a review inline in your commit. You should see it in the PR. Othterwise search for "self.cleint" in gns3_proxy.py:

self.cleint.real_ip, backend_server, self.backend_port))

I think it should be "self.client"

@srieger1
Copy link
Owner

The latest commit was to fix an oopsies I found today.

perfect!

Maybe we could move to GHCR and automatically push images based on tags again, which was dropped (but maybe now is supported again?) for free DockerHub accounts?

I too am waiting on the stable GNS3 3.0 release but it's been a long time in the making. I am all for leveraging GHCR to host the latest images but to be honest with you I haven't used it myself and only have experience with Docker Hub. My understanding of it is that unless you automate the builds with GitHub Actions, it is pretty much the same as manually building and pushing images to Docker Hub.

same here. Though I thought about moving to GHCR when Docker dropped the support for free accounts for a brief moment...

I'd be really interested to know how you use the external authentication in your setup?

I have this test setup with gns3-proxy deployed alongside 2 GNS3 servers, with traefik2 as a reverse proxy, and authentik for handling authentication. Everything uses docker compose, except the GNS3 servers which are Proxmox VMs. A single application proxy provider is configured for gns3-proxy in authentik. The documentation is here if you are interested. Personally, I use the GNS3 desktop client since it has more administrative functionality for creating appliance templates and whatnot. However this setup also works well for users of the webclient, especially because the browser can follow SSO workflows that incorporate 2FA. Much more secure than HTTP Basic Auth (even if it's encrypted over TLS).

nice! We also use the desktop client - still.

@ekrekeler
Copy link
Contributor Author

self.cleint.real_ip, backend_server, self.backend_port))

I think it should be "self.client"

Whoops, another typo. Thanks for catching that.

@srieger1 srieger1 merged commit 21ce3e1 into srieger1:develop Sep 14, 2023
@srieger1
Copy link
Owner

@ekrekeler Thanks again for your PR! Merged! Will look into updating the container image later. Maybe it's also about time to release 1.0 ;)

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.

2 participants