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).
Are there clients in the wild that fail to handle such a redirect?
> Index: usr.sbin/httpd/httpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> retrieving revision 1.156
> diff -u -p -r1.156 httpd.h
> --- usr.sbin/httpd/httpd.h 20 Apr 2021 21:11:56 -0000 1.156
> +++ usr.sbin/httpd/httpd.h 28 Apr 2021 12:10:18 -0000
> @@ -392,6 +392,7 @@ SPLAY_HEAD(client_tree, client);
> #define SRVFLAG_DEFAULT_TYPE 0x00800000
> #define SRVFLAG_PATH_REWRITE 0x01000000
> #define SRVFLAG_NO_PATH_REWRITE 0x02000000
> +#define SRVFLAG_RELATIVE_REDIR 0x04000000
> #define SRVFLAG_LOCATION_FOUND 0x40000000
> #define SRVFLAG_LOCATION_NOT_FOUND 0x80000000
>
> @@ -401,7 +402,7 @@ SPLAY_HEAD(client_tree, client);
> "\14SYSLOG\15NO_SYSLOG\16TLS\17ACCESS_LOG\20ERROR_LOG" \
> "\21AUTH\22NO_AUTH\23BLOCK\24NO_BLOCK\25LOCATION_MATCH" \
> "\26SERVER_MATCH\27SERVER_HSTS\30DEFAULT_TYPE\31PATH\32NO_PATH" \
> - "\37LOCATION_FOUND\40LOCATION_NOT_FOUND"
> + "\33RELATIVE_REDIR\37LOCATION_FOUND\40LOCATION_NOT_FOUND"
>
> #define TCPFLAG_NODELAY 0x01
> #define TCPFLAG_NNODELAY 0x02
> Index: usr.sbin/httpd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.125
> diff -u -p -r1.125 parse.y
> --- usr.sbin/httpd/parse.y 10 Apr 2021 10:10:07 -0000 1.125
> +++ usr.sbin/httpd/parse.y 28 Apr 2021 12:10:18 -0000
> @@ -137,9 +137,10 @@ typedef struct {
> %token ACCESS ALIAS AUTO BACKLOG BODY BUFFER CERTIFICATE CHROOT
> CIPHERS COMMON
> %token COMBINED CONNECTION DHE DIRECTORY ECDHE ERR FCGI INDEX IP KEY
> LIFETIME
> %token LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON
> PORT PREFORK
> -%token PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG
> TCP TICKET
> -%token TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD
> REQUEST
> -%token ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
> +%token PROTOCOLS REDIRECTS RELATIVE REQUESTS ROOT SACK SERVER SOCKET
> STRIP
> +%token STYLE SYSLOG TCP TICKET TIMEOUT TLS TYPE TYPES HSTS MAXAGE
> SUBDOMAINS
> +%token DEFAULT PRELOAD REQUEST ERROR INCLUDE AUTHENTICATE WITH BLOCK
> DROP
> +%token RETURN PASS REWRITE
> %token CA CLIENT CRL OPTIONAL PARAM FORWARDED FOUND NOT
> %token <v.string> STRING
> %token <v.number> NUMBER
> @@ -1038,7 +1039,11 @@ dirflags : INDEX STRING {
> srv_conf->flags &= ~SRVFLAG_AUTO_INDEX;
> srv_conf->flags |= SRVFLAG_NO_AUTO_INDEX;
> }
> - ;
> + | RELATIVE REDIRECTS {
> + srv_conf->flags &= ~SRVFLAG_RELATIVE_REDIR;
> + srv_conf->flags |= SRVFLAG_RELATIVE_REDIR;
> + }
> + ;
>
>
> logformat : LOG logflags
> @@ -1425,6 +1430,8 @@ lookup(char *s)
> { "prefork", PREFORK },
> { "preload", PRELOAD },
> { "protocols", PROTOCOLS },
> + { "redirects", REDIRECTS },
> + { "relative", RELATIVE },
> { "request", REQUEST },
> { "requests", REQUESTS },
> { "return", RETURN },
> Index: usr.sbin/httpd/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
> --- usr.sbin/httpd/server_file.c 16 Mar 2021 06:44:14 -0000 1.69
> +++ usr.sbin/httpd/server_file.c 28 Apr 2021 12:10:18 -0000
> @@ -85,13 +85,17 @@ 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) {
> - free(encodedpath);
> - return (500);
> - }
> +
> + if (srv_conf->flags & SRVFLAG_RELATIVE_REDIR)
> + ret = asprintf(&newpath, "%s/", encodedpath);
> + else
> + ret = asprintf(&newpath, "http%s://%s%s/",
> + srv_conf->flags & SRVFLAG_TLS ? "s" : "",
> + desc->http_host, encodedpath);
> +
> free(encodedpath);
> + if (ret < 0)
> + return (500);
>
> /* Path alias will be used for the redirection */
> desc->http_path_alias = newpath;
>
--
:wq Claudio
