ietf-wg-gnap / gnap-core-protocol

141 stars 26 forks source link

Are relative URLs allowed? #439

Closed adrianhopebailie closed 1 year ago

adrianhopebailie commented 1 year ago

While it doesn't say so in the spec and the example suggest otherwise, are relative URLs allowed for properties such as the continue response.

Scenario, an implementation is deployed at a URL: server.example.com The root of the server handles grant requests: POST / to https://server.example.com

The application code that builds the continue response must be able to construct the full URL and therefor have knowledge of the host at which the application is deployed including any path prefix.

This is not impossible, but in complex cloud deployments with load balancers and ingress services etc between the application code and the public endpoint this can get ugly.

The current example from Section 3.1 of the spec.

{
    "continue": {
        "access_token": {
            "value": "80UPRY5NM33OMUKMKSKU"
        },
        "uri": "https://server.example.com/continue",
        "wait": 60
    }
}

Implementations would be significantly easier to build if they could simply return:

{
    "continue": {
        "access_token": {
            "value": "80UPRY5NM33OMUKMKSKU"
        },
        "uri": "./continue",
        "wait": 60
    }
}
adrianhopebailie commented 1 year ago

To be clear, the base URL for any relative URL response would be the request URL.

jricher commented 1 year ago

Things get really tricky if you start to allow for things like directory ascension (../../continue) and other such things. Lots of attacks in the past stem from exploitation of devices like this.

Application deployments that deal with external urls already have techniques for managing such urls, from proxy rewrites to internal configuration.

With these together I don't see a good argument for this being anything but an absolute uri.

adrianhopebailie commented 1 year ago

Is it necessary to state somewhere that all URIs are absolute?

jricher commented 1 year ago

Yes, we should say exactly that, that's why I am keeping this issue open. Thanks for pointing this out!