diff --git a/bin/varnishtest/tests/c00005.vtc b/bin/varnishtest/tests/c00005.vtc index 255f422324a..04679cfccd0 100644 --- a/bin/varnishtest/tests/c00005.vtc +++ b/bin/varnishtest/tests/c00005.vtc @@ -162,3 +162,108 @@ varnish v1 -errvcl {Non-zero bits in masked part} { if (client.ip ~ acl1) {} } } + +# this is both an OK test for pedantic and merge +varnish v1 -vcl { + import std; + + backend dummy None; + + acl acl1 +log +pedantic +merge { + # 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; + } + + 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; +} +} + +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: merged\E} + expect 1 = ReqHeader {^\Qip: 2.3.5.255\E$} + expect 0 = VCL_acl {^\QMATCH acl1 "2.3.0.0"/21 fixed: merged\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$} +} -start + +client c1 { + txreq + rxresp +} -run + +logexpect l1 -wait diff --git a/doc/sphinx/reference/vcl.rst b/doc/sphinx/reference/vcl.rst index 3513f0eb642..183475465b6 100644 --- a/doc/sphinx/reference/vcl.rst +++ b/doc/sphinx/reference/vcl.rst @@ -292,6 +292,27 @@ individually: However, if the name resolves to both IPv4 and IPv6 you will still get an error. +* `+merge` - Merge 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 merged (e.g. if both 1.2.3.0/25 and + 1.2.3.128/25 are added, they will be merged to 1.2.3.0/24). + + Skip and merge 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 merged 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: merged``. + + Negated ACL entries are never merged. + VCL objects ----------- diff --git a/include/tbl/vsl_tags.h b/include/tbl/vsl_tags.h index 2d3d3cf8656..ccbc1777512 100644 --- a/include/tbl/vsl_tags.h +++ b/include/tbl/vsl_tags.h @@ -289,15 +289,20 @@ 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 vcc_acl_pedantic parameter)\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 the ``vcc_acl_pedantic`` parameter) or ``merged`` for entries " + "which were the result of a merge operation (see the ``vcc_acl_merge`` " + "parameter.\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 5ec82105766..182133ed0d6 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_merge; 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(*aep); } -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; @@ -125,7 +133,7 @@ vcl_acl_cmp(const struct acl_e *ae1, const struct acl_e *ae2) if (ae2->mask < m) m = ae2->mask; for (; m >= 8; m -= 8) { - CMP(1, *p1, *p2); + CMP(ACL_GT, *p1, *p2); p1++; p2++; } @@ -135,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 @@ -160,20 +168,22 @@ vcl_acl_disjoint(const struct acl_e *ae1, const struct acl_e *ae2) if (ae2->mask < m) m = ae2->mask; for (; m >= 8; m -= 8) { - CMP(*p1, *p2); + CMPA(*p1, *p2); p1++; p2++; } if (m) { m = 0xff00 >> m; m &= 0xff; - CMP(*p1 & m, *p2 & m); + CMPA(*p1 & m, *p2 & m); } return (0); } VRBT_GENERATE_INSERT_COLOR(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) @@ -232,10 +242,66 @@ vcc_acl_chk(struct vcc *tl, const struct acl_e *ae, const int l, return (r); } +static void +vcl_acl_merge(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 = "merged"; + 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); @@ -248,7 +314,24 @@ vcc_acl_insert_entry(struct vcc *tl, struct acl_e **aenp) } return; } + + r = *aenp; *aenp = NULL; + + if (tl->acl->flag_merge == 0) + return; + + l = VRBT_PREV(acl_tree, &tl->acl->acl_tree, r); + if (l != NULL) { + vcl_acl_merge(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_merge(tl, &l, &r); } static void @@ -752,6 +835,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, "merge")) { + acl->flag_merge = sign; + vcc_NextToken(tl); } else if (vcc_IdIs(tl->t, "pedantic")) { acl->flag_pedantic = sign; vcc_NextToken(tl);