On Wed, Jan 4, 2017 at 2:21 PM, William A Rowe Jr <[email protected]> wrote:
> On Wed, Jan 4, 2017 at 6:57 AM, Yann Ylavic <[email protected]> wrote:
>> I'm using a (third-party/closed) module which replaces newlines in
>> header values (like base64 encoded PEMs) with obs-fold.
>> That's probably obsolete, but not forbidden per se...
>
> Actually, it is, c.f. 3.2.4 of RFC 7230
>
> [...] This specification deprecates such
> line folding except within the message/http media type
> (Section 8.3.1). A sender MUST NOT generate a message that includes
> line folding (i.e., that has any field-value that contains a match to
> the obs-fold rule) unless the message is intended for packaging
> within the message/http media type.
OK, pretty clear...
>
>> How about something like:
>
> -1. If we accept obs-fold from CGI, or internally within the
> headers_out, we must
> replace them with a single SP and conform to the spec on the wire.
This would work for me (on the proxy side), too.
The patch (attached) is a bit longer, but still reasonable IMHO.
WDYT?
Index: modules/http/http_filters.c
===================================================================
--- modules/http/http_filters.c (revision 1776920)
+++ modules/http/http_filters.c (working copy)
@@ -690,10 +690,11 @@ struct check_header_ctx {
};
/* check a single header, to be used with apr_table_do() */
-static int check_header(void *arg, const char *name, const char *val)
+static int check_header(struct check_header_ctx *ctx,
+ const char *name, const char **val)
{
- struct check_header_ctx *ctx = arg;
- const char *test;
+ const char *pos, *end;
+ char *dst = NULL;
if (name[0] == '\0') {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02428)
@@ -702,12 +703,12 @@ struct check_header_ctx {
}
if (ctx->strict) {
- test = ap_scan_http_token(name);
+ end = ap_scan_http_token(name);
}
else {
- test = ap_scan_vchar_obstext(name);
+ end = ap_scan_vchar_obstext(name);
}
- if (*test) {
+ if (*end) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02429)
"Response header name '%s' contains invalid "
"characters, aborting request",
@@ -715,17 +716,55 @@ struct check_header_ctx {
return 0;
}
- test = ap_scan_http_field_content(val);
- if (*test) {
- ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02430)
- "Response header '%s' value of '%s' contains invalid "
- "characters, aborting request",
- name, val);
- return 0;
+ for (pos = *val; *pos; pos = end) {
+ end = ap_scan_http_field_content(pos);
+ if (*end) {
+ if (end[0] != CR || end[1] != LF || (end[2] != ' ' &&
+ end[2] != '\t')) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02430)
+ "Response header '%s' value of '%s' contains "
+ "invalid characters, aborting request",
+ name, *val);
+ return 0;
+ }
+ if (!dst) {
+ *val = dst = apr_palloc(ctx->r->pool, strlen(*val) + 1);
+ }
+ }
+ if (dst) {
+ memcpy(dst, pos, end - pos);
+ dst += end - pos;
+ if (*end) {
+ /* turn folding into a single space */
+ *dst++ = ' ';
+ end += 3;
+ }
+ }
}
+ if (dst) {
+ *dst = '\0';
+ }
return 1;
}
+static int check_headers_table(apr_table_t *t, struct check_header_ctx *ctx)
+{
+ const apr_array_header_t *headers = apr_table_elts(t);
+ apr_table_entry_t *header;
+ int i;
+
+ for (i = 0; i < headers->nelts; ++i) {
+ header = APR_ARRAY_IDX(headers, i, apr_table_entry_t*);
+ if (!header->key) {
+ continue;
+ }
+ if (!check_header(ctx, header->key, (const char **)&header->val)) {
+ return 0;
+ }
+ }
+ return 1;
+}
+
/**
* Check headers for HTTP conformance
* @return 1 if ok, 0 if bad
@@ -738,8 +777,8 @@ static APR_INLINE int check_headers(request_rec *r
ctx.r = r;
ctx.strict = (conf->http_conformance != AP_HTTP_CONFORMANCE_UNSAFE);
- return apr_table_do(check_header, &ctx, r->headers_out, NULL) &&
- apr_table_do(check_header, &ctx, r->err_headers_out, NULL);
+ return check_headers_table(r->headers_out, &ctx) &&
+ check_headers_table(r->err_headers_out, &ctx);
}
static int check_headers_recursion(request_rec *r)
