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

OOD reverse proxy is modifying HTTP Location header in-flight #695

Closed
sync-by-unito bot opened this issue Dec 27, 2021 · 4 comments
Closed

OOD reverse proxy is modifying HTTP Location header in-flight #695

sync-by-unito bot opened this issue Dec 27, 2021 · 4 comments

Comments

@sync-by-unito
Copy link

sync-by-unito bot commented Dec 27, 2021

Summary description:

The current design of the rnode proxy assumes that all 3xx redirects specified in a response from the server itself will be to other relative paths on that server, not to a different server. Thus the current design prevents you from doing two things:

  1. starting two servers on the same host, different ports, and in a http response from one server redirecting one to the other server
  2. redirecting a user to another server running on another host

These are edge cases, but I'm unaware of any at this point that we don't have a work around for.

A modification to the Apache config to avoid Header edit Location when we know it is a redirect and the location header specified is a different server would be the solution.

Original description:

Route /rnode/<host>/<port>/ is performing undesirable behavior. This route does not allow the origin server to set Location HTTP headers.

To reproduce this, run this Flask application:

#!/usr/bin/env python

import os, hashlib
from flask import Flask, url_for, redirect, request, make_response, render_template

app = Flask(__name__)

@app.route('/test', methods=['GET'])
def test():
    return 'Test'

@app.route('/', methods=['GET'])
def index():
    response = make_response('Test')
    response.headers['Location'] = 'http://example.com'
    return response

if __name__ == '__main__':
    port = int(os.environ.get('proxy_port', 5000))
    app.run(host='0.0.0.0', port=port, debug=True)

Accessing the above test application with the /rnode reverse proxy:
https://ondemand-test.osc.edu/webtest01.hpc.osc.edu/5000/

The response HTTP headers are:

Cache-Control: max-age=0, no-store
Connection: Keep-Alive
Content-Length: 4
Content-Type: text/html; charset=utf-8
Date: Thu, 07 May 2020 18:38:13 GMT
Keep-Alive: timeout=15, max=60
Location: /rnode/webtest01.hpc.osc.edu/5000
Server: Werkzeug/0.14.1 Python/3.6.6
Set-Cookie: mod_auth_openidc_session=<removed>; Path=/; Secure; HttpOnly; SameSite=None
Via: 1.1 ondemand-test.osc.edu
X-DNS-Prefetch-Control: off

If we overwrite the Server header:

@app.route('/', methods=['GET'])
def index():
    response = make_response('Test')
    response.headers['Location'] = 'http://example.com'
    response.headers['Server'] = 'Testing'
    return response

We will see it in the response:

Location: /rnode/webtest01.hpc.osc.edu/5000
Server: Testing

Location isn't being overwritten. If we do a redirect like this:

@app.route('/', methods=['GET'])
def index():
    response = make_response(redirect('https://ondemand-test.osc.edu/rnode/webtest01.hpc.osc.edu/5000/test/'))
    return response

The HTTP response headers look like this:

Location: /rnode/webtest01.hpc.osc.edu/5000/rnode/webtest01.hpc.osc.edu/5000/test/

Something is modifying the Location header in-flight and not respecting what the origin server sends as Location

This issue is relevant for OSC/bc_osc_codeserver#2 (comment)

┆Issue is synchronized with this Asana task by Unito

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Dec 27, 2021

➤ Eric Franz commented:

When you set response.headers['Location'] don't you also need to set the status code to 302 (or 301)?

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Dec 27, 2021

➤ Mario Squeo commented:

ericfranz

I just talked with johrstrom and he sent me this: https://github.com/OSC/ondemand/blob/master/ood-portal-generator/templates/ood-portal.conf.erb#L180

That would explain why we can't overwrite Location headers.

If we set the response code to 302, the same behavior happens:

@app.route('/', methods=['GET'])
def index():
response = make_response('Test', 302)
response.headers['Location'] = 'http://example.com'
return responseStatus Code: 302 FOUND
Location: /rnode/webtest01.hpc.osc.edu/5000

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Dec 27, 2021

➤ Eric Franz commented:

The related line of code as shared by johrstrom & msquee https://github.com/OSC/ondemand/blob/cc88e23b2221296871764deabb47e069d6b8454a/ood-portal-generator/templates/ood-portal.conf.erb#L180

@sync-by-unito
Copy link
Author

sync-by-unito bot commented Dec 27, 2021

➤ Eric Franz commented:

The current design of the rnode proxy assumes that all 3xx redirects specified in a response from the server itself will be to other relative paths on that server, not to a different server. Thus the current design prevents you from doing two things:

  1. starting two servers on the same host, different port, and in a http response from one server redirecting one to the other server
  2. redirecting a user to another server running on another host

These are edge cases, but I'm unaware of any at this point that we don't have a work around for.

A modification to the Apache config to avoid Header edit Location when we know it is a redirect and the location header specified is a different server would be the solution.

@ghost ghost closed this as completed Dec 30, 2021
This issue was closed.
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

No branches or pull requests

0 participants