[patch] security.html page
Hi @tech, This patch for fix security.html page - cleanup not found link to errata20.html - add link to errata57.html Index: security.html === RCS file: /cvs/www/security.html,v retrieving revision 1.419 diff -u -p -u -p -r1.419 security.html --- security.html6 Sep 2014 14:12:17 -1.419 +++ security.html2 May 2015 08:20:49 - @@ -21,7 +21,6 @@ For security advisories for specific releases, click below: -2.0, 2.1, 2.2, 2.3, @@ -38,8 +37,8 @@ For security advisories for specific rel 3.4, 3.5, 3.6, - 3.7, + 3.8, 3.9, 4.0, @@ -56,10 +55,11 @@ For security advisories for specific rel 5.1, 5.2, 5.3, - 5.4, + 5.5, -5.6. +5.6, +5.7.
Re: Byte range implementation for httpd(8)
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)
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: [PATCH] Support If-Modified-Since header on requests in httpd
On Sat, Apr 18, 2015 at 12:19:46PM -0500, jmp wrote:
> I found 'timeoff' to be useful for converting to a time_t that is in
> GMT; however, did not find documentation on this in the man pages. It
> seems to be a function dating back to at least the NetBSD fork. If
> there is a better time function I should be using please let me know.
I think timegm(3) is acceptable.
> Index: usr.sbin/httpd/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
> --- usr.sbin/httpd/server_file.c 12 Feb 2015 10:05:29 - 1.51
> +++ usr.sbin/httpd/server_file.c 18 Apr 2015 16:41:55 -
> @@ -42,6 +42,7 @@ int server_file_request(struct httpd *,
> struct stat *);
> int server_file_index(struct httpd *, struct client *, struct stat *);
> int server_file_method(struct client *);
> +int server_file_modified_since(struct http_descriptor *, struct stat *);
>
> int
> server_file_access(struct httpd *env, struct client *clt,
> @@ -123,6 +124,10 @@ server_file_access(struct httpd *env, st
> goto fail;
> }
>
> + if ((ret = server_file_modified_since(desc, &st)) != -1) {
> + return ret;
> + }
> +
RFC 7232
A recipient MUST ignore the If-Modified-Since header field if the
received field-value is not a valid HTTP-date, or if the request
method is neither GET nor HEAD.
> return (server_file_request(env, clt, path, &st));
>
> fail:
> @@ -466,4 +471,24 @@ server_file_error(struct bufferevent *be
> }
> server_close(clt, "unknown event error");
> return;
> +}
> +
> +int
> +server_file_modified_since(struct http_descriptor * desc, struct stat * st)
> +{
> + struct kvkey, *since;
> + struct tmtm;
> +
> + memset(&tm, 0, sizeof(struct tm));
> +
> + key.kv_key = "If-Modified-Since";
> + if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
> + since->kv_value != NULL) {
> + if (strptime(since->kv_value, "%a, %d %h %Y %T %Z", &tm) !=
> NULL &&
> + timeoff(&tm, 0L) >= st->st_mtim.tv_sec) {
> + return 304;
> + }
> + }
RFC 7231 defines 3 formats for HTTP-date and then goes on:
A recipient that parses a timestamp value in an HTTP header field
MUST accept all three HTTP-date formats.
I think it's ok here to only parse one variation and ignore
If-Modified-Since otherwise, we will just respond with a 200.
> +
> + return (-1);
> }
>
Here is a tweaked version of the diff (check for GET/HEAD; timegm(3)).
This is OK florian@ or if someone wants to OK it I can commit it.
diff --git server_file.c server_file.c
index 3580bbb..9abf83b 100644
--- server_file.c
+++ server_file.c
@@ -42,6 +42,7 @@ intserver_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 *);
+int server_file_modified_since(struct http_descriptor *, struct stat *);
int
server_file_access(struct httpd *env, struct client *clt,
@@ -123,6 +124,12 @@ server_file_access(struct httpd *env, struct client *clt,
goto fail;
}
+ if (desc->http_method == HTTP_METHOD_GET || desc->http_method ==
+ HTTP_METHOD_HEAD) {
+ if ((ret = server_file_modified_since(desc, &st)) != -1)
+ return ret;
+ }
+
return (server_file_request(env, clt, path, &st));
fail:
@@ -469,3 +476,23 @@ server_file_error(struct bufferevent *bev, short error,
void *arg)
server_close(clt, "unknown event error");
return;
}
+
+int
+server_file_modified_since(struct http_descriptor * desc, struct stat * st)
+{
+ struct kvkey, *since;
+ struct tmtm;
+
+ memset(&tm, 0, sizeof(struct tm));
+
+ key.kv_key = "If-Modified-Since";
+ if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
+ since->kv_value != NULL) {
+ if (strptime(since->kv_value, "%a, %d %h %Y %T %Z", &tm) !=
+ NULL && timegm(&tm) >= st->st_mtim.tv_sec) {
+ return (304);
+ }
+ }
+
+ return (-1);
+}
--
I'm not entirely sure you are real.
Re: [PATCH] If-Modified-Since support in httpd
I haven't heard back from anyone. Since the release has passed, has
anyone had time to look at this?
I think that I should move the time parsing out of server_file
to server_http so it can be reused later. I'm also not sure about
the placement of the check. Additionally, I'm using timeoff which seems
to not be documented anywhere.
On Sat, Apr 18, 2015 at 04:28:40PM -0500, jmp wrote:
> If-Modified-Since is already sent by most web browsers to httpd. This
> patch adds a check to server_file_access before we send the file back.
>
> This does not do any checks for autoindex directories as the size and
> last modification dates of files would not get updated properly.
>
> I separated the logic for checking the header values as it can be
> reused for different side effects of other headers like Range.
>
>
> Index: usr.sbin/httpd/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
> --- usr.sbin/httpd/server_file.c 12 Feb 2015 10:05:29 - 1.51
> +++ usr.sbin/httpd/server_file.c 18 Apr 2015 16:41:55 -
> @@ -42,6 +42,7 @@ int server_file_request(struct httpd *,
> struct stat *);
> int server_file_index(struct httpd *, struct client *, struct stat *);
> int server_file_method(struct client *);
> +int server_file_modified_since(struct http_descriptor *, struct stat *);
>
> int
> server_file_access(struct httpd *env, struct client *clt,
> @@ -123,6 +124,10 @@ server_file_access(struct httpd *env, st
> goto fail;
> }
>
> + if ((ret = server_file_modified_since(desc, &st)) != -1) {
> + return ret;
> + }
> +
> return (server_file_request(env, clt, path, &st));
>
> fail:
> @@ -466,4 +471,24 @@ server_file_error(struct bufferevent *be
> }
> server_close(clt, "unknown event error");
> return;
> +}
> +
> +int
> +server_file_modified_since(struct http_descriptor * desc, struct stat * st)
> +{
> + struct kvkey, *since;
> + struct tmtm; > +
> + memset(&tm, 0, sizeof(struct tm));
> +
> + key.kv_key = "If-Modified-Since";
> + if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
> + since->kv_value != NULL) {
> + if (strptime(since->kv_value, "%a, %d %h %Y %T %Z", &tm) !=
> NULL &&
> + timeoff(&tm, 0L) >= st->st_mtim.tv_sec) {
> + return 304;
> + }
> + }
> +
> + return (-1);
> }
>
Re: [PATCH] Support If-Modified-Since header on requests in httpd
On Sun, May 03, 2015 at 03:00:40PM +, Florian Obser wrote:
> On Sat, Apr 18, 2015 at 12:19:46PM -0500, jmp wrote:
> RFC 7232
>
>A recipient MUST ignore the If-Modified-Since header field if the
>received field-value is not a valid HTTP-date, or if the request
>method is neither GET nor HEAD.
>
Does httpd allow any other types of requests through server_file.c? All
other types of requests should only get sent through the CGI scripts. It
doesn't make since to allow POST, PUT, etc.. through to the file
handler.
>
> > return (server_file_request(env, clt, path, &st));
> >
> > fail:
> > @@ -466,4 +471,24 @@ server_file_error(struct bufferevent *be
> > }
> > server_close(clt, "unknown event error");
> > return;
> > +}
> > +
> > +int
> > +server_file_modified_since(struct http_descriptor * desc, struct stat * st)
> > +{
> > + struct kvkey, *since;
> > + struct tmtm;
> > +
> > + memset(&tm, 0, sizeof(struct tm));
> > +
> > + key.kv_key = "If-Modified-Since";
> > + if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
> > + since->kv_value != NULL) {
> > + if (strptime(since->kv_value, "%a, %d %h %Y %T %Z", &tm) !=
> > NULL &&
> > + timeoff(&tm, 0L) >= st->st_mtim.tv_sec) {
> > + return 304;
> > + }
> > + }
>
> RFC 7231 defines 3 formats for HTTP-date and then goes on:
>A recipient that parses a timestamp value in an HTTP header field
>MUST accept all three HTTP-date formats.
>
> I think it's ok here to only parse one variation and ignore
> If-Modified-Since otherwise, we will just respond with a 200.
>
>From looking at Apache and nginx code, I wasn't able to see that they
used any other method. Like I said in my 'other' reply, we can always
extract this out to server_http since the Date header is created there.
Re: [PATCH] Support If-Modified-Since header on requests in httpd
On Sun, May 03, 2015 at 11:14:48AM -0500, Kyle Thompson wrote:
> On Sun, May 03, 2015 at 03:00:40PM +, Florian Obser wrote:
> > On Sat, Apr 18, 2015 at 12:19:46PM -0500, jmp wrote:
> > RFC 7232
> >
> >A recipient MUST ignore the If-Modified-Since header field if the
> >received field-value is not a valid HTTP-date, or if the request
> >method is neither GET nor HEAD.
> >
>
> Does httpd allow any other types of requests through server_file.c? All
> other types of requests should only get sent through the CGI scripts. It
> doesn't make since to allow POST, PUT, etc.. through to the file
> handler.
yes, the check for that is later. This is better:
diff --git server_file.c server_file.c
index 3580bbb..42e2965 100644
--- server_file.c
+++ server_file.c
@@ -42,6 +42,7 @@ intserver_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 *);
+int server_file_modified_since(struct http_descriptor *, struct stat *);
int
server_file_access(struct httpd *env, struct client *clt,
@@ -209,6 +210,9 @@ server_file_request(struct httpd *env, struct client *clt,
char *path,
goto abort;
}
+ if ((ret = server_file_modified_since(clt->clt_descreq, st)) != -1)
+ return ret;
+
/* Now open the file, should be readable or we have another problem */
if ((fd = open(path, O_RDONLY)) == -1)
goto abort;
@@ -469,3 +473,23 @@ server_file_error(struct bufferevent *bev, short error,
void *arg)
server_close(clt, "unknown event error");
return;
}
+
+int
+server_file_modified_since(struct http_descriptor * desc, struct stat * st)
+{
+ struct kvkey, *since;
+ struct tmtm;
+
+ memset(&tm, 0, sizeof(struct tm));
+
+ key.kv_key = "If-Modified-Since";
+ if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
+ since->kv_value != NULL) {
+ if (strptime(since->kv_value, "%a, %d %h %Y %T %Z", &tm) !=
+ NULL && timegm(&tm) >= st->st_mtim.tv_sec) {
+ return (304);
+ }
+ }
+
+ return (-1);
+}
--
I'm not entirely sure you are real.
Re: Byte range implementation for httpd(8)
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: Fix for smtpd offline enqueue
On Sat, May 02, 2015 at 12:27:46PM +0800, Nathanael Rensen wrote: > The smtpd enqueue -S option does not take an argument. > committed, thanks -- Gilles Chehade https://www.poolp.org @poolpOrg
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.
seccomp system call
I am wondering if the seccomp system call [1] would be welcomed in the OpenBSD tree. I remember it was among the subjects of last year's Google Summer of Code. If there is still interest in having it implemented, I am willing to work on it: I have a diff that creates the system call and allows seccomp to be called with the SECCOMP_SET_MODE_STRICT operation. It's a first step, the next (big) one would be BPF(4) syscall filtering. [1] http://man7.org/linux/man-pages/man2/seccomp.2.html
Re: seccomp system call
On Sun, May 3, 2015 at 8:18 PM, Nicolas Bedos wrote: > I am wondering if the seccomp system call [1] would be welcomed in the > OpenBSD tree. I remember it was among the subjects of last year's Google > Summer of Code. If there is still interest in having it implemented, I > am willing to work on it: I have a diff that creates the system call and > allows seccomp to be called with the SECCOMP_SET_MODE_STRICT operation. > It's a first step, the next (big) one would be BPF(4) syscall filtering. > > > [1] http://man7.org/linux/man-pages/man2/seccomp.2.html > OpenBSD already has systrace. http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man4/systrace.4?query=systrace&arch=i386 If you have interest in this kind of stuff, I would advise looking at what is done in sshd, and more recently, file(1). (for file(1): see http://www.freshbsd.org/commit/openbsd/95b5f38db7636a4aaf9af03aaf0bd2019f8aa6cf).
Re: seccomp system call
On Sun, 3 May 2015, Nicolas Bedos wrote: > I am wondering if the seccomp system call [1] would be welcomed > in the OpenBSD tree. I remember it was among the subjects of last > year's Google Summer of Code. If there is still interest in having > it implemented, I am willing to work on it: I have a diff that > creates the system call and allows seccomp to be called with the > SECCOMP_SET_MODE_STRICT operation. It's a first step, the next (big) > one would be BPF(4) syscall filtering. Personally, I think seccomp-bpf could be a superior alternative to systrace and I'd love to see an implementation. Other developers (inc. Theo) are skeptical though, but this is probably a case where the argument won't be settled without a concrete implementation to look at. I'd welcome any work you do on it. -d
Re: Byte range implementation for httpd(8)
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.
more precise O_ACCMODE handling
The flags passed to open(2) and openat(2) are not a simple bitset.
Instead, the bits in O_ACCMODE are effectively an enumeration, and the
other bits are or'ed onto that.
For example, a function that wraps open(2), taking a flag argument that it
passes through, that wants to verify that it was invoked with either
O_WRONLY or O_RDWR *cannot* just say
if ((flags & O_ACCMODE) & ~(O_WRONLY | O_RDWR))
return (EINVAL);
as that will accept O_RDONLY as well! The correct test is to mask off
O_ACCMODE and compare the results to the acceptable values:
if ((flags & O_ACCMODE) != O_WRONLY && (flags & O_ACCMODE) != O_RDWR)
return (EINVAL);
So, in anticipation of future changes, let's make interfaces that wrap
open() be more precise in the handling of flags for open(), testing
O_ACCMODE bits separately from others.
While here, document that shm_open() accept O_CLOEXEC and O_NOFOLLOW as
extensions to POSIX, and use O_CLOEXEC on the temporary fd in
posix_openpt()
ok?
Philip
Index: libc/db/db/db.c
===
RCS file: /cvs/src/lib/libc/db/db/db.c,v
retrieving revision 1.10
diff -u -p -r1.10 db.c
--- libc/db/db/db.c 5 Aug 2005 13:03:00 - 1.10
+++ libc/db/db/db.c 4 May 2015 04:48:11 -
@@ -48,9 +48,10 @@ dbopen(const char *fname, int flags, int
#defineDB_FLAGS(DB_LOCK | DB_SHMEM | DB_TXN)
#defineUSE_OPEN_FLAGS
\
(O_CREAT | O_EXCL | O_EXLOCK | O_NOFOLLOW | O_NONBLOCK |\
-O_RDONLY | O_RDWR | O_SHLOCK | O_SYNC | O_TRUNC)
+O_SHLOCK | O_SYNC | O_TRUNC)
- if ((flags & ~(USE_OPEN_FLAGS | DB_FLAGS)) == 0)
+ if (((flags & O_ACCMODE) == O_RDONLY || (flags & O_ACCMODE) == O_RDWR)
+ && (flags & ~(O_ACCMODE | USE_OPEN_FLAGS | DB_FLAGS)) == 0)
switch (type) {
case DB_BTREE:
return (__bt_open(fname, flags & USE_OPEN_FLAGS,
Index: libc/gen/shm_open.3
===
RCS file: /cvs/src/lib/libc/gen/shm_open.3,v
retrieving revision 1.4
diff -u -p -r1.4 shm_open.3
--- libc/gen/shm_open.3 8 Jul 2014 00:40:56 - 1.4
+++ libc/gen/shm_open.3 4 May 2015 04:48:11 -
@@ -45,7 +45,7 @@ and must include at least
or
.Dv O_RDWR
and may also include a combination of
-.Dv O_CREAT , O_EXCL ,
+.Dv O_CREAT , O_EXCL , O_CLOEXEC , O_NOFOLLOW ,
or
.Dv O_TRUNC .
This implementation forces the
@@ -81,6 +81,13 @@ and
.Fn shm_unlink
appear in
.St -p1003.1-2001 .
+Using
+.Dv O_CLOEXEC
+or
+.Dv O_NOFOLLOW
+with
+.Fn shm_open
+is an extension to that standard.
This implementation deviates from the standard by permitting less sharing.
.Pp
.Fn shm_mkstemp
Index: libc/gen/shm_open.c
===
RCS file: /cvs/src/lib/libc/gen/shm_open.c,v
retrieving revision 1.4
diff -u -p -r1.4 shm_open.c
--- libc/gen/shm_open.c 12 Nov 2013 06:09:48 - 1.4
+++ libc/gen/shm_open.c 4 May 2015 04:48:11 -
@@ -31,6 +31,9 @@
/* "/tmp/" + sha256 + ".shm" */
#define SHM_PATH_SIZE (5 + SHA256_DIGEST_STRING_LENGTH + 4)
+/* O_CLOEXEC and O_NOFOLLOW are extensions to POSIX */
+#define OK_FLAGS (O_CREAT | O_EXCL | O_TRUNC | O_CLOEXEC | O_NOFOLLOW)
+
static void
makeshmpath(const char *origpath, char *shmpath, size_t len)
{
@@ -47,8 +50,8 @@ shm_open(const char *path, int flags, mo
struct stat sb;
int fd;
- if (flags & ~(O_RDONLY | O_RDWR |
- O_CREAT | O_EXCL | O_TRUNC | O_CLOEXEC | O_NOFOLLOW)) {
+ if (((flags & O_ACCMODE) != O_RDONLY && (flags & O_ACCMODE) != O_RDWR)
+ || (flags & ~(O_ACCMODE | OK_FLAGS))) {
errno = EINVAL;
return -1;
}
Index: libc/stdlib/posix_pty.c
===
RCS file: /cvs/src/lib/libc/stdlib/posix_pty.c,v
retrieving revision 1.1
diff -u -p -r1.1 posix_pty.c
--- libc/stdlib/posix_pty.c 3 Dec 2012 20:08:33 - 1.1
+++ libc/stdlib/posix_pty.c 4 May 2015 04:48:11 -
@@ -35,13 +35,14 @@ posix_openpt(int oflag)
int fd, mfd = -1;
/* User must specify O_RDWR in oflag. */
- if (!(oflag & O_RDWR)) {
+ if ((oflag & O_ACCMODE) != O_RDWR ||
+ (oflag & ~(O_ACCMODE | O_NOCTTY)) != 0) {
errno = EINVAL;
return -1;
}
/* Get pty master and slave (this API only uses the master). */
- fd = open(PATH_PTMDEV, O_RDWR);
+ fd = open(PATH_PTMDEV, O_RDWR | O_CLOEXEC);
if (fd != -1) {
if (ioctl(fd, PTMGET, &ptm) != -1) {
close(ptm.sfd);
Index: libkvm/kvm.c
===
RCS file: /cvs/src/lib/libkvm/kvm.c,v
retrieving revision 1.54
diff -u -p -r1.54 kvm.c
--- libkv
