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

Confirm if Akita payment pointer resolution is spec compliant #60

Closed
sharon-wang opened this issue Jun 14, 2020 · 9 comments · Fixed by #87
Closed

Confirm if Akita payment pointer resolution is spec compliant #60

sharon-wang opened this issue Jun 14, 2020 · 9 comments · Fixed by #87
Assignees
Labels
bug Something isn't working priority 2 Secondary priority

Comments

@sharon-wang
Copy link
Member

As per the payment pointer spec https://paymentpointers.org/syntax-resolution/#requirements:

Clients MUST support HTTP redirects as a result of 3XX responses per RFC 7231.

If a website is using a vanity payment pointer (payment pointer redirects to another payment pointer), then a request to the payment pointer will respond with a redirect, i.e. 3XX response. Right now, we only check that a 200 OK is returned to validate that the payment pointer is working. We need to add handling to accept the 3XX response, send a new request with the redirect endpoint, and confirm that a 200 OK is returned.

Relevant links

@sharon-wang sharon-wang added bug Something isn't working next steps labels Jun 14, 2020
@sharon-wang
Copy link
Member Author

Our current implementation actually does support 3XX responses, since the default behaviour of the Fetch API is to follow redirects (search 'redirect' here). However, we can't be sure that the redirected URL is actually a valid payment pointer endpoint, since our current implementation would just expect to receive a 200 OK. That means that if the redirected URL is just some accessible domain, then we'll receive a 200 OK, even though it's not actually an SPSP Endpoint.

If we could extract the redirect URL and validate that it matches the payment pointer format before following the URL, that would be better. After doing some reading, it seems that the Fetch API does not expose the redirection Location in the response header for clients (a measure to avoid leaking info in XSS attacks) - source 1, source 2.

So, it seems we can't use a manual redirect to check the endpoint before initiating a new request. It might be possible to expose Location by adding a header Access-Control-Expose-Headers: Location to the request - I have not tried this yet. Adding this header seems like a way to bypass the CORS (Cross-origin resource sharing, which is in effect for browser extension code) standard headers restriction, allowing access to Location. I plan to set up a vanity pointer and redirect to try this out.

Additionally, we should be verifying that the response body contains destination_account (ILP Address) and shared_secret (32 bytes, base64 (base64url) encoded (including padding)) as per the spec. This will get us closer to ensuring that the payment pointer is valid, but again, you could easily implement something that just returns the info we're looking for here.

Since we're doing all the payment pointer validation client-side, there are some challenges with guaranteeing that the provided payment pointer is valid. I'm not sure there's a way for us to properly validate the payment pointer client-side, but we can do all these checks to try to get pretty close.

@sharon-wang
Copy link
Member Author

@vezwork ping for your thoughts 🙂

@sharon-wang sharon-wang added the priority 2 Secondary priority label Oct 18, 2020
@sharon-wang sharon-wang self-assigned this Oct 18, 2020
@vezwork
Copy link
Member

vezwork commented Nov 15, 2020

I think I understand the problem. The Fetch API does not give the location on a 3XX response, so we can't know if the redirect goes to a secure HTTPS endpoint. BUT by the payment pointer spec says that "Clients MUST support HTTP redirects as a result of 3XX responses per RFC 7231 however these MUST always redirect to a secure (https) URL."

Is there a way to check if a redirect is HTTPS without knowing the redirect location?

@vezwork
Copy link
Member

vezwork commented Nov 15, 2020

It is possible to obtain the FINAL url that resolves after redirects using https://developer.mozilla.org/en-US/docs/Web/API/Response/url, but if there is more than one redirect in the redirect chain, then we won't know if some of the redirects were over HTTP and not HTTPS. The payment pointer spec is slightly ambiguous, it says that "Payment Pointers MUST resolve to an https URL as defined in RFC7230."

Is there a way to use fetch where we don't allow redirecting to non-HTTPS endpoints? CORS may do this by default...

@sharon-wang
Copy link
Member Author

sharon-wang commented Nov 15, 2020

Some confirmation of our assumption that every redirect url in the chain must be HTTPS...but not entirely sure if the originating URL must be HTTPS as well?

the whole redirect-chain must use HTTPS

Source: https://forum.interledger.org/t/redirected-payment-pointers/809/3

@vezwork
Copy link
Member

vezwork commented Nov 15, 2020

This stackoverflow answer seems to indicate that CORS causes an error on request from HTTPS origin to HTTP origin: https://stackoverflow.com/a/37068063/5425899

@sharon-wang
Copy link
Member Author

sharon-wang commented Nov 15, 2020

The originating URL must also be HTTPS, because we expect the url to start with either $ or https:// to begin with. Then, under the assumption that CORS does not allow redirections from HTTPS to HTTP, we will not have to worry about redirecting to HTTP at any point in the redirection chain. Upon resolving to the final url, we will need to validate the response body for the expected fields.


As per Mixed Content documentation, content served via HTTP on an HTTPS site is blocked by browsers

Since Firefox 55, the loading of mixed content is allowed on http://127.0.0.1/ (see bug 903966). Chrome allows mixed content on http://127.0.0.1/ and http://localhost/. Safari does not allow any mixed content.

So, it seems that if any intermediate redirections involve an HTTP url, the fetch request will be blocked by the browser. We will need to handle the Blocked Loading of Mixed Content error.


TODOs

  • handle blocked mixed content error in fetch request

@sharon-wang
Copy link
Member Author

Not a priority for this milestone, but it would be good to validate the response body for the expected fields defined in the spec. Opened issue #83 to note this.

@sharon-wang sharon-wang changed the title Akita payment pointer resolution is not spec compliant Confirm if Akita payment pointer resolution is spec compliant Nov 15, 2020
@sharon-wang
Copy link
Member Author

sharon-wang commented Nov 18, 2020

According to the W3 spec on blocking Fetch when mixed content is encountered:

Return blocked if one or more of the following conditions are met:

  1. user agent is configured to block optionally-blockable mixed content, as described in §7.4 User Controls.
  2. request’s client’s strict mixed content checking flag is true.
  3. request’s mode is CORS or CORS-with-forced-preflight.

Although the default mode for fetch() is cors, it would be better to explicitly specify mode: 'cors' in our fetch request.


It seems we may not be able to catch when mixed content occurs, since it's just a warning that's displayed in the console.

From Mozilla mixed content docs:

As well as finding these warnings in the Web Console, you could use Content Security Policy (CSP) to report issues. You could also use an online crawler like SSL-check or Missing Padlock that will check your website recursively and find links to insecure content.

Starting in Firefox 23, mixed active content is blocked by default (and mixed display content can be blocked by setting a preference). To make it easier for web developers to find mixed content errors, all blocked mixed content requests are logged to the Security pane of the Web Console...

Currently, we consider a payment pointer invalid if the fetch response does not return 200 OK. Aside from checking the response body contents proposed in #83, we may not be able to do much more validation/handling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority 2 Secondary priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants