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 #493

Closed
msquee opened this issue May 7, 2020 · 5 comments
Closed

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

msquee opened this issue May 7, 2020 · 5 comments
Assignees
Milestone

Comments

@msquee
Copy link
Contributor

msquee commented May 7, 2020

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

@github-actions github-actions bot added this to the Needs Triaged milestone May 7, 2020
@ericfranz
Copy link
Contributor

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

@msquee
Copy link
Contributor Author

msquee commented May 7, 2020

@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 response
Status Code: 302 FOUND
Location: /rnode/webtest01.hpc.osc.edu/5000

@ericfranz
Copy link
Contributor

The related line of code as shared by @johrstrom & @msquee

Header edit Location "^([^/]+//[^/]+)|(?=/)" "<%= @rnode_uri %>/%{MATCH_HOST}e/%{MATCH_PORT}e"

@ericfranz
Copy link
Contributor

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.

@johrstrom
Copy link
Contributor

  • starting two servers on the same host, different port, and in a http response from one server redirecting one to the other server

  • redirecting a user to another server running on another host

If we have clear use cases for these we'll get on them, but as we don't have use cases for them I'm just going to close the issue as won't do. As for the initial comment in the ticket, this is expected behavior and will be until it isn't.

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

3 participants