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

Don't redirect to HTTPS for API #2416

Closed
Sjord opened this issue Nov 27, 2024 · 49 comments · Fixed by #2529
Closed

Don't redirect to HTTPS for API #2416

Sjord opened this issue Nov 27, 2024 · 49 comments · Fixed by #2529
Assignees
Labels
6) PR awaiting review V13 _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@Sjord
Copy link
Contributor

Sjord commented Nov 27, 2024

Your API Shouldn't Redirect HTTP to HTTPS

The argument is that when a client uses 'http://api.example.org', it should fail instead of silently be insecure.

I propose to add a requirement that specifies what behavior a site should have when the HTTP version is accessed, whether it should redirect or return an error.

@elarlang
Copy link
Collaborator

Situation at the moment:

# Description L1 L2 L3 CWE
9.1.1 [MODIFIED] Verify that TLS is used for all connectivity between a client and external facing, HTTP-based services, and does not fall back to insecure or unencrypted communications. 319
9.2.2 [MODIFIED] Verify that an encrypted protocol such as TLS is used for all inbound and outbound connections to and from the application, including monitoring systems, management tools, remote access and SSH, middleware, databases, mainframes, partner systems, or external APIs. The server must not fall back to insecure or unencrypted protocols. 319
9.3.1 [ADDED] Verify that TLS or another appropriate transport encryption mechanism used for all connectivity between internal, HTTP-based services within the application, and does not fall back to insecure or unencrypted communications. 319

Section titles to better understand the context of the requirements above:

  • V9.1 HTTPS Communication with External Facing Services
  • V9.2 General Service to Service Communication Security
  • V9.3 HTTPS Communication between Internal Services

I find it quite complicated to write a requirement "If you use http: for an API, then don't auto-redirect to https:" as we disallow use http: in any situation.

If we find it worth to be mentioned, I think it should be done in 9.2.2.

@elarlang elarlang added V9 _5.0 - prep This needs to be addressed to prepare 5.0 labels Nov 27, 2024
@randomstuff
Copy link
Contributor

I find it quite complicated to write a requirement "If you use http: for an API, then don't auto-redirect to https:" as we disallow use http: in any situation.

I don't think that 9.1.1, 9.2.2 or 9.3.1 intends to disallow listening on http: for redirecting browsers to HTTPS.

Even if you are certain that all your software is properly communicating using https:, third party applications communicating with your application might use http: by mistake. This makes sure that this problematic configuration does not go unnoticed.

@elarlang
Copy link
Collaborator

elarlang commented Nov 28, 2024

This makes sure that this problematic configuration does not go unnoticed.

I agree it decreases the likelihood, but at the same time it must be clear, that it does not fix the issue.

If a third party app communicates over http: and there is already man-in-the-middle (MiTM) in place, MiTM participant can do the redirect itself - it works "correctly" for the third party client and it works "correctly" for the service provider.

@elarlang elarlang added V13 and removed V9 labels Nov 28, 2024
@tghosth tghosth added the 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet label Nov 28, 2024
@elarlang
Copy link
Collaborator

The points to achieve:

  • redirect to https only navigation requests
  • do not redirect if the request contains sensitive data
    • give an error as a response
    • do not listen on http: (port 80) at all (for API)

@tghosth
Copy link
Collaborator

tghosth commented Dec 2, 2024

Ok, I think we need to focus this on API calls specifically here.

It seems unlikely but not impossible that you would use Cookies with an API so I don't think we can limit to just tokens in headers.

Since this would apply for non-browser clients which would not necessarily comply with Secure flag and HSTS.

Overall, I would propose:

13.1.8 - Verify that API endpoints will not respond to a non-HTTPS request, even with with a redirect to the HTTPS endpoint. This is to avoid API clients accidentally sending data over plaintext HTTP but this not being discovered due to an automatic redirect.

(Level 3)

Thoughts @Sjord @elarlang @randomstuff

@tghosth tghosth added 4) proposal for review Issue contains clear proposal for add/change something and removed 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet labels Dec 2, 2024
@elarlang
Copy link
Collaborator

elarlang commented Dec 2, 2024

It is a wider topic than API. For example, redirects using OAuth or some SSO and containing secrets or sensitive information.

@randomstuff
Copy link
Contributor

randomstuff commented Dec 2, 2024

It seems unlikely but not impossible that you would use Cookies with an API

For what it's worth, I have seen that :)

@tghosth
Copy link
Collaborator

tghosth commented Dec 2, 2024

It is a wider topic than API. For example, redirects using OAuth or some SSO and containing secrets or sensitive information.

How would you expand the proposal then?

@elarlang
Copy link
Collaborator

elarlang commented Dec 4, 2024

It is a wider topic than API. For example, redirects using OAuth or some SSO and containing secrets or sensitive information.

How would you expand the proposal then?

Got stuck with providing alternative proposal. We can say, that the service getting the request from SSO or whatever other request is an API and the proposed request is valid.

The requirement is also negative requirement, but when I tried to make it positive, it's hard to keep the main problem in the focus, say probably we can leave it like that.

@tghosth
Copy link
Collaborator

tghosth commented Dec 5, 2024

Any better?

13.1.8 - Verify that API endpoints, including those used in SSO processes, will only respond to HTTPS requests and that an HTTP endpoint is not available, even for redirection. This is to avoid API clients accidentally sending data over plaintext HTTP but this not being discovered due to an automatic redirect.

@randomstuff
Copy link
Contributor

Is is clear enough that we are talking about HTTP-based APIs (in contrast to some other RPC mechanism)?

Would we need something like:

13.1.8 - Verify that HTTPS API endpoints, including those used in SSO processes, will only respond to HTTPS requests and that an HTTP endpoint is not available, even for redirection. This is to avoid API clients accidentally sending data over plaintext HTTP but this not being discovered due to an automatic redirect.

or

13.1.8 - Verify that HTTP-based API endpoints, including those used in SSO processes, will only respond to HTTPS requests and that an HTTP endpoint is not available when used over the network, even for redirection. This is to avoid API clients accidentally sending data over plaintext HTTP but this not being discovered due to an automatic redirect.

Is it clear that the "always use HTTPS" requirements do not apply to HTTP-over-IPC (such as HTTP over Unix Domain Socket)?

Is it clear that "HTTP endpoint" is about the actual virtual + port + path (+ HTTP method)? i.e. if my API is https://www.example.com/, it is OK if:

  • the server is listening on http://www.example.com/;
  • the server is redirecting for example http://www.example.com/ to https://www.example.com/;
  • the server must not redirect http://www.example.com/api/ to https://www.example.com/;
  • but could instead answer with some 404.

@tghosth
Copy link
Collaborator

tghosth commented Dec 5, 2024

Ok so how about:

13.1.8 - Verify that HTTPS-based API endpoints, including those used in SSO processes, will only successfully respond to HTTPS requests. The endpoint should respond to plaintext HTTP requests with either an error or no response, and never with a redirect to the HTTPS endpoint. This is to avoid clients accidentally sending data over plaintext HTTP, but this not being discovered due to an automatic redirect.

or even

13.1.8 - Verify that HTTPS-based endpoints will only successfully respond to HTTPS requests. The endpoint should respond to plaintext HTTP requests with either an error or no response, and never with a redirect to the HTTPS endpoint. This is to avoid clients accidentally sending data over plaintext HTTP, but this not being discovered due to an automatic redirect.

@tghosth
Copy link
Collaborator

tghosth commented Dec 5, 2024

Opened #2432 based on discussion with Elar

@tghosth tghosth closed this as completed in 66da30f Dec 5, 2024
@jmanico
Copy link
Member

jmanico commented Dec 6, 2024

I know this is merged but this text is a little awkward.

non-encrypted HTTP

This is redundant.

requests with an error

This alone can be problematic. The better call it to not respond at all:

So I suggest we just keep this simple, with text like.

Verify that HTTPS-based endpoints do not respond to HTTP requests.

This drops the negative requirement of not redirecting, drops the "error message" part, and simplifies the language.

@Sjord
Copy link
Contributor Author

Sjord commented Dec 10, 2024

Verify that services do not transparently redirect HTTP requests to HTTPS unless the URL is specifically designed to be accessed manually by end-users via a web browser.

@tghosth
Copy link
Collaborator

tghosth commented Dec 10, 2024

Could you reword it to be worded as a positive requirement and also include the text that explains why?

@tghosth tghosth added 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos _5.0 - Not blocker This issue does not block 5.0 so if it gets addressed then great, if not then fine. and removed _5.0 - prep This needs to be addressed to prepare 5.0 labels Dec 11, 2024
@jmanico
Copy link
Member

jmanico commented Jan 1, 2025

Something along these lines?

Verify that only user-facing endpoints (intended for manual web-browser access) automatically redirect from HTTP to HTTPS, while other services or endpoints do not implement transparent redirects.

@tghosth
Copy link
Collaborator

tghosth commented Jan 2, 2025

@elarlang what do you think about this proposal?

@elarlang
Copy link
Collaborator

elarlang commented Jan 2, 2025

The proposed part is ok, but I think the requirement needs an explanatory part to be clear about why it exists or what problem it addresses.

@tghosth
Copy link
Collaborator

tghosth commented Jan 2, 2025

So how about this:

"Verify that only user-facing endpoints (intended for manual web-browser access) automatically redirect from HTTP to HTTPS, while other services or endpoints do not implement transparent redirects. If a client is erroneously sending unencrypted HTTP requests but the requests are being automatically redirected to HTTPS, this leakage of sensitive data may go undiscovered."

@elarlang
Copy link
Collaborator

Can we change the second part to something like:

This is to avoid the potential leakage of sensitive data may go undiscovered in case a client is erroneously sending unencrypted HTTP requests but the requests are being automatically redirected to HTTPS.

If the additional sentence starts with an "if" it feels like a separate condition in the requirement, not a start of the explanation part.

@elarlang elarlang added 4) proposal for review Issue contains clear proposal for add/change something and removed 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos labels Jan 13, 2025
@tghosth
Copy link
Collaborator

tghosth commented Jan 16, 2025

Verify that only user-facing endpoints (intended for manual web-browser access) automatically redirect from HTTP to HTTPS, while other services or endpoints do not implement transparent redirects. This is to avoid a situation where a client is erroneously sending unencrypted HTTP requests but, since the requests are being automatically redirected to HTTPS, the leakage of sensitive data goes undiscovered.

@elarlang what do you think?

@tghosth tghosth added _5.0 - prep This needs to be addressed to prepare 5.0 and removed 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet _5.0 - Not blocker This issue does not block 5.0 so if it gets addressed then great, if not then fine. labels Jan 16, 2025
@elarlang elarlang added 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR and removed 4) proposal for review Issue contains clear proposal for add/change something labels Jan 16, 2025
@elarlang
Copy link
Collaborator

I'm ok with this.

Ping @Sjord

tghosth added a commit that referenced this issue Jan 16, 2025
@tghosth tghosth linked a pull request Jan 16, 2025 that will close this issue
@tghosth
Copy link
Collaborator

tghosth commented Jan 16, 2025

Drafted #2529 to resolve.

@set-reminder @tghosth to mark as ready for review in 1 week if we don't get further input

@tghosth tghosth added 6) PR awaiting review and removed 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR labels Jan 16, 2025
@elarlang
Copy link
Collaborator

I think it makes sense to merge it in. If there are something to improve, we can do another commit/PR, but I don't like those hanging PR's to collect conflicts to solve.

jmanico pushed a commit that referenced this issue Jan 16, 2025
Co-authored-by: Elar Lang <47597707+elarlang@users.noreply.github.com>
@OWASP OWASP deleted a comment from octo-reminder bot Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6) PR awaiting review V13 _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants