Claudio Jeker writes:
> On Wed, Apr 28, 2021 at 09:55:16AM -0400, Dave Voutila wrote:
>>
>> Claudio Jeker writes:
>>
>> > On Wed, Apr 28, 2021 at 08:18:47AM -0400, Dave Voutila wrote:
>> >>
>> >> Claudio Jeker writes:
>> >>
>> >> > Another thing to consider is that X-Forwarded headers should only be
>> >> > accepted from trusted sources. I don't think this particular usage of
>> >> > X-Forwarded-Proto is probelmatic. In the end for this particular case of
>> >> > redirect using a relative URL seems to be a better choice since it will
>> >> > preserve the host, port and proto from the original requests without any
>> >> > configuration magic.
>> >>
>> >> Here's a quick effort to test out the idea. Not sure I like it yet...but
>> >> figured this could help move discussion forward. (Not looking for an OK
>> >> yet as I doubt this config style is good yet.)
>> >>
>> >> Couple points:
>> >> * the srvflag bitmap is sort of at the semantic breaking point as a
>> >> uint32_t...hesitant to bump it to uint64_t just to allow better flag
>> >> groupings but that's a thought
>> >>
>> >> * not sure i like my config style...maybe (yet another) optional token
>> >> or two in the "auto index" pattern instead?
>> >>
>> >> Quick example config folks can test with. First `# cp -R /var/www /tmp`
>> >> and then stage some files there for your enjoyment.
>> >>
>> >> chroot "/tmp/www"
>> >> server "default" {
>> >> listen on * port 80
>> >> directory {
>> >> auto index
>> >> relative redirects
>> >> }
>> >> }
>> >>
>> >> Example without "relative redirects":
>> >>
>> >> ~ $ curl -i localhost/bgplg
>> >> HTTP/1.0 301 Moved Permanently
>> >> Server: OpenBSD httpd
>> >> Connection: close
>> >> Content-Type: text/html
>> >> Content-Length: 510
>> >> Location: http://localhost/bgplg/
>> >>
>> >> With "relative redirects":
>> >>
>> >> ~ $ curl -i localhost/bgplg
>> >> HTTP/1.0 301 Moved Permanently
>> >> Server: OpenBSD httpd
>> >> Connection: close
>> >> Content-Type: text/html
>> >> Content-Length: 510
>> >> Location: /bgplg/
>> >>
>> >
>> > To be honest I feel this is total overkill. This is an automatic redirect
>> > from /foo/bar to /foo/bar/ when accessing directories that have an index
>> > file or have auto index on. It is one very specific case of redirect that
>> > never changes the host, port or protocol. I see no reason why this can't
>> > default to a relative redirect (but using an absolute path).
>> >
>>
>> My first concern is the behavior of user-agent caching. Since the
>> request was a full url, but the redirect is relative, it's not clear if
>> there might be issues or not.
>
> Unless the client is broken it should not matter I think.
>
>> > Are there clients in the wild that fail to handle such a redirect?
>>
>> I'm doing some research, but I found this interesting commit [1] from long
>> ago (2006) which allude to this caching issue I'm describing above.
>>
>> Moreover, that commit had me look into RFC 2616 (the HTTP/1.1 spec),
>> which states we technically violate spec if we move to "always relative"
>> redirects:
>>
>> 14.30 Location
>>
>> The Location response-header field is used to redirect the recipient
>> to a location other than the Request-URI for completion of the
>> request or identification of a new resource. For 201 (Created)
>> responses, the Location is that of the new resource which was created
>> by the request. For 3xx responses, the location SHOULD indicate the
>> server's preferred URI for automatic redirection to the resource. The
>> field value consists of a single absolute URI.
>>
>> Location = "Location" ":" absoluteURI
>>
>> In practice, it seems user-agents handle relative Location values...so
>> the question is do we want to follow the RFC by default or a "de facto"
>> behavior? (Not sure how other decisions have been made in httpd(8)
>> previously.)
>
> But then there is RFC7231 (update of 2616) that defines Location a bit
> different.
>
> 7.1.2. Location
>
> The "Location" header field is used in some responses to refer to a
> specific resource in relation to the response. The type of
> relationship is defined by the combination of request method and
> status code semantics.
>
> Location = URI-reference
>
> The field value consists of a single URI-reference. When it has the
> form of a relative reference ([RFC3986], Section 4.2), the final
> value is computed by resolving it against the effective request URI
> ([RFC3986], Section 5).
>
> ...
>
> With RFC3986 defining the various relative URI. As usual with anything web
> related there are a lot of cargo cults and this feels like one of those
> cases.
Ha, I should have known it was superseded! In that case, I like the
default relative redirect idea. It's as simple as the below diff.
Any objections?
Index: server_file.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
retrieving revision 1.69
diff -u -p -r1.69 server_file.c
--- server_file.c 16 Mar 2021 06:44:14 -0000 1.69
+++ server_file.c 28 Apr 2021 14:25:06 -0000
@@ -85,9 +85,7 @@ server_file_access(struct httpd *env, st
if (path[strlen(path) - 1] != '/') {
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/", encodedpath) == -1) {
free(encodedpath);
return (500);
}
