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

Web ssl #282

Closed
wants to merge 3 commits into from
Closed

Web ssl #282

wants to merge 3 commits into from

Conversation

SandyChapman
Copy link

Based on code / comments in #278


The Locust web interface also supports SSL encrypted connections. This is especially useful
if, for instance, you're testing an API which may incidentally expose personal information
or application secrets through the web interface.
Copy link
Member

Choose a reason for hiding this comment

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

I don't follow this argument. Any sensitive data would be sent from the locust boxes to the application under load, which depends on the application having SSL. What sensitive data would exist in the UI and how would SSL protect that?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not crazy about the wording either.,, perhaps you can give it another shot.

But the web UI displays the hostname being tested and the url's you are hitting. That might be disclosing more info over cleartext than someone is comfortable with.

Copy link
Member

Choose a reason for hiding this comment

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

I still question the value for the wider locust audience. SSL encrypted UI may prevent MITM, but it's irrelevant if the page is unsecured. Related: #284. My concern here is the same as the comment there. In my opinion, it's cleaner and more secure to attach layers of security on top of locust rather than trying to make the locust page secure. If the encryption and auth are important for your organization, the time investment to set up nginx + let's encrypt with basic auth is relatively small.

Copy link
Member

Choose a reason for hiding this comment

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

lets close this then

Copy link
Author

Choose a reason for hiding this comment

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

@justiniso , @cgoldberg : Our use case was to run locust distributed across several AWS EC2 instances. In such a case, the provisioning of SSL and nginx would have additional complexity. I understand if you guys want to keep locust insecure by default and let everyone have to add their own security as you guys may not have the time to maintain these features.

To answer your previous question, sensitive information can appear in the paths being used which are displayed in the UI. For instance, URL encoded forms that may be used for authentication (not that I used this, but there could be APIs that do use it).

I see value in these features specifically because we had a real-life client specifically ask for them (multi-billion dollar corp).

@justiniso justiniso closed this Feb 13, 2017
@mattdodge
Copy link
Contributor

I'm a little bummed this was closed - it's a pretty straightforward feature and I see merit in it. I wouldn't expect it to be SSL-enabled by default, but if someone has a key/cert and they want to serve the UI over https they can't do that? Or can't without setting up nginx or some other kind of reverse proxy?

I'd like to spin up a locust server in our private cluster and share the URL with my team. I'd feel a lot better sharing an https URL than an http one. There's no specific attack vector I'm worried about, moreso that it's 2020 and the web should be secure by default. Prohibiting this when it's just a 2 line code change seems a bit strange, but those are just my 2 cents.

@heyman
Copy link
Member

heyman commented Apr 25, 2020

I think we should consider accepting an updated PR for this, especially now that we've added Basic Auth support to the Web UI.

@mattdodge
Copy link
Contributor

Agreed, thanks @heyman - took a crack at it here #1354

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.

5 participants