Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 54: Add policy check to ignore unnecessary rrsigs #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion common.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct file_info

extern struct file_info *file_info;

#define N_POLICY_CHECKS 10
#define N_POLICY_CHECKS 11

#define POLICY_SINGLE_NS 0
#define POLICY_CNAME_OTHER_DATA 1
Expand All @@ -51,6 +51,7 @@ extern struct file_info *file_info;
#define POLICY_DNSKEY 7
#define POLICY_TLSA_HOST 8
#define POLICY_KSK_EXISTS 9
#define POLICY_BIND_RRSIG_BUG_4305 10

#define MAX_TIMES_TO_CHECK 32

Expand Down
3 changes: 3 additions & 0 deletions main.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ void usage(char *err)
fprintf(stderr, "\t\t\trp-txt-exists\n");
fprintf(stderr, "\t\t\ttlsa-host\n");
fprintf(stderr, "\t\t\tksk-exists\n");
fprintf(stderr, "\t\t\tbind-rrsig-bug-4305\n");
fprintf(stderr, "\t\t\tall\n");

fprintf(stderr, "\t-n N\t\tuse N worker threads\n");
Expand Down Expand Up @@ -569,6 +570,8 @@ main(int argc, char **argv)
G.opt.policy_checks[POLICY_TLSA_HOST] = 1;
} else if (strcmp(optarg, "ksk-exists") == 0) {
G.opt.policy_checks[POLICY_KSK_EXISTS] = 1;
} else if (strcmp(optarg, "bind-rrsig-bug-4305") == 0) {
G.opt.policy_checks[POLICY_BIND_RRSIG_BUG_4305] = 1;
} else {
usage("unknown policy name");
}
Expand Down
59 changes: 32 additions & 27 deletions rrsig.c
Original file line number Diff line number Diff line change
Expand Up @@ -327,13 +327,6 @@ static void *rrsig_validate(struct rr *rrv)
return moan(rr->rr.file_name, rr->rr.line, "%s signature is too old", named_rr->name);
}
}
signed_set = find_rr_set_in_named_rr(named_rr, rr->type_covered);
if (!signed_set) {
return moan(rr->rr.file_name, rr->rr.line, "%s RRSIG exists for non-existing type %s", named_rr->name, rdtype2str(rr->type_covered));
}
if (signed_set->tail->ttl != rr->orig_ttl) {
return moan(rr->rr.file_name, rr->rr.line, "%s RRSIG's original TTL differs from corresponding record's", named_rr->name);
}
dnskey_rr_set = find_rr_set(T_DNSKEY, rr->signer);
if (!dnskey_rr_set) {
return moan(rr->rr.file_name, rr->rr.line, "%s RRSIG(%s): cannot find a signer key (%s)", named_rr->name, rdtype2str(rr->type_covered), rr->signer);
Expand All @@ -346,28 +339,40 @@ static void *rrsig_validate(struct rr *rrv)
}
key = (struct rr_dnskey *)key->rr.next;
}
if (candidate_keys == 0)
return moan(rr->rr.file_name, rr->rr.line, "%s RRSIG(%s): cannot find the right signer key (%s)", named_rr->name, rdtype2str(rr->type_covered), rr->signer);

candidates = getmem(sizeof(struct keys_to_verify) + (candidate_keys-1) * sizeof(struct verification_data));
candidates->next = all_keys_to_verify;
candidates->rr = rr;
candidates->signed_set = signed_set;
candidates->n_keys = candidate_keys;
all_keys_to_verify = candidates;
key = (struct rr_dnskey *)dnskey_rr_set->tail;
while (key) {
if (key->algorithm == rr->algorithm && key->key_tag == rr->key_tag) {
candidates->to_verify[i].key = key;
candidates->to_verify[i].rr = rr;
candidates->to_verify[i].ok = 0;
candidates->to_verify[i].openssl_error = 0;
candidates->to_verify[i].next = NULL;
i++;
signed_set = find_rr_set_in_named_rr(named_rr, rr->type_covered);
/*
* dnssec-signzone was not removing unnecessary rrsigs when
* re-signing a zone (bug 4305). Policy check ignores these.
*/
if (!G.opt.policy_checks[POLICY_BIND_RRSIG_BUG_4305] || signed_set) {
if (!signed_set) {
return moan(rr->rr.file_name, rr->rr.line, "%s RRSIG exists for non-existing type %s", named_rr->name, rdtype2str(rr->type_covered));
}
if (signed_set->tail->ttl != rr->orig_ttl) {
return moan(rr->rr.file_name, rr->rr.line, "%s RRSIG's original TTL differs from corresponding record's", named_rr->name);
}
if (candidate_keys == 0)
return moan(rr->rr.file_name, rr->rr.line, "%s RRSIG(%s): cannot find the right signer key (%s)", named_rr->name, rdtype2str(rr->type_covered), rr->signer);

candidates = getmem(sizeof(struct keys_to_verify) + (candidate_keys-1) * sizeof(struct verification_data));
candidates->next = all_keys_to_verify;
candidates->rr = rr;
candidates->signed_set = signed_set;
candidates->n_keys = candidate_keys;
all_keys_to_verify = candidates;
key = (struct rr_dnskey *)dnskey_rr_set->tail;
while (key) {
if (key->algorithm == rr->algorithm && key->key_tag == rr->key_tag) {
candidates->to_verify[i].key = key;
candidates->to_verify[i].rr = rr;
candidates->to_verify[i].ok = 0;
candidates->to_verify[i].openssl_error = 0;
candidates->to_verify[i].next = NULL;
i++;
}
key = (struct rr_dnskey *)key->rr.next;
}
key = (struct rr_dnskey *)key->rr.next;
}

return rr;
}

Expand Down
110 changes: 110 additions & 0 deletions t/issues/54-bind-rrsig-bug-4305/8.example.com.resign
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
; File written on Fri Mar 25 17:52:37 2016
; dnssec_signzone version 9.9.8-P4-RedHat-9.9.8_P4-2.el7.0
8.example.com. 86400 IN SOA ns1.example.com. hostmaster.example.com. (
2014012401 ; serial
14400 ; refresh (4 hours)
1800 ; retry (30 minutes)
1209600 ; expire (2 weeks)
3600 ; minimum (1 hour)
)
86400 RRSIG SOA 8 3 86400 (
20160424155237 20160325155237 40162 8.example.com.
WJ5UJNP6oxOf0ESrhAC1gHS844t3gdLbRNJ9
ky5AxDSotB5KvdT1FUAeyyydebH9u+n3cXV+
s5IPGC4k0uicIXiLZMS01Gl6mV/evPxG+9SD
HX31bom1MI9rs96PEWQRzTgSQECSPaJBpmyP
xSfQt0qsBD4fN7LHAgGBhilTjagBV8bodk8s
M+x9nQ6MaqVZTqERj8jns+dKayfU8jRoQRUG
6ye9HIdzYdl9XwaFJ33ekBuUpiG8eYDexG+U
3xbWP/tyKbg428QJNcpZTPYOI0EX5aZ2Q5vg
46QWWniLrlPgwUAkffJIic5GSDvPtaTQGzYd
PovIt6qgiDRJ8XeOSQ== )
172800 NS ns1.switch.ch.
172800 NS ns2.switch.ch.
172800 RRSIG NS 8 3 172800 (
20160424155237 20160325155237 40162 8.example.com.
XTmOThLXEi3O0ej+yiP+V4pbw5J/Z7fFv+EG
EOnf42sXqIxW4d686ZTG3jDyEgHBAFCGgvyE
XN5RqlOk42k6NWCja1SdGY7hQ0tJh3cv9gwv
tfmOk2650TJ64hG6QjreXg27tPWUWV56HxVY
P7E0w1I+lksdKGuO9I1SZUy984GYzwj1hWb8
TwImgUE77BDr0vdQQRbs1D5USAXUJMhMrFpt
waEKD7yJDXQ3fvtYtcEAjGiSedJ5YF8rNvMr
Pt3fVmqzrwc8+1tLysbD1CmnKSMFHpXXkwJZ
VVeBQt8jRsvFxLsxJm+8ZYeUHEHkCpIOKtc1
xmy+GdooTdttAx9G+A== )
86400 RRSIG A 8 3 86400 (
20160401093304 20160302093304 40162 8.example.com.
cALbnjysGSk3OH9fd73ejhBQrj752CI+qfxI
MUqaZr3TFHC3ZYVtTK0t7mC5M/FhDi1f8SVx
7VhcyJpMD7GedNJqQoWtoGQv6YdQix84Xt8v
c8OC+EIdGSFzeue5VxUW+Cj/hWgVk6AXodQA
zgBDKdG/5SMZRhxf2m/gtjGgledxogAOTOhW
CxXOptnZoq0bM7j8aM4UJl+MttGCTdfjHOO2
wqLzrzl6fMIENkFL7IugVqxKSMYLofZpHs9g
VvJf+PwecZXJlJohw6aiiE4+8rSeGG8HInuZ
hPu9pc39GFwI4k5LGSNYqVAuNg26OkmAMddP
nZbc4nXbMqSMD6CEaQ== )
3600 NSEC www.8.example.com. NS SOA RRSIG NSEC DNSKEY
3600 RRSIG NSEC 8 3 3600 (
20160424155237 20160325155237 40162 8.example.com.
FuGXOmCa/22zVS4QSaGU2ykW92nj/7CXVMqU
obMQDO+QV1Z/x1P3HzxFQVxN5fpdwEx0cXRh
2bP0ypfU9e5Hhuz86unp66ABbTyqPYFIQQAL
5zNo2p/eSx4zdSbXS15010AdD1dglvS6V+iB
dv0gB4Ijg4UZDsKl/+eww3rBBpJe3PrbXWFs
Yws3Oq0OyNd+v5aZ3gS2ZSYN9O58G40ocPm7
skmgnWoYB0rXwk8O/XpL3U8aND38uo5oycpO
WomQSc3KaYscO9kTRWVbacc4cNV/g3I94rOm
mC2l5p7DxSdQLRFlohzSHIOmdpIFKK+mytPi
kxJnNUNxv1StYwE7ig== )
86400 DNSKEY 257 3 8 (
AwEAAdQg6nL8wit3k8bXSOyK8we295xDSIcl
rEz6jD0MVog64xMovnf5445u3my0T9eBCpjL
MzYcsWqgm3kBxcCLwwL8yixZUjI1X3TCuX4t
q9tRvry6KxmxhlO7d1dlDeQH9iKqarYc/uK5
L3SsiOxVaBTzlR/D+Id9s36tW297YPrPD8tk
dgTX3UAKs1vcyiKPcbvLX6HuCGYNBBXN2/7z
pJTvXLSDX5vVFc0Gny4R0GLDnGLoLmW+b/Ev
oH6Fk2nmSrBZCE+3DmyaaP3KrhV7LXxZTSrf
V4uiQeJf1uMPPDSPW2vlZpvQe43b4LUgwjtu
cTtXctAW1OuzyAz5GEplET8=
) ; KSK; alg = RSASHA256; key id = 40162
86400 RRSIG DNSKEY 8 3 86400 (
20160424155237 20160325155237 40162 8.example.com.
kBXmP65mwAwTpyHX9gjNdbiSNCQ4rhhPI/Ef
tkV7P51MYTyBnlICcSwyYc/B2I02bvkg7Yo7
ecE9qwIXEdBWtUv0OKp73KfHGaPCeRU/JFCk
beYBURSGW8bBnF196GR0dbho+nAqU3LTG+NP
8gV6VapjDxmPbQi56UYOHEPQ/diaJkKHyizj
Bxiw7fjZ1VgeLtLihcLaYxZhAKXpc0hNaJza
vAcvw48D0gGQW+6rIxKLZrG+VqRlXJ4ulW4t
F+Tler9h09iIOqF6HAN8Ym2F8DmJ7d3o3JtY
VTNomeJ1Lm6GLD7tOwwYQ/3xh7xddd7Gt4uu
CeJiDaqiL70qXKGwOg== )
www.8.example.com. 86400 IN CNAME 8.example.com.
86400 RRSIG CNAME 8 4 86400 (
20160424155237 20160325155237 40162 8.example.com.
Vg7n1td1uc274ZVuNeYTrhPcKR2bjRhOx3BO
myGVpTK1uIX6awbZGs+FRzPCzheFgi765c4s
RBuPr3mqAW1YxQ6F8GANTDQEDM+i636c5NR8
uaT8h6hoUy/p8OKxwBdz6LA+sRkFxqIoKV++
Mk+UCTNy+QJDRQAJKaXyUPO5pyEKF4hBrdjg
1JUEmKod6XrfXD/4xEvtPCk1eWVhiIHEdGC5
SpaCObOHD/CFtWRbuBWLt2+I0JWtxK+K0PX0
rUOwLbF97Os/Ijw/WAlF4w4GO3TkLL+7+CxK
vpxP9ussiW3oB/Jbm8GDFxLpFoIgb8uVNtCi
ahLu+oC58Fre4xkNQg== )
3600 NSEC 8.example.com. CNAME RRSIG NSEC
3600 RRSIG NSEC 8 4 3600 (
20160424155237 20160325155237 40162 8.example.com.
kE0CYn9ezf6kc32Jtzp8XtWSRIAKpkwIKovS
rF/tD0Gi0/NIok9XEjemAYeTCQFQzNH4t35w
DcKcVsKEWGFVc3Am5z76JgAFndzEAh5u5MkL
7xzo421FFBzgbrID0/DKqt9lvcoc+xTceQOi
bc6BSyK0DkfbXrIi7K1S6jNaNouOXMGgiadY
kcKs1RhFu4fLV1lYX7l37IR/0Q0BKMOnqe3a
oYlXvZXVvIf71zg2UsLv2Bvlr+ypJ+IQnSEf
91o8Cu59iQJ0hv5ansQ3kEqN99O9NEQO0lpx
Oescyd+UZyimctfm4pCemTwYO6qnXkPpJ3DR
yLBgvsQmY6ermz9g9A== )
6 changes: 6 additions & 0 deletions t/test.pl
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,12 @@
@e = split /\n/, stderr;
like(shift @e, qr/signature is too old/, "multitime: signature is too old");

# issue 54: dnssec-signzone was not removing unnecessary rrsigs from zone
run('./validns', @threads, '-t1458924400', 't/issues/54-bind-rrsig-bug-4305/8.example.com.resign');
isnt(rc, 0, 'issue 51: unnecessary rrsigs detected without policy check');
run('./validns', @threads, '-t1458924400', '-p', 'bind-rrsig-bug-4305', 't/issues/54-bind-rrsig-bug-4305/8.example.com.resign');
is(rc, 0, 'issue 51: unnecessary rrsigs ignored with policy check enabled');

}

done_testing;