(Cc: semarie@ added)
On Wed, Jul 22, 2015 at 10:04:30PM -0400, Jean-Philippe Ouellet wrote:
> If you have a CGI script that produces multiple headers in separate writes,
> then they may be delivered to httpd as separate FastCGI records, depending on
> a race between the CGI script writing more headers and slowcgi framing them
> and passing them on to httpd.
>
> The fcgi code erroneously believes all headers will appear in the first chunk.
>
> From server_fcgi.c v1.58:
> 545 case FCGI_STDOUT:
> 546 if (++clt->clt_chunk == 1) {
> 547 if (server_fcgi_header(clt,
>
> The The net effect is subsequent headers being leaked into the body of the
> http reply.
Thanks, nice find.
The intention was to only get the Status header from fcgi, there
is even a comment alluding to that:
/* Write initial header (fcgi might append more) */
Don't know if that ever worked or if it broke once we started messing
with the headers ourself.
I first tried to restore the original intention but that won't work
with chunked encoding - we would chunk encode part of the headers.
This is the big hammer solution: read all http headers comming from fcgi.
OK?
Index: httpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.92
diff -u -p -r1.92 httpd.h
--- httpd.h 19 Jul 2015 05:17:27 -0000 1.92
+++ httpd.h 25 Jul 2015 21:00:52 -0000
@@ -316,6 +316,12 @@ struct client {
int clt_fcgi_type;
int clt_fcgi_chunked;
int clt_fcgi_end;
+ struct evbuffer *clt_fcgi_http_header_evb;
+ int clt_fcgi_http_header_state;
+#define FCGI_HTTP_HEADER_ERROR -1
+#define FCGI_HTTP_HEADER_UNREAD 0
+#define FCGI_HTTP_HEADER_READ 1
+#define FCGI_HTTP_HEADER_DONE 2
char *clt_remote_user;
struct evbuffer *clt_srvevb;
Index: server.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server.c,v
retrieving revision 1.71
diff -u -p -r1.71 server.c
--- server.c 18 Jul 2015 22:19:50 -0000 1.71
+++ server.c 25 Jul 2015 21:00:52 -0000
@@ -1093,6 +1093,8 @@ server_close(struct client *clt, const c
evbuffer_free(clt->clt_output);
if (clt->clt_srvevb != NULL)
evbuffer_free(clt->clt_srvevb);
+ if (clt->clt_fcgi_http_header_evb != NULL)
+ evbuffer_free(clt->clt_fcgi_http_header_evb);
if (clt->clt_srvbev != NULL)
bufferevent_free(clt->clt_srvbev);
Index: server_fcgi.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.58
diff -u -p -r1.58 server_fcgi.c
--- server_fcgi.c 19 Jul 2015 16:34:35 -0000 1.58
+++ server_fcgi.c 25 Jul 2015 21:00:52 -0000
@@ -76,6 +76,7 @@ struct server_fcgi_param {
uint8_t buf[FCGI_RECORD_SIZE];
};
+int server_fcgi_read_header(struct client *);
int server_fcgi_header(struct client *, u_int);
void server_fcgi_read(struct bufferevent *, void *);
int server_fcgi_writeheader(struct client *, struct kv *, void *);
@@ -154,6 +155,15 @@ server_fcgi(struct httpd *env, struct cl
goto fail;
}
+ if (clt->clt_fcgi_http_header_evb != NULL)
+ evbuffer_free(clt->clt_fcgi_http_header_evb);
+
+ clt->clt_fcgi_http_header_evb = evbuffer_new();
+ if (clt->clt_fcgi_http_header_evb == NULL) {
+ errstr = "failed to allocate evbuffer";
+ goto fail;
+ }
+
close(clt->clt_fd);
clt->clt_fd = fd;
@@ -543,7 +553,22 @@ server_fcgi_read(struct bufferevent *bev
}
break;
case FCGI_STDOUT:
- if (++clt->clt_chunk == 1) {
+ clt->clt_chunk++;
+ if (clt->clt_fcgi_http_header_state ==
+ FCGI_HTTP_HEADER_UNREAD) {
+ clt->clt_fcgi_http_header_state =
+ server_fcgi_read_header(clt);
+ if (clt->clt_fcgi_http_header_state ==
+ FCGI_HTTP_HEADER_ERROR) {
+ server_abort_http(clt,
+ 500, "out of memory");
+ return;
+ }
+ }
+ if (clt->clt_fcgi_http_header_state ==
+ FCGI_HTTP_HEADER_READ) {
+ clt->clt_fcgi_http_header_state =
+ FCGI_HTTP_HEADER_DONE;
if (server_fcgi_header(clt,
server_fcgi_getheaders(clt))
== -1) {
@@ -553,7 +578,8 @@ server_fcgi_read(struct bufferevent *bev
}
if (!EVBUFFER_LENGTH(clt->clt_srvevb))
break;
- }
+ } else
+ break;
/* FALLTHROUGH */
case FCGI_END_REQUEST:
if (server_fcgi_writechunk(clt) == -1) {
@@ -587,6 +613,35 @@ server_fcgi_read(struct bufferevent *bev
}
int
+server_fcgi_read_header(struct client *clt)
+{
+ size_t len;
+ int ret = FCGI_HTTP_HEADER_UNREAD;
+ u_char *ptr;
+
+ if ((ptr = evbuffer_find(clt->clt_srvevb, "\r\n\r\n", 4)) != NULL) {
+ len = ptr - EVBUFFER_DATA(clt->clt_srvevb) + 4;
+ if (evbuffer_add(clt->clt_fcgi_http_header_evb,
+ EVBUFFER_DATA(clt->clt_srvevb), len) == -1)
+ ret = FCGI_HTTP_HEADER_ERROR;
+ ret = FCGI_HTTP_HEADER_READ;
+ } else if ((ptr = evbuffer_find(clt->clt_srvevb, "\n\n", 2)) != NULL) {
+ len = ptr - EVBUFFER_DATA(clt->clt_srvevb) + 2;
+ if (evbuffer_add(clt->clt_fcgi_http_header_evb,
+ EVBUFFER_DATA(clt->clt_srvevb), len) == -1)
+ ret = FCGI_HTTP_HEADER_ERROR;
+ ret = FCGI_HTTP_HEADER_READ;
+ } else {
+ len = EVBUFFER_LENGTH(clt->clt_srvevb);
+ if (evbuffer_add_buffer(clt->clt_fcgi_http_header_evb,
+ clt->clt_srvevb) == -1)
+ ret = FCGI_HTTP_HEADER_ERROR;
+ }
+ evbuffer_drain(clt->clt_srvevb, len);
+ return (ret);
+}
+
+int
server_fcgi_header(struct client *clt, u_int code)
{
struct http_descriptor *desc = clt->clt_descreq;
@@ -639,7 +694,7 @@ server_fcgi_header(struct client *clt, u
kv_add(&resp->http_headers, "Date", tmbuf) == NULL)
return (-1);
- /* Write initial header (fcgi might append more) */
+ /* Write header */
if (server_writeresponse_http(clt) == -1 ||
server_bufferevent_print(clt, "\r\n") == -1 ||
server_headers(clt, resp, server_writeheader_http, NULL) == -1 ||
@@ -730,7 +785,7 @@ int
server_fcgi_getheaders(struct client *clt)
{
struct http_descriptor *resp = clt->clt_descresp;
- struct evbuffer *evb = clt->clt_srvevb;
+ struct evbuffer *evb = clt->clt_fcgi_http_header_evb;
int code = 200;
char *line, *key, *value;
const char *errstr;
Index: server_http.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.93
diff -u -p -r1.93 server_http.c
--- server_http.c 23 Jul 2015 09:36:32 -0000 1.93
+++ server_http.c 25 Jul 2015 21:00:52 -0000
@@ -632,6 +632,7 @@ server_reset_http(struct client *clt)
clt->clt_line = 0;
clt->clt_done = 0;
clt->clt_chunk = 0;
+ clt->clt_fcgi_http_header_state = FCGI_HTTP_HEADER_UNREAD;
free(clt->clt_remote_user);
clt->clt_remote_user = NULL;
clt->clt_bev->readcb = server_read_http;
--
I'm not entirely sure you are real.
