[master] 0c02c6758 Implment strict HTTP comparisons.
Poul-Henning Kamp
phk at FreeBSD.org
Sat Aug 28 09:12:05 UTC 2021
commit 0c02c67586821517aeb43a4ba2e07cb9f5c4a8c2
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Sat Aug 28 09:09:30 2021 +0000
Implment strict HTTP comparisons.
Most of this is taken from #3650 but instead of `str[n]casecmp`
it uses VCT functionality, in order to avoid LOCALE poisoning.
Closes #3650
diff --git a/bin/varnishd/builtin.vcl b/bin/varnishd/builtin.vcl
index 89c4c54fa..770081366 100644
--- a/bin/varnishd/builtin.vcl
+++ b/bin/varnishd/builtin.vcl
@@ -53,7 +53,7 @@ sub vcl_req_host {
}
if (!req.http.host &&
req.esi_level == 0 &&
- req.proto ~ "^(?i)HTTP/1.1") {
+ req.proto == "HTTP/1.1") {
# In HTTP/1.1, Host is required.
return (synth(400));
}
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 8b00b4fd4..b9e4022cb 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -646,6 +646,36 @@ extern const char H__Status[];
extern const char H__Proto[];
extern const char H__Reason[];
+// rfc7233,l,1207,1208
+#define http_tok_eq(s1, s2) (!vct_casecmp(s1, s2))
+#define http_tok_at(s1, s2, l) (!vct_caselencmp(s1, s2, l))
+#define http_ctok_at(s, cs) (!vct_caselencmp(s, cs, sizeof(cs) - 1))
+
+// rfc7230,l,1037,1038
+#define http_scheme_at(str, tok) http_ctok_at(str, #tok "://")
+
+// rfc7230,l,1144,1144
+// rfc7231,l,1156,1158
+#define http_method_eq(str, tok) (!strcmp(str, #tok))
+
+// rfc7230,l,1222,1222
+// rfc7230,l,2848,2848
+// rfc7231,l,3883,3885
+// rfc7234,l,1339,1340
+// rfc7234,l,1418,1419
+#define http_hdr_eq(s1, s2) http_tok_eq(s1, s2)
+#define http_hdr_at(s1, s2, l) http_tok_at(s1, s2, l)
+
+// rfc7230,l,1952,1952
+// rfc7231,l,604,604
+#define http_coding_eq(str, tok) http_tok_eq(str, #tok)
+
+// rfc7231,l,1864,1864
+#define http_expect_eq(str, tok) http_tok_eq(str, #tok)
+
+// rfc7233,l,1207,1208
+#define http_range_at(str, tok) http_ctok_at(str, #tok)
+
/* cache_main.c */
#define VXID(u) ((u) & VSL_IDENTMASK)
uint32_t VXID_Get(const struct worker *, uint32_t marker);
diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 466e096a4..7518b215b 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -40,6 +40,7 @@
#include "cache_filter.h"
#include "cache_vgz.h"
+#include "vct.h"
#include "vtim.h"
#include "cache_esi.h"
#include "vend.h"
@@ -860,7 +861,7 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody)
return;
if (http_GetHdr(req->resp, H_Content_Encoding, &p))
- i = !strcasecmp(p, "gzip");
+ i = http_coding_eq(p, gzip);
if (i)
i = ObjCheckFlag(req->wrk, req->objcore, OF_GZIPED);
diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c
index dd4843c1c..233dd4f7e 100644
--- a/bin/varnishd/cache/cache_http.c
+++ b/bin/varnishd/cache/cache_http.c
@@ -153,7 +153,7 @@ http_hdr_flags(const char *b, const char *e)
retval = &http_hdrflg[u];
if (retval->hdr == NULL)
return(NULL);
- if (strncasecmp(retval->hdr + 1, b, e - b))
+ if (!http_hdr_at(retval->hdr + 1, b, e - b))
return(NULL);
return(retval);
}
@@ -433,7 +433,7 @@ http_IsHdr(const txt *hh, hdr_t hdr)
assert(l == strlen(hdr + 1));
assert(hdr[l] == ':');
hdr++;
- return (!strncasecmp(hdr, hh->b, l));
+ return (http_hdr_at(hdr, hh->b, l));
}
/*--------------------------------------------------------------------*/
@@ -449,7 +449,7 @@ http_findhdr(const struct http *hp, unsigned l, const char *hdr)
continue;
if (hp->hd[u].b[l] != ':')
continue;
- if (strncasecmp(hdr, hp->hd[u].b, l))
+ if (!http_hdr_at(hdr, hp->hd[u].b, l))
continue;
return (u);
}
@@ -654,7 +654,7 @@ http_split(const char **src, const char *stop, const char *sep,
* Comparison rule for tokens:
* if target string starts with '"', we use memcmp() and expect closing
* double quote as well
- * otherwise we use strncasecmp()
+ * otherwise we use http_tok_at()
*
* On match we increment *bp past the token name.
*/
@@ -676,7 +676,7 @@ http_istoken(const char **bp, const char *e, const char *token)
*bp += fl + 2;
return (1);
}
- if (b + fl <= e && !strncasecmp(b, token, fl) &&
+ if (b + fl <= e && http_tok_at(b, token, fl) &&
(b + fl == e || !vct_istchar(b[fl]))) {
*bp += fl;
return (1);
@@ -876,9 +876,9 @@ http_DoConnection(struct http *hp, enum sess_close sc_close)
AN(h);
while (http_split(&h, NULL, ",", &b, &e)) {
u = pdiff(b, e);
- if (u == 5 && !strncasecmp(b, "close", u))
+ if (u == 5 && http_hdr_at(b, "close", u))
retval = sc_close;
- if (u == 10 && !strncasecmp(b, "keep-alive", u))
+ if (u == 10 && http_hdr_at(b, "keep-alive", u))
retval = SC_NULL;
/* Refuse removal of well-known-headers if they would pass. */
@@ -892,7 +892,7 @@ http_DoConnection(struct http *hp, enum sess_close sc_close)
continue;
if (hp->hd[v].b[u] != ':')
continue;
- if (strncasecmp(b, hp->hd[v].b, u))
+ if (!http_hdr_at(b, hp->hd[v].b, u))
continue;
hp->hdf[v] |= HDF_FILTER;
}
@@ -910,7 +910,7 @@ http_HdrIs(const struct http *hp, hdr_t hdr, const char *val)
if (!http_GetHdr(hp, hdr, &p))
return (0);
AN(p);
- if (!strcasecmp(p, val))
+ if (http_tok_eq(p, val))
return (1);
return (0);
}
@@ -989,10 +989,14 @@ http_ForceField(struct http *to, unsigned n, const char *t)
CHECK_OBJ_NOTNULL(to, HTTP_MAGIC);
assert(n < HTTP_HDR_FIRST);
+ assert(n == HTTP_HDR_METHOD || n == HTTP_HDR_PROTO);
AN(t);
+
+ /* NB: method names and protocol versions are case-sensitive. */
if (to->hd[n].b == NULL || strcmp(to->hd[n].b, t)) {
i = (HTTP_HDR_UNSET - HTTP_HDR_METHOD);
i += to->logtag;
+ /* XXX: this is a dead branch */
if (n >= HTTP_HDR_FIRST)
VSLbt(to->vsl, (enum VSL_tag_e)i, to->hd[n]);
http_SetH(to, n, t);
@@ -1204,6 +1208,7 @@ HTTP_GetHdrPack(struct worker *wrk, struct objcore *oc, hdr_t hdr)
AN(ptr);
ptr += 4; /* Skip nhd and status */
+ /* XXX: should we also have h2_hdr_eq() ? */
if (!strcmp(hdr, ":proto:"))
return (ptr);
ptr = strchr(ptr, '\0') + 1;
@@ -1216,7 +1221,7 @@ HTTP_GetHdrPack(struct worker *wrk, struct objcore *oc, hdr_t hdr)
}
HTTP_FOREACH_PACK(wrk, oc, ptr) {
- if (!strncasecmp(ptr, hdr, l)) {
+ if (http_hdr_at(ptr, hdr, l)) {
ptr += l;
while (vct_islws(*ptr))
ptr++;
diff --git a/bin/varnishd/cache/cache_range.c b/bin/varnishd/cache/cache_range.c
index be2340a19..7451c3812 100644
--- a/bin/varnishd/cache/cache_range.c
+++ b/bin/varnishd/cache/cache_range.c
@@ -108,7 +108,7 @@ vrg_dorange(struct req *req, const char *r, void **priv)
ssize_t low, high, has_low, has_high, t;
struct vrg_priv *vrg_priv;
- if (strncasecmp(r, "bytes=", 6))
+ if (!http_range_at(r, bytes=))
return ("Not Bytes");
r += 6;
@@ -214,6 +214,7 @@ vrg_ifrange(struct req *req)
return (0);
if ((e[0] == 'W' && e[1] == '/')) // rfc7232,l,547,548
return (0);
+ /* XXX: should we also have http_etag_cmp() ? */
return (strcmp(p, e) == 0); // rfc7232,l,548,548
}
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 75ea7b27c..9ad2aa28d 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -50,6 +50,7 @@
#include "storage/storage.h"
#include "common/heritage.h"
#include "vcl.h"
+#include "vct.h"
#include "vsha256.h"
#include "vtim.h"
@@ -93,7 +94,7 @@ cnt_transport(struct worker *wrk, struct req *req)
AN(req->req_body_status);
if (http_GetHdr(req->http, H_Expect, &p)) {
- if (strcasecmp(p, "100-continue")) {
+ if (!http_expect_eq(p, 100-continue)) {
req->doclose = SC_RX_JUNK;
(void)req->transport->minimal_response(req, 417);
wrk->stats->client_req_417++;
@@ -423,7 +424,7 @@ cnt_transmit(struct worker *wrk, struct req *req)
clval = http_GetContentLength(req->resp);
/* RFC 7230, 3.3.3 */
status = http_GetStatus(req->resp);
- head = !strcmp(req->http0->hd[HTTP_HDR_METHOD].b, "HEAD");
+ head = http_method_eq(req->http0->hd[HTTP_HDR_METHOD].b, HEAD);
if (boc != NULL || (req->objcore->flags & (OC_F_FAILED)))
req->resp_len = clval;
diff --git a/bin/varnishd/cache/cache_rfc2616.c b/bin/varnishd/cache/cache_rfc2616.c
index f00b37ebb..93bc30843 100644
--- a/bin/varnishd/cache/cache_rfc2616.c
+++ b/bin/varnishd/cache/cache_rfc2616.c
@@ -265,6 +265,7 @@ rfc2616_strong_compare(const char *p, const char *e)
if ((p[0] == 'W' && p[1] == '/') ||
(e[0] == 'W' && e[1] == '/'))
return (0);
+ /* XXX: should we also have http_etag_cmp() ? */
return (strcmp(p, e) == 0);
}
@@ -276,6 +277,7 @@ rfc2616_weak_compare(const char *p, const char *e)
p += 2;
if (e[0] == 'W' && e[1] == '/')
e += 2;
+ /* XXX: should we also have http_etag_cmp() ? */
return (strcmp(p, e) == 0);
}
diff --git a/bin/varnishd/cache/cache_vary.c b/bin/varnishd/cache/cache_vary.c
index 1e77730b3..88567674d 100644
--- a/bin/varnishd/cache/cache_vary.c
+++ b/bin/varnishd/cache/cache_vary.c
@@ -200,7 +200,7 @@ vry_cmp(const uint8_t *v1, const uint8_t *v2)
/* Different header */
retval = 1;
} else if (cache_param->http_gzip_support &&
- !strcasecmp(H_Accept_Encoding, (const char*) v1 + 2)) {
+ http_hdr_eq(H_Accept_Encoding, (const char*) v1 + 2)) {
/*
* If we do gzip processing, we do not vary on Accept-Encoding,
* because we want everybody to get the gzip'ed object, and
diff --git a/bin/varnishd/http1/cache_http1_fetch.c b/bin/varnishd/http1/cache_http1_fetch.c
index dfd0555b4..1861d9a12 100644
--- a/bin/varnishd/http1/cache_http1_fetch.c
+++ b/bin/varnishd/http1/cache_http1_fetch.c
@@ -240,7 +240,7 @@ V1F_FetchRespHdr(struct busyobj *bo)
* Figure out how the fetch is supposed to happen, before the
* headers are adultered by VCL
*/
- if (!strcasecmp(http_GetMethod(bo->bereq), "head")) {
+ if (http_method_eq(http_GetMethod(bo->bereq), HEAD)) {
/*
* A HEAD request can never have a body in the reply,
* no matter what the headers might say.
diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c
index ebf9ad1f1..eb19825cc 100644
--- a/bin/varnishd/http1/cache_http1_proto.c
+++ b/bin/varnishd/http1/cache_http1_proto.c
@@ -315,7 +315,7 @@ http1_body_status(const struct http *hp, struct http_conn *htc, int request)
if (cl == -2)
return (BS_ERROR);
if (http_GetHdr(hp, H_Transfer_Encoding, &b)) {
- if (strcasecmp(b, "chunked"))
+ if (!http_coding_eq(b, chunked))
return (BS_ERROR);
if (cl != -1) {
/*
@@ -375,10 +375,10 @@ HTTP1_DissectRequest(struct http_conn *htc, struct http *hp)
return (400);
/* RFC2616, section 5.2, point 1 */
- if (!strncasecmp(hp->hd[HTTP_HDR_URL].b, "http://", 7))
+ if (http_scheme_at(hp->hd[HTTP_HDR_URL].b, http))
b = hp->hd[HTTP_HDR_URL].b + 7;
else if (FEATURE(FEATURE_HTTPS_SCHEME) &&
- !strncasecmp(hp->hd[HTTP_HDR_URL].b, "https://", 8))
+ http_scheme_at(hp->hd[HTTP_HDR_URL].b, https))
b = hp->hd[HTTP_HDR_URL].b + 8;
if (b) {
e = strchr(b, '/');
@@ -399,13 +399,13 @@ HTTP1_DissectRequest(struct http_conn *htc, struct http *hp)
if (htc->body_status == BS_EOF) {
assert(hp->protover == 10);
/* RFC1945 8.3 p32 and D.1.1 p58 */
- if (!strcasecmp(p, "post") || !strcasecmp(p, "put"))
+ if (http_method_eq(p, POST) || http_method_eq(p, PUT))
return (400);
htc->body_status = BS_NONE;
}
/* HEAD with a body is a hard error */
- if (htc->body_status != BS_NONE && !strcasecmp(p, "head"))
+ if (htc->body_status != BS_NONE && http_method_eq(p, HEAD))
return (400);
return (retval);
diff --git a/bin/varnishtest/vtc_http.c b/bin/varnishtest/vtc_http.c
index d4265f4c4..5eb5d6306 100644
--- a/bin/varnishtest/vtc_http.c
+++ b/bin/varnishtest/vtc_http.c
@@ -1201,7 +1201,7 @@ cmd_http_txreq(CMD_ARGS)
} else if (!strcmp(*av, "-method") ||
!strcmp(*av, "-req")) {
req = av[1];
- hp->head_method = !strcasecmp(av[1], "HEAD") ;
+ hp->head_method = !strcmp(av[1], "HEAD") ;
av++;
} else if (!hp->sfd && !strcmp(*av, "-up")) {
up = av[1];
@@ -1216,7 +1216,7 @@ cmd_http_txreq(CMD_ARGS)
"Upgrade: h2c%s"
"HTTP2-Settings: %s%s", nl, nl, up, nl);
- nohost = strcasecmp(proto, "HTTP/1.1") != 0;
+ nohost = strcmp(proto, "HTTP/1.1") != 0;
av = http_tx_parse_args(av, vl, hp, NULL, nohost);
if (*av != NULL)
vtc_fatal(hp->vl, "Unknown http txreq spec: %s\n", *av);
More information about the varnish-commit
mailing list