diff --git a/bin/varnishtest/tests/c00005.vtc b/bin/varnishtest/tests/c00005.vtc index 4520088c7d6..818ba531822 100644 --- a/bin/varnishtest/tests/c00005.vtc +++ b/bin/varnishtest/tests/c00005.vtc @@ -162,3 +162,170 @@ varnish v1 -errvcl {Non-zero bits in masked part} { if (client.ip ~ acl1) {} } } + +# this is both an OK test for pedantic and fold +varnish v1 -vcl { + import std; + + backend dummy None; + + acl acl1 +log +pedantic +fold { + # bad notation (confusing) + "1.2.3.0"/24; + "1.2.3.64"/26; + + # all contained in 1.3.0.0/21 and 1.4.4.0/22 + "1.4.4.0"/22; + "1.3.4.0"/23; + "1.3.5.0"/26; + "1.3.6.0"/25; + "1.3.6.128"/25; + "1.3.0.0"/21; + "1.4.7"; + "1.4.6.0"/24; + + # right,left adjacent + "2.3.2.0"/23; + "2.3.0.0"/23; + # left,right adjacent + "2.3.4.0"/23; + "2.3.6.0"/23; + + # 12/14 folded, not 10 + "2.10.0.0"/15; + "2.12.0.0"/15; + "2.14.0.0"/15; + + # 226/227 folded, not 225 + "2.225.0.0"/16; + "2.226.0.0"/16; + "2.227.0.0"/16; + + # phks test case + "10.0.0.0"/23; + "10.0.2.0"/23; + + "10.1.0.0"/24; + "10.1.1.0"/24; + + "10.2.0.0"/25; + "10.2.0.128"/25; + } + + sub vcl_recv { + return (synth(200)); + } + sub t { + if (std.ip(req.http.ip) ~ acl1) { } + } + sub vcl_synth { + # variables would be nice, but not in core (yet?) + set req.http.ip = "1.2.3.0"; call t; + set req.http.ip = "1.2.3.63"; call t; + set req.http.ip = "1.2.3.64"; call t; + + set req.http.ip = "1.3.4.255"; call t; + set req.http.ip = "1.3.5.0"; call t; + set req.http.ip = "1.3.5.255"; call t; + set req.http.ip = "1.3.6.0"; call t; + set req.http.ip = "1.3.6.140"; call t; + set req.http.ip = "1.3.7.255"; call t; + + set req.http.ip = "1.4.5.255"; call t; + set req.http.ip = "1.4.6.64"; call t; + set req.http.ip = "1.4.7.64"; call t; + + set req.http.ip = "2.3.0.0"; call t; + set req.http.ip = "2.3.5.255"; call t; + + set req.http.ip = "2.2.255.255";call t; + set req.http.ip = "2.3.8.0"; call t; + + set req.http.ip = "2.9.1.1"; call t; + set req.http.ip = "2.10.1.1"; call t; + set req.http.ip = "2.12.0.0"; call t; + set req.http.ip = "2.15.255.255";call t; + set req.http.ip = "2.16.1.1"; call t; + + set req.http.ip = "2.224.1.1"; call t; + set req.http.ip = "2.225.1.1"; call t; + set req.http.ip = "2.226.1.1"; call t; + set req.http.ip = "2.227.1.1"; call t; + + set req.http.ip = "10.0.3.255"; call t; + set req.http.ip = "10.1.1.255"; call t; + set req.http.ip = "10.2.0.255"; call t; +} +} + +logexpect l1 -v v1 -g raw { + expect * 1009 ReqHeader {^\Qip: 1.2.3.0\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.2.3.0"/24\E$} + expect 1 = ReqHeader {^\Qip: 1.2.3.63\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.2.3.0"/24\E$} + expect 1 = ReqHeader {^\Qip: 1.2.3.64\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.2.3.0"/24\E$} + + expect 1 = ReqHeader {^\Qip: 1.3.4.255\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.3.0.0"/21\E$} + expect 1 = ReqHeader {^\Qip: 1.3.5.0\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.3.0.0"/21\E$} + expect 1 = ReqHeader {^\Qip: 1.3.5.255\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.3.0.0"/21\E$} + expect 1 = ReqHeader {^\Qip: 1.3.6.0\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.3.0.0"/21\E$} + expect 1 = ReqHeader {^\Qip: 1.3.6.140\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.3.0.0"/21\E$} + expect 1 = ReqHeader {^\Qip: 1.3.7.255\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.3.0.0"/21\E$} + + expect 1 = ReqHeader {^\Qip: 1.4.5.255\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.4.4.0"/22\E$} + expect 1 = ReqHeader {^\Qip: 1.4.6.64\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.4.4.0"/22\E$} + expect 1 = ReqHeader {^\Qip: 1.4.7.64\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "1.4.4.0"/22\E$} + + expect 1 = ReqHeader {^\Qip: 2.3.0.0\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "2.3.0.0"/21 fixed: folded\E} + expect 1 = ReqHeader {^\Qip: 2.3.5.255\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "2.3.0.0"/21 fixed: folded\E} + expect 1 = ReqHeader {^\Qip: 2.2.255.255\E$$} + expect 0 = VCL_acl {^\QNO_MATCH acl1\E$} + expect 1 = ReqHeader {^\Qip: 2.3.8.0\E$} + expect 0 = VCL_acl {^\QNO_MATCH acl1\E$} + + expect 1 = ReqHeader {^\Qip: 2.9.1.1\E$} + expect 0 = VCL_acl {^\QNO_MATCH acl1\E$} + expect 1 = ReqHeader {^\Qip: 2.10.1.1\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "2.10.0.0"/15\E$} + expect 1 = ReqHeader {^\Qip: 2.12.0.0\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "2.12.0.0"/14 fixed: folded\E} + expect 1 = ReqHeader {^\Qip: 2.15.255.255\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "2.12.0.0"/14 fixed: folded\E} + expect 1 = ReqHeader {^\Qip: 2.16.1.1\E$} + expect 0 = VCL_acl {^\QNO_MATCH acl1\E} + + expect 1 = ReqHeader {^\Qip: 2.224.1.1\E$} + expect 0 = VCL_acl {^\QNO_MATCH acl1\E$} + expect 1 = ReqHeader {^\Qip: 2.225.1.1\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "2.225.0.0"/16\E$} + expect 1 = ReqHeader {^\Qip: 2.226.1.1\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "2.226.0.0"/15 fixed: folded\E} + expect 1 = ReqHeader {^\Qip: 2.227.1.1\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "2.226.0.0"/15 fixed: folded\E} + + expect 1 = ReqHeader {^\Qip: 10.0.3.255\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "10.0.0.0"/22 fixed: folded\E} + expect 1 = ReqHeader {^\Qip: 10.1.1.255\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "10.1.0.0"/23 fixed: folded\E} + expect 1 = ReqHeader {^\Qip: 10.2.0.255\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "10.2.0.0"/24 fixed: folded\E} +} -start + +client c1 { + txreq + rxresp +} -run + +logexpect l1 -wait diff --git a/doc/sphinx/reference/vcl.rst b/doc/sphinx/reference/vcl.rst index 5143f0ff02a..9c5de4bcf75 100644 --- a/doc/sphinx/reference/vcl.rst +++ b/doc/sphinx/reference/vcl.rst @@ -299,6 +299,27 @@ individually: However, if the name resolves to both IPv4 and IPv6 you will still get an error. +* `+fold` - Fold ACL supernets and adjacent networks. + + With this parameter set to on, ACLs are optimized in that subnets + contained in other entries are skipped (e.g. if 1.2.3.0/24 is part + of the ACL, an entry for 1.2.3.128/25 will not be added) and + adjacent entries get folded (e.g. if both 1.2.3.0/25 and + 1.2.3.128/25 are added, they will be folded to 1.2.3.0/24). + + Skip and fold operations on VCL entries are output as warnings + during VCL compilation as entries from the VCL are processed in + order. + + Logging under the ``VCL_acl`` tag can change with this parameter + enabled: Matches on skipped subnet entries are now logged as matches + on the respective supernet entry. Matches on folded entries are + logged with a shorter netmask which might not be contained in the + original ACL as defined in VCL. Such log entries are marked by + ``fixed: folded``. + + Negated ACL entries are never folded. + VCL objects ----------- diff --git a/include/tbl/vsl_tags.h b/include/tbl/vsl_tags.h index 20a563daea6..ebcb3ac3606 100644 --- a/include/tbl/vsl_tags.h +++ b/include/tbl/vsl_tags.h @@ -264,15 +264,19 @@ SLTM(Fetch_Body, 0, "Body fetched from backend", SLTM(VCL_acl, 0, "VCL ACL check results", "ACLs with the `+log` flag emits this record with the result.\n\n" "The format is::\n\n" - "\t%s %s [%s [fixed: %s]]\n" - "\t| | | |\n" - "\t| | | +- Fixed entry (see acl +pedantic flag)\n" - "\t| | +------------ Matching entry (only for MATCH)\n" - "\t| +---------------- Name of the ACL\n" - "\t+-------------------- MATCH or NO_MATCH\n" - "\n" - "MATCH denotes an ACL match\n" - "NO_MATCH denotes that a checked ACL has not matched\n" + "\t%s [%s [%s [fixed: %s]]]\n" + "\t| | | |\n" + "\t| | | +- Fix info (see below)\n" + "\t| | +------------ Matching entry (only for MATCH)\n" + "\t| +---------------- Name of the ACL for MATCH or NO_MATCH\n" + "\t+-------------------- MATCH, NO_MATCH or NO_FAM\n" + "\n" + "* Fix info: either contains network/mask for non-canonical entries " + "(see acl +pedantic flag) or ``folded`` for entries " + "which were the result of a fold operation (see acl +fold flag).\n" + "* ``MATCH`` denotes an ACL match\n" + "* ``NO_MATCH`` denotes that a checked ACL has not matched\n" + "* ``NO_FAM`` denotes a missing address family and should not occur.\n" "\n" ) diff --git a/lib/libvcc/vcc_acl.c b/lib/libvcc/vcc_acl.c index d4b78ad8d11..3585c3c818e 100644 --- a/lib/libvcc/vcc_acl.c +++ b/lib/libvcc/vcc_acl.c @@ -53,6 +53,7 @@ struct acl { #define VCC_ACL_MAGIC 0xb9fb3cd0 int flag_log; + int flag_fold; int flag_pedantic; int flag_table; @@ -74,19 +75,26 @@ struct acl_e { struct token *t_mask; }; +enum acl_cmp_e { + ACL_EQ = 0, + ACL_LT = -1, // a < b + ACL_GT = 1, // b > a + ACL_CONTAINED = -2, // b contains a + ACL_CONTAINS = 2, // a contains b + ACL_LEFT = -3, // a + 1 == b + ACL_RIGHT = 3 // a == b + 1 +}; + +static void vcc_acl_insert_entry(struct vcc *, struct acl_e **); + /* - * Compare two acl rules for ordering - * returns: - * 0 same - * -1/1 strictly less/greater - * -2/2 b contains a / a contains b - * -3/3 a left of b / b left of a + * Compare two acl rules for relation */ #define CMP(n, a, b) \ do { \ if ((a) < (b)) \ - return (-n); \ + return (enum acl_cmp_e)(-n); \ else if ((b) < (a)) \ return (n); \ } while (0) @@ -94,9 +102,9 @@ struct acl_e { #define CMPA(a, b) \ do { \ if (((a) | 1) == (b)) \ - return (-3); \ + return (ACL_LEFT); \ else if (((b) | 1) == (a)) \ - return (3); \ + return (ACL_RIGHT); \ } while (0) static void @@ -109,7 +117,7 @@ vcl_acl_free(struct acl_e **aep) FREE_OBJ(a); } -static int +static enum acl_cmp_e vcl_acl_cmp(const struct acl_e *ae1, const struct acl_e *ae2) { const unsigned char *p1, *p2; @@ -123,7 +131,9 @@ vcl_acl_cmp(const struct acl_e *ae1, const struct acl_e *ae2) p2 = ae2->data; m = vmin_t(unsigned, ae1->mask, ae2->mask); for (; m >= 8; m -= 8) { - CMP(1, *p1, *p2); + if (m == 8 && ae1->mask == ae2->mask) + CMPA(*p1, *p2); + CMP(ACL_GT, *p1, *p2); p1++; p2++; } @@ -133,14 +143,14 @@ vcl_acl_cmp(const struct acl_e *ae1, const struct acl_e *ae2) a2 = *p2 >> (8 - m); if (ae1->mask == ae2->mask) CMPA(a1, a2); - CMP(1, a1, a2); + CMP(ACL_GT, a1, a2); } else if (ae1->mask == ae2->mask) { CMPA(*p1, *p2); } /* Long mask is less than short mask */ - CMP(2, ae2->mask, ae1->mask); + CMP(ACL_CONTAINS, ae2->mask, ae1->mask); - return (0); + return (ACL_EQ); } static int @@ -156,14 +166,14 @@ vcl_acl_disjoint(const struct acl_e *ae1, const struct acl_e *ae2) p2 = ae2->data; m = vmin_t(unsigned, ae1->mask, ae2->mask); for (; m >= 8; m -= 8) { - CMP(1, *p1, *p2); + CMP(ACL_GT, *p1, *p2); p1++; p2++; } if (m) { m = 0xff00 >> m; m &= 0xff; - CMP(1, *p1 & m, *p2 & m); + CMP(ACL_GT, *p1 & m, *p2 & m); } return (0); } @@ -171,6 +181,8 @@ vcl_acl_disjoint(const struct acl_e *ae1, const struct acl_e *ae2) VRBT_GENERATE_INSERT_COLOR(acl_tree, acl_e, branch, static) VRBT_GENERATE_INSERT_FINISH(acl_tree, acl_e, branch, static) VRBT_GENERATE_INSERT(acl_tree, acl_e, branch, vcl_acl_cmp, static) +VRBT_GENERATE_REMOVE_COLOR(acl_tree, acl_e, branch, static) +VRBT_GENERATE_REMOVE(acl_tree, acl_e, branch, static) VRBT_GENERATE_MINMAX(acl_tree, acl_e, branch, static) VRBT_GENERATE_NEXT(acl_tree, acl_e, branch, static) VRBT_GENERATE_PREV(acl_tree, acl_e, branch, static) @@ -229,10 +241,66 @@ vcc_acl_chk(struct vcc *tl, const struct acl_e *ae, const int l, return (r); } +static void +vcl_acl_fold(struct vcc *tl, struct acl_e **l, struct acl_e **r) +{ + enum acl_cmp_e cmp; + + AN(l); + AN(r); + CHECK_OBJ_NOTNULL(*l, VCC_ACL_E_MAGIC); + CHECK_OBJ_NOTNULL(*r, VCC_ACL_E_MAGIC); + + if ((*l)->not || (*r)->not) + return; + + cmp = vcl_acl_cmp(*l, *r); + + assert(cmp < 0); + if (cmp == ACL_LT) + return; + + do { + switch (cmp) { + case ACL_CONTAINED: + VSB_cat(tl->sb, "ACL entry:\n"); + vcc_ErrWhere(tl, (*r)->t_addr); + VSB_cat(tl->sb, "supersedes / removes:\n"); + vcc_ErrWhere(tl, (*l)->t_addr); + vcc_Warn(tl); + VRBT_REMOVE(acl_tree, &tl->acl->acl_tree, *l); + FREE_OBJ(*l); + *l = VRBT_PREV(acl_tree, &tl->acl->acl_tree, *r); + break; + case ACL_LEFT: + (*l)->mask--; + (*l)->fixed = "folded"; + VSB_cat(tl->sb, "ACL entry:\n"); + vcc_ErrWhere(tl, (*l)->t_addr); + VSB_cat(tl->sb, "left of:\n"); + vcc_ErrWhere(tl, (*r)->t_addr); + VSB_printf(tl->sb, "removing the latter and expanding " + "mask of the former by one to /%d\n", + (*l)->mask - 8); + vcc_Warn(tl); + VRBT_REMOVE(acl_tree, &tl->acl->acl_tree, *r); + FREE_OBJ(*r); + VRBT_REMOVE(acl_tree, &tl->acl->acl_tree, *l); + vcc_acl_insert_entry(tl, l); + return; + default: + INCOMPL(); + } + if (*l == NULL || *r == NULL) + break; + cmp = vcl_acl_cmp(*l, *r); + } while (cmp != ACL_LT); +} + static void vcc_acl_insert_entry(struct vcc *tl, struct acl_e **aenp) { - struct acl_e *ae2; + struct acl_e *ae2, *l, *r; CHECK_OBJ_NOTNULL(*aenp, VCC_ACL_E_MAGIC); ae2 = VRBT_INSERT(acl_tree, &tl->acl->acl_tree, *aenp); @@ -245,7 +313,24 @@ vcc_acl_insert_entry(struct vcc *tl, struct acl_e **aenp) } return; } + + r = *aenp; *aenp = NULL; + + if (tl->acl->flag_fold == 0) + return; + + l = VRBT_PREV(acl_tree, &tl->acl->acl_tree, r); + if (l != NULL) { + vcl_acl_fold(tl, &l, &r); + } + if (r == NULL) + return; + l = r; + r = VRBT_NEXT(acl_tree, &tl->acl->acl_tree, l); + if (r == NULL) + return; + vcl_acl_fold(tl, &l, &r); } static void @@ -749,6 +834,9 @@ vcc_ParseAcl(struct vcc *tl) if (vcc_IdIs(tl->t, "log")) { acl->flag_log = sign; vcc_NextToken(tl); + } else if (vcc_IdIs(tl->t, "fold")) { + acl->flag_fold = sign; + vcc_NextToken(tl); } else if (vcc_IdIs(tl->t, "pedantic")) { acl->flag_pedantic = sign; vcc_NextToken(tl);