Re: Byte range implementation for httpd(8)

2015-05-03 Thread Ian Mcwilliam
This might be what your thinking of.

https://httpd.apache.org/security/CVE-2011-3192.txt

Description:


A denial of service vulnerability has been found in the way the multiple
overlapping ranges are handled by the Apache HTTPD server prior to version
2.2.20:

 http://seclists.org/fulldisclosure/2011/Aug/175

An attack tool is circulating in the wild. Active use of this tool has
been observed.

The attack can be done remotely and with a modest number of requests can
cause very significant memory and CPU usage on the server.



Ian McWilliam


From: owner-t...@openbsd.org [owner-t...@openbsd.org] on behalf of Florian 
Obser [flor...@openbsd.org]
Sent: Monday, 4 May 2015 4:34 AM
To: tech@openbsd.org
Cc: Sunil Nimmagadda
Subject: Re: Byte range implementation for httpd(8)

On Sun, May 03, 2015 at 08:14:25PM +0200, Sebastian Benoit wrote:
> one question though: whats the reasoning behind MAX_RANGES 4? nginx seems to
> have a default of "unlimited" (which i think questionable), but what is

Wasn't there a cve about this last year or so? You can try to burn cpu
and io on the server by requesting stupid ranges, like one byte at a
time, backwards for the whole file or something...

> reasonably seen on the internet?

my best guess is one range, from some byte position to the end, when
you resume a transfer.

--
I'm not entirely sure you are real.



Re: Byte range implementation for httpd(8)

2015-05-03 Thread Florian Obser
On Sun, May 03, 2015 at 08:14:25PM +0200, Sebastian Benoit wrote:
> one question though: whats the reasoning behind MAX_RANGES 4? nginx seems to
> have a default of "unlimited" (which i think questionable), but what is

Wasn't there a cve about this last year or so? You can try to burn cpu
and io on the server by requesting stupid ranges, like one byte at a
time, backwards for the whole file or something... 

> reasonably seen on the internet?

my best guess is one range, from some byte position to the end, when
you resume a transfer.

-- 
I'm not entirely sure you are real.



Re: Byte range implementation for httpd(8)

2015-05-03 Thread Sebastian Benoit
Florian Obser(flor...@openbsd.org) on 2015.05.03 12:39:02 +:
> On Sun, May 03, 2015 at 01:46:56PM +0200, Sunil Nimmagadda wrote:
> > On Sat, May 02, 2015 at 02:49:30PM +, Florian Obser wrote:
> > > Sorry for the very late reply, I'm currently very busy :/
> > 
> > Thank you for taking time to review it. A new patch with style nits
> > fixed and a gratuitous NULL check removed.
> > 
> > [trimming some text]
> >  
> > > this is missing the server_file_method() song and dance from
> > > server_file_request()
> > 
> > Fixed in a slightly different way than using server_file_method().
> > Since range request should be ignored for methods other than GET,
> > fallback to server_file_request() for further processing/validation.
> 
> yep, this is good.
> 
> > 
> > > > +   if ((range = parse_range(range_str, st->st_size, &nranges)) == 
> > > > NULL) {
> > > > +   code = 416;
> > > > +   (void)snprintf(content_range, sizeof(content_range),
> > > > +   "bytes */%lld", st->st_size);
> > > > +   errstr = content_range;
> > > > +   goto abort;
> > > > +   }
> > > 
> > > hm, apache answers with the full file and 200 if the range header is
> > > syntactically incorrect or if end < start. As far as I can tell it
> > > only answers 416 if the range actually lies outside of the file.
> > 
> > I am confused about the RFC here, section 4.4 states that 416 is
> > returned if "...the set of ranges requested has been rejected due
> > to invalid ranges..." and a note follows immediately, "Because
> > servers are free to ignore Range, many implementations will simply
> > respond with the entire selected representation in a 200 response"
> > 
> > As you noted apache returns 200 while nginx returns 416 for an
> > incorrect range. What should httpd do ideally?
> 
> Thanks for checking nginx, I don't have one around.
> I noted this for 2 reasons:
> 1) I guess apache is the defacto standard, so if it behaves
> differently there might be a reason, maybe broken clients in the wild.
> 2) The way the code is currently structured it will require a bit of
> shuffling around if we ever want apache's behaviour.
> 
> That being said, my understanding of the RFC is that your
> implementation is correct. And with nginx behaving this way, too, it
> shouldn't bite us in the future. :)
> 
> > 
> > > > +
> > > > +   /* Accept byte ranges */
> > > > +   if (code == 200 &&
> > > > +   kv_add(&resp->http_headers, "Accept-Ranges", "bytes") == 
> > > > NULL)
> > > > return (-1);
> > > 
> > > I don't think we should advertise ranges for all 200 results, for
> > > example we don't support it for directory indexes.
> > 
> > Agree, since RFC says "server MAY send Accept-Range header" and
> > "clients MAY generate range requests without having received this
> > header field", dropping this header shouldn't make a difference.
> > 
> > Comments?
> 
> This looks good.
> 
> OK florian@ if someone wants to commit it. Or give me an OK and I'll
> commit.
>

fwiw i dont know much about ranges, but this reads ok. two whitespace bits
below. otherwise ok benno.

one question though: whats the reasoning behind MAX_RANGES 4? nginx seems to
have a default of "unlimited" (which i think questionable), but what is
reasonably seen on the internet?

> > 
> > Index: server_file.c
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> > retrieving revision 1.52
> > diff -u -p -r1.52 server_file.c
> > --- server_file.c   25 Apr 2015 14:40:35 -  1.52
> > +++ server_file.c   3 May 2015 11:18:07 -
> > @@ -36,12 +36,25 @@
> >  
> >  #define MINIMUM(a, b)  (((a) < (b)) ? (a) : (b))
> >  #define MAXIMUM(a, b)  (((a) > (b)) ? (a) : (b))
> > +#define MAX_RANGES 4
> >  
> > -int server_file_access(struct httpd *, struct client *, char *, 
> > size_t);
> > -int server_file_request(struct httpd *, struct client *, char *,
> > -   struct stat *);
> > -int server_file_index(struct httpd *, struct client *, struct stat 
> > *);
> > -int server_file_method(struct client *);
> > +struct range {
> > +   off_t   start;
> > +   off_t   end;
> > +};
> > +
> > +int server_file_access(struct httpd *, struct client *,
> > +   char *, size_t);
> > +int server_file_request(struct httpd *, struct client *,
> > +   char *, struct stat *);
> > +int server_partial_file_request(struct httpd *, struct 
> > client *,
> > +   char *, struct stat *, char *);
> > +int server_file_index(struct httpd *, struct client *,
> > +   struct stat *);
> > +int server_file_method(struct client *);
> > +int parse_range_spec(char *, size_t, struct range *);
> > +struct range   *parse_range(char *, size_t, int *);
> > +int buffer_add_

Re: Byte range implementation for httpd(8)

2015-05-03 Thread Florian Obser
On Sun, May 03, 2015 at 01:46:56PM +0200, Sunil Nimmagadda wrote:
> On Sat, May 02, 2015 at 02:49:30PM +, Florian Obser wrote:
> > Sorry for the very late reply, I'm currently very busy :/
> 
> Thank you for taking time to review it. A new patch with style nits
> fixed and a gratuitous NULL check removed.
> 
> [trimming some text]
>  
> > this is missing the server_file_method() song and dance from
> > server_file_request()
> 
> Fixed in a slightly different way than using server_file_method().
> Since range request should be ignored for methods other than GET,
> fallback to server_file_request() for further processing/validation.

yep, this is good.

> 
> > > + if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
> > > + code = 416;
> > > + (void)snprintf(content_range, sizeof(content_range),
> > > + "bytes */%lld", st->st_size);
> > > + errstr = content_range;
> > > + goto abort;
> > > + }
> > 
> > hm, apache answers with the full file and 200 if the range header is
> > syntactically incorrect or if end < start. As far as I can tell it
> > only answers 416 if the range actually lies outside of the file.
> 
> I am confused about the RFC here, section 4.4 states that 416 is
> returned if "...the set of ranges requested has been rejected due
> to invalid ranges..." and a note follows immediately, "Because
> servers are free to ignore Range, many implementations will simply
> respond with the entire selected representation in a 200 response"
> 
> As you noted apache returns 200 while nginx returns 416 for an
> incorrect range. What should httpd do ideally?

Thanks for checking nginx, I don't have one around.
I noted this for 2 reasons:
1) I guess apache is the defacto standard, so if it behaves
differently there might be a reason, maybe broken clients in the wild.
2) The way the code is currently structured it will require a bit of
shuffling around if we ever want apache's behaviour.

That being said, my understanding of the RFC is that your
implementation is correct. And with nginx behaving this way, too, it
shouldn't bite us in the future. :)

> 
> > > +
> > > + /* Accept byte ranges */
> > > + if (code == 200 &&
> > > + kv_add(&resp->http_headers, "Accept-Ranges", "bytes") == NULL)
> > >   return (-1);
> > 
> > I don't think we should advertise ranges for all 200 results, for
> > example we don't support it for directory indexes.
> 
> Agree, since RFC says "server MAY send Accept-Range header" and
> "clients MAY generate range requests without having received this
> header field", dropping this header shouldn't make a difference.
> 
> Comments?

This looks good.

OK florian@ if someone wants to commit it. Or give me an OK and I'll
commit.

> 
> Index: server_file.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 server_file.c
> --- server_file.c 25 Apr 2015 14:40:35 -  1.52
> +++ server_file.c 3 May 2015 11:18:07 -
> @@ -36,12 +36,25 @@
>  
>  #define MINIMUM(a, b)(((a) < (b)) ? (a) : (b))
>  #define MAXIMUM(a, b)(((a) > (b)) ? (a) : (b))
> +#define MAX_RANGES   4
>  
> -int   server_file_access(struct httpd *, struct client *, char *, size_t);
> -int   server_file_request(struct httpd *, struct client *, char *,
> - struct stat *);
> -int   server_file_index(struct httpd *, struct client *, struct stat *);
> -int   server_file_method(struct client *);
> +struct range {
> + off_t   start;
> + off_t   end;
> +};
> +
> +int   server_file_access(struct httpd *, struct client *,
> + char *, size_t);
> +int   server_file_request(struct httpd *, struct client *,
> + char *, struct stat *);
> +int   server_partial_file_request(struct httpd *, struct client *,
> + char *, struct stat *, char *);
> +int   server_file_index(struct httpd *, struct client *,
> + struct stat *);
> +int   server_file_method(struct client *);
> +int   parse_range_spec(char *, size_t, struct range *);
> +struct range *parse_range(char *, size_t, int *);
> +int   buffer_add_range(int, struct evbuffer *, struct range *);
>  
>  int
>  server_file_access(struct httpd *env, struct client *clt,
> @@ -50,6 +63,7 @@ server_file_access(struct httpd *env, st
>   struct http_descriptor  *desc = clt->clt_descreq;
>   struct server_config*srv_conf = clt->clt_srv_conf;
>   struct stat  st;
> + struct kv   *r, key;
>   char*newpath;
>   int  ret;
>  
> @@ -123,7 +137,13 @@ server_file_access(struct httpd *env, st
>   goto fail;
>   }
>  
> - return (server_file_request(env, clt, path, &st));
> + key.kv_key = "Range";
> + r = kv_find(&desc->http_headers, &key);
>

Re: Byte range implementation for httpd(8)

2015-05-03 Thread Sunil Nimmagadda
On Sat, May 02, 2015 at 02:49:30PM +, Florian Obser wrote:
> Sorry for the very late reply, I'm currently very busy :/

Thank you for taking time to review it. A new patch with style nits
fixed and a gratuitous NULL check removed.

[trimming some text]
 
> this is missing the server_file_method() song and dance from
> server_file_request()

Fixed in a slightly different way than using server_file_method().
Since range request should be ignored for methods other than GET,
fallback to server_file_request() for further processing/validation.

> > +   if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
> > +   code = 416;
> > +   (void)snprintf(content_range, sizeof(content_range),
> > +   "bytes */%lld", st->st_size);
> > +   errstr = content_range;
> > +   goto abort;
> > +   }
> 
> hm, apache answers with the full file and 200 if the range header is
> syntactically incorrect or if end < start. As far as I can tell it
> only answers 416 if the range actually lies outside of the file.

I am confused about the RFC here, section 4.4 states that 416 is
returned if "...the set of ranges requested has been rejected due
to invalid ranges..." and a note follows immediately, "Because
servers are free to ignore Range, many implementations will simply
respond with the entire selected representation in a 200 response"

As you noted apache returns 200 while nginx returns 416 for an
incorrect range. What should httpd do ideally?

> > +
> > +   /* Accept byte ranges */
> > +   if (code == 200 &&
> > +   kv_add(&resp->http_headers, "Accept-Ranges", "bytes") == NULL)
> > return (-1);
> 
> I don't think we should advertise ranges for all 200 results, for
> example we don't support it for directory indexes.

Agree, since RFC says "server MAY send Accept-Range header" and
"clients MAY generate range requests without having received this
header field", dropping this header shouldn't make a difference.

Comments?

Index: server_file.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
retrieving revision 1.52
diff -u -p -r1.52 server_file.c
--- server_file.c   25 Apr 2015 14:40:35 -  1.52
+++ server_file.c   3 May 2015 11:18:07 -
@@ -36,12 +36,25 @@
 
 #define MINIMUM(a, b)  (((a) < (b)) ? (a) : (b))
 #define MAXIMUM(a, b)  (((a) > (b)) ? (a) : (b))
+#define MAX_RANGES 4
 
-int server_file_access(struct httpd *, struct client *, char *, size_t);
-int server_file_request(struct httpd *, struct client *, char *,
-   struct stat *);
-int server_file_index(struct httpd *, struct client *, struct stat *);
-int server_file_method(struct client *);
+struct range {
+   off_t   start;
+   off_t   end;
+};
+
+int server_file_access(struct httpd *, struct client *,
+   char *, size_t);
+int server_file_request(struct httpd *, struct client *,
+   char *, struct stat *);
+int server_partial_file_request(struct httpd *, struct client *,
+   char *, struct stat *, char *);
+int server_file_index(struct httpd *, struct client *,
+   struct stat *);
+int server_file_method(struct client *);
+int parse_range_spec(char *, size_t, struct range *);
+struct range   *parse_range(char *, size_t, int *);
+int buffer_add_range(int, struct evbuffer *, struct range *);
 
 int
 server_file_access(struct httpd *env, struct client *clt,
@@ -50,6 +63,7 @@ server_file_access(struct httpd *env, st
struct http_descriptor  *desc = clt->clt_descreq;
struct server_config*srv_conf = clt->clt_srv_conf;
struct stat  st;
+   struct kv   *r, key;
char*newpath;
int  ret;
 
@@ -123,7 +137,13 @@ server_file_access(struct httpd *env, st
goto fail;
}
 
-   return (server_file_request(env, clt, path, &st));
+   key.kv_key = "Range";
+   r = kv_find(&desc->http_headers, &key);
+   if (r != NULL)
+   return (server_partial_file_request(env, clt, path, &st,
+   r->kv_value));
+   else
+   return (server_file_request(env, clt, path, &st));
 
  fail:
switch (errno) {
@@ -262,6 +282,143 @@ server_file_request(struct httpd *env, s
 }
 
 int
+server_partial_file_request(struct httpd *env, struct client *clt, char *path,
+struct stat *st, char *range_str)
+{
+   struct http_descriptor  *resp = clt->clt_descresp;
+   struct http_descriptor  *desc = clt->clt_descreq;
+   struct media_type   *media, multipart_media;
+   struct range*range;
+   struct evbuffer *evb = NULL;
+   size_t   content_length;
+   int  code = 500, fd = -1, i, nranges, ret;
+   

Re: Byte range implementation for httpd(8)

2015-05-02 Thread Florian Obser
Sorry for the very late reply, I'm currently very busy :/

On Fri, Apr 17, 2015 at 05:04:01AM +0200, Sunil Nimmagadda wrote:
> Range requests as defined in RFC7233 is required for resuming
> interrupted http(s) downloads for example:
> ftp -C http://foo.bar/install57.iso
> 
> With this diff, httpd parses "Range" header in the requests and
> provide either 206(Partial Content) or 416(Range not Satisfiable)
> responses with "Content-Range" header set appropriately. Further,
> it understands multi range request and generate satisfiable payloads
> with "multipart/byteranges" media type.
> 
> Suggestions/comments to improve the diff are welcome.
> 
> Note, "If-Range" isn't implemented yet.
> 
> Index: server_file.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 server_file.c
> --- server_file.c 12 Feb 2015 10:05:29 -  1.51
> +++ server_file.c 17 Apr 2015 02:22:12 -
> @@ -36,12 +36,23 @@
>  
>  #define MINIMUM(a, b)(((a) < (b)) ? (a) : (b))
>  #define MAXIMUM(a, b)(((a) > (b)) ? (a) : (b))
> +#define MAX_RANGES   4
> +
> +struct range {
> + off_t   start;
> + off_t   end;
> +};
>  
>  int   server_file_access(struct httpd *, struct client *, char *, size_t);
>  int   server_file_request(struct httpd *, struct client *, char *,
>   struct stat *);
> +int   server_partial_file_request(struct httpd *, struct client *, char *,
> + struct stat *, char *);
>  int   server_file_index(struct httpd *, struct client *, struct stat *);
>  int   server_file_method(struct client *);
> +int   parse_range_spec(char *, size_t, struct range *);
> +struct range *parse_range(char *, size_t, int *);
> +int   buffer_add_range(int, struct evbuffer *, struct range *);

This whole block now needs another tab indentation after the type
(except for parse_range of course)

>  
>  int
>  server_file_access(struct httpd *env, struct client *clt,
> @@ -50,6 +61,7 @@ server_file_access(struct httpd *env, st
>   struct http_descriptor  *desc = clt->clt_descreq;
>   struct server_config*srv_conf = clt->clt_srv_conf;
>   struct stat  st;
> + struct kv   *r, key;
>   char*newpath;
>   int  ret;
>  
> @@ -123,7 +135,13 @@ server_file_access(struct httpd *env, st
>   goto fail;
>   }
>  
> - return (server_file_request(env, clt, path, &st));
> + key.kv_key = "Range";
> + r = kv_find(&desc->http_headers, &key);
> + if (r != NULL)
> + return (server_partial_file_request(env, clt, path, &st,
> + r->kv_value));
> + else
> + return (server_file_request(env, clt, path, &st));
>  
>   fail:
>   switch (errno) {
> @@ -262,6 +280,138 @@ server_file_request(struct httpd *env, s
>  }
>  
>  int
> +server_partial_file_request(struct httpd *env, struct client *clt, char 
> *path,
> +struct stat *st, char *range_str)
> +{
> + struct http_descriptor  *resp = clt->clt_descresp;
> + struct media_type   *media, multipart_media;
> + struct range*range;
> + struct evbuffer *evb = NULL;
> + size_t   content_length;
> + int  code = 500, fd = -1, i, nranges, ret;
> + char content_range[64];
> + const char  *errstr = NULL;
> + uint32_t boundary;

Nit: uint32_t should be below int

> +

this is missing the server_file_method() song and dance from
server_file_request()

> + if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
> + code = 416;
> + (void)snprintf(content_range, sizeof(content_range),
> + "bytes */%lld", st->st_size);
> + errstr = content_range;
> + goto abort;
> + }

hm, apache answers with the full file and 200 if the range header is
syntactically incorrect or if end < start. As far as I can tell it
only answers 416 if the range actually lies outside of the file.

> +
> + /* Now open the file, should be readable or we have another problem */
> + if ((fd = open(path, O_RDONLY)) == -1)
> + goto abort;
> +
> + media = media_find(env->sc_mediatypes, path);
> + if ((evb = evbuffer_new()) == NULL) {
> + errstr = "failed to allocate file buffer";
> + goto abort;
> + }
> +
> + if (nranges == 1) {
> + (void)snprintf(content_range, sizeof(content_range),
> + "bytes %lld-%lld/%lld", range->start, range->end,
> + st->st_size);
> + if (kv_add(&resp->http_headers, "Content-Range",
> + content_range) == NULL)
> + goto abort;
> +
> + content_length = range->end - range->start + 1;
> + if (buffer_add_range(fd, evb, range) 

Re: Byte range implementation for httpd(8)

2015-04-23 Thread Sunil Nimmagadda
Any interest/comments/suggestions for this diff...

On Fri, Apr 17, 2015 at 05:04:01AM +0200, Sunil Nimmagadda wrote:
> Range requests as defined in RFC7233 is required for resuming
> interrupted http(s) downloads for example:
> ftp -C http://foo.bar/install57.iso
> 
> With this diff, httpd parses "Range" header in the requests and
> provide either 206(Partial Content) or 416(Range not Satisfiable)
> responses with "Content-Range" header set appropriately. Further,
> it understands multi range request and generate satisfiable payloads
> with "multipart/byteranges" media type.
> 
> Suggestions/comments to improve the diff are welcome.
> 
> Note, "If-Range" isn't implemented yet.
> 
> Index: server_file.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 server_file.c
> --- server_file.c 12 Feb 2015 10:05:29 -  1.51
> +++ server_file.c 17 Apr 2015 02:22:12 -
> @@ -36,12 +36,23 @@
>  
>  #define MINIMUM(a, b)(((a) < (b)) ? (a) : (b))
>  #define MAXIMUM(a, b)(((a) > (b)) ? (a) : (b))
> +#define MAX_RANGES   4
> +
> +struct range {
> + off_t   start;
> + off_t   end;
> +};
>  
>  int   server_file_access(struct httpd *, struct client *, char *, size_t);
>  int   server_file_request(struct httpd *, struct client *, char *,
>   struct stat *);
> +int   server_partial_file_request(struct httpd *, struct client *, char *,
> + struct stat *, char *);
>  int   server_file_index(struct httpd *, struct client *, struct stat *);
>  int   server_file_method(struct client *);
> +int   parse_range_spec(char *, size_t, struct range *);
> +struct range *parse_range(char *, size_t, int *);
> +int   buffer_add_range(int, struct evbuffer *, struct range *);
>  
>  int
>  server_file_access(struct httpd *env, struct client *clt,
> @@ -50,6 +61,7 @@ server_file_access(struct httpd *env, st
>   struct http_descriptor  *desc = clt->clt_descreq;
>   struct server_config*srv_conf = clt->clt_srv_conf;
>   struct stat  st;
> + struct kv   *r, key;
>   char*newpath;
>   int  ret;
>  
> @@ -123,7 +135,13 @@ server_file_access(struct httpd *env, st
>   goto fail;
>   }
>  
> - return (server_file_request(env, clt, path, &st));
> + key.kv_key = "Range";
> + r = kv_find(&desc->http_headers, &key);
> + if (r != NULL)
> + return (server_partial_file_request(env, clt, path, &st,
> + r->kv_value));
> + else
> + return (server_file_request(env, clt, path, &st));
>  
>   fail:
>   switch (errno) {
> @@ -262,6 +280,138 @@ server_file_request(struct httpd *env, s
>  }
>  
>  int
> +server_partial_file_request(struct httpd *env, struct client *clt, char 
> *path,
> +struct stat *st, char *range_str)
> +{
> + struct http_descriptor  *resp = clt->clt_descresp;
> + struct media_type   *media, multipart_media;
> + struct range*range;
> + struct evbuffer *evb = NULL;
> + size_t   content_length;
> + int  code = 500, fd = -1, i, nranges, ret;
> + char content_range[64];
> + const char  *errstr = NULL;
> + uint32_t boundary;
> +
> + if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
> + code = 416;
> + (void)snprintf(content_range, sizeof(content_range),
> + "bytes */%lld", st->st_size);
> + errstr = content_range;
> + goto abort;
> + }
> +
> + /* Now open the file, should be readable or we have another problem */
> + if ((fd = open(path, O_RDONLY)) == -1)
> + goto abort;
> +
> + media = media_find(env->sc_mediatypes, path);
> + if ((evb = evbuffer_new()) == NULL) {
> + errstr = "failed to allocate file buffer";
> + goto abort;
> + }
> +
> + if (nranges == 1) {
> + (void)snprintf(content_range, sizeof(content_range),
> + "bytes %lld-%lld/%lld", range->start, range->end,
> + st->st_size);
> + if (kv_add(&resp->http_headers, "Content-Range",
> + content_range) == NULL)
> + goto abort;
> +
> + content_length = range->end - range->start + 1;
> + if (buffer_add_range(fd, evb, range) == 0)
> + goto abort;
> +
> + } else {
> + content_length = 0;
> + boundary = arc4random();
> + /* Generate a multipart payload of byteranges */
> + while (nranges--) {
> + if ((i = evbuffer_add_printf(evb, "\r\n--%ud\r\n",
> + boundary)) == -1)
> + goto abort;
> +
> + 

Byte range implementation for httpd(8)

2015-04-16 Thread Sunil Nimmagadda
Range requests as defined in RFC7233 is required for resuming
interrupted http(s) downloads for example:
ftp -C http://foo.bar/install57.iso

With this diff, httpd parses "Range" header in the requests and
provide either 206(Partial Content) or 416(Range not Satisfiable)
responses with "Content-Range" header set appropriately. Further,
it understands multi range request and generate satisfiable payloads
with "multipart/byteranges" media type.

Suggestions/comments to improve the diff are welcome.

Note, "If-Range" isn't implemented yet.

Index: server_file.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
retrieving revision 1.51
diff -u -p -r1.51 server_file.c
--- server_file.c   12 Feb 2015 10:05:29 -  1.51
+++ server_file.c   17 Apr 2015 02:22:12 -
@@ -36,12 +36,23 @@
 
 #define MINIMUM(a, b)  (((a) < (b)) ? (a) : (b))
 #define MAXIMUM(a, b)  (((a) > (b)) ? (a) : (b))
+#define MAX_RANGES 4
+
+struct range {
+   off_t   start;
+   off_t   end;
+};
 
 int server_file_access(struct httpd *, struct client *, char *, size_t);
 int server_file_request(struct httpd *, struct client *, char *,
struct stat *);
+int server_partial_file_request(struct httpd *, struct client *, char *,
+   struct stat *, char *);
 int server_file_index(struct httpd *, struct client *, struct stat *);
 int server_file_method(struct client *);
+int parse_range_spec(char *, size_t, struct range *);
+struct range *parse_range(char *, size_t, int *);
+int buffer_add_range(int, struct evbuffer *, struct range *);
 
 int
 server_file_access(struct httpd *env, struct client *clt,
@@ -50,6 +61,7 @@ server_file_access(struct httpd *env, st
struct http_descriptor  *desc = clt->clt_descreq;
struct server_config*srv_conf = clt->clt_srv_conf;
struct stat  st;
+   struct kv   *r, key;
char*newpath;
int  ret;
 
@@ -123,7 +135,13 @@ server_file_access(struct httpd *env, st
goto fail;
}
 
-   return (server_file_request(env, clt, path, &st));
+   key.kv_key = "Range";
+   r = kv_find(&desc->http_headers, &key);
+   if (r != NULL)
+   return (server_partial_file_request(env, clt, path, &st,
+   r->kv_value));
+   else
+   return (server_file_request(env, clt, path, &st));
 
  fail:
switch (errno) {
@@ -262,6 +280,138 @@ server_file_request(struct httpd *env, s
 }
 
 int
+server_partial_file_request(struct httpd *env, struct client *clt, char *path,
+struct stat *st, char *range_str)
+{
+   struct http_descriptor  *resp = clt->clt_descresp;
+   struct media_type   *media, multipart_media;
+   struct range*range;
+   struct evbuffer *evb = NULL;
+   size_t   content_length;
+   int  code = 500, fd = -1, i, nranges, ret;
+   char content_range[64];
+   const char  *errstr = NULL;
+   uint32_t boundary;
+
+   if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
+   code = 416;
+   (void)snprintf(content_range, sizeof(content_range),
+   "bytes */%lld", st->st_size);
+   errstr = content_range;
+   goto abort;
+   }
+
+   /* Now open the file, should be readable or we have another problem */
+   if ((fd = open(path, O_RDONLY)) == -1)
+   goto abort;
+
+   media = media_find(env->sc_mediatypes, path);
+   if ((evb = evbuffer_new()) == NULL) {
+   errstr = "failed to allocate file buffer";
+   goto abort;
+   }
+
+   if (nranges == 1) {
+   (void)snprintf(content_range, sizeof(content_range),
+   "bytes %lld-%lld/%lld", range->start, range->end,
+   st->st_size);
+   if (kv_add(&resp->http_headers, "Content-Range",
+   content_range) == NULL)
+   goto abort;
+
+   content_length = range->end - range->start + 1;
+   if (buffer_add_range(fd, evb, range) == 0)
+   goto abort;
+
+   } else {
+   content_length = 0;
+   boundary = arc4random();
+   /* Generate a multipart payload of byteranges */
+   while (nranges--) {
+   if ((i = evbuffer_add_printf(evb, "\r\n--%ud\r\n",
+   boundary)) == -1)
+   goto abort;
+
+   content_length += i;
+   if ((i = evbuffer_add_printf(evb,
+   "Content-Type: %s/%s\r\n",
+   media == NULL ? "application" : media->media_type,
+