Hi all,
Consider the following situation. A reverse proxy which performs TLS
termination is deployed in front of httpd, which listens unencrypted on
localhost.
There is code in httpd to handle the case where a directory is accessed,
but the path named does not end with a slash. In this case, httpd
issues a 301 redirect to the path with a slash appended.
The logic here sets the protocol of the redirect path based on
whether the httpd virtual server is configured with TLS, but this isn't
enough because it will cause redirects to plain http when there is a
reverse proxy in front of httpd that performs TLS termination.
This will either cause an extra redirect round trip to get back to HTTPS,
or break the site if it's not publicly served on plain HTTP.
Instead, we should be reading X-Forwarded-Proto, which most reverse proxies
add to inform the backing server what the original protocol was.
(relayd doesn't expose this to my knowledge, but I will look into doing so)
The below attached diff does this for httpd. This is my first diff to the
project, so please give feedback on whether I've done everything right.
Thanks!
PS: the X-Forwarded-* headers seem to now have a standardized form
called "Forwarded", but I opted for the X- version since the rest
of httpd only supports the X- headers, to keep consistency.
Index: server_file.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
retrieving revision 1.69
diff -u -p -u -p -r1.69 server_file.c
--- server_file.c 16 Mar 2021 06:44:14 -0000 1.69
+++ server_file.c 27 Apr 2021 15:25:51 -0000
@@ -83,11 +83,28 @@ server_file_access(struct httpd *env, st
/* Redirect to path with trailing "/" */
if (path[strlen(path) - 1] != '/') {
+ const char *proto;
+
+ proto = srv_conf->flags & SRVFLAG_TLS
+ ? "https" : "http";
+
+ /* If request specifies a X-Forwarded-Proto header,
+ * respect that. */
+ key.kv_key = "X-Forwarded-Proto";
+ if ((r = kv_find(&desc->http_headers, &key)) != NULL &&
+ r->kv_value != NULL) {
+ if (strcmp("https", r->kv_value) == 0)
+ proto = "https";
+ else if (strcmp("http", r->kv_value) == 0)
+ proto = "http";
+ else
+ return (500);
+ }
+
if ((encodedpath = url_encode(desc->http_path)) == NULL)
return (500);
- if (asprintf(&newpath, "http%s://%s%s/",
- srv_conf->flags & SRVFLAG_TLS ? "s" : "",
- desc->http_host, encodedpath) == -1) {
+ if (asprintf(&newpath, "%s://%s%s/",
+ proto, desc->http_host, encodedpath) == -1) {
free(encodedpath);
return (500);
}