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

rec: extend the validity period of signatures by a number of seconds #7081

Merged
merged 3 commits into from Nov 6, 2018

Conversation

Projects
None yet
6 participants
@mind04
Contributor

mind04 commented Oct 18, 2018

Short description

There are some weird, maybe even broken, dnssec implementations out there. This pull is a workaround for the sign with inception is NOW problem. This is done by a loadbalancer vendor with a letter and a number in it's name. This is bad because only a very small time difference between signer and validator is enough for a domain to go bogus from time to time.
This pull is adding an option to extent the signature validity by a number of seconds at inception and expire. This will increase the resilience of the validation process and avoid a number of hard to explain bogus results for high profile domains. The default is to add 60 seconds to the signature lifetime. If you think you need to increase this value check your clock. Ntp is your friend...

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
``extend-signature-validity``
----------------------------------
- Integer

This comment has been minimized.

@rgacogne

rgacogne Oct 18, 2018

Member

A versionadded:: 4.2.0 (for example) would be nice

This comment has been minimized.

@mind04

mind04 Oct 18, 2018

Contributor

I was hoping to see a backport to stable label, since this issue is causing real problems atm.

This comment has been minimized.

@pieterlexis

pieterlexis Oct 26, 2018

Member

Then please add

.. versionadded:: 4.1.5

:)

@rgacogne rgacogne added this to the rec-4.2.0 milestone Oct 18, 2018

@mnordhoff

This comment has been minimized.

Contributor

mnordhoff commented Oct 26, 2018

If you do backport this to 4.1, I'd suggest having it off by default. (And turning it on by default in 4.2.)

This is weird and paranoid, but people could have compliance reasons for being unable to accept expired RRSIGs, and people are less careful about reading the changelogs of point releases.

(A CA using a different resolver got in trouble because this feature was on by default. But they just made a mistake -- it was always on by default, they didn't miss a change.)

@mind04

This comment has been minimized.

Contributor

mind04 commented Oct 26, 2018

We are talking about 60 seconds to compensate for clock skew, not hours or days (like in other software).

When your validation process is this critical and compliance is that important, you should always read change-logs. If you fail to do so, don't blame software for a more sane mode of operation. For normal users this small number of seconds will prevent some insanely hard to debug validation failures. And by doing so it will save many hours of debugging and speed up the dnssec adoption as a whole. Something CAs should love...

I'm against a switched off default in 4.1.x, because this will mean people will choose a shitty value for this option when they hit this problem. And this value stay in there config for years.

In my opinion this is one of those pulls where “The Needs of the Many Outweigh the Needs of the Few”.

Or maybe we should just release 4.2.0 ;)

@Habbie

This comment has been minimized.

Member

Habbie commented Oct 26, 2018

(A CA using a different resolver got in trouble because this feature was on by default. But they just made a mistake -- it was always on by default, they didn't miss a change.)

Do you have a reference for this? It sounds juicy.

@phonedph1

This comment has been minimized.

Contributor

phonedph1 commented Oct 26, 2018

Looks to be this https://bugzilla.mozilla.org/show_bug.cgi?id=1398427 for anyone else interested.

@Habbie Habbie modified the milestones: rec-4.2.0, rec-4.1.x Oct 31, 2018

@Habbie

This comment has been minimized.

Member

Habbie commented Oct 31, 2018

Milestone changed to 4.1.x to focus the discussion. Note that the PR has been updated to only affect inception.

mind04 added some commits Oct 31, 2018

@Habbie Habbie modified the milestones: rec-4.1.x, rec-4.2.0 Nov 1, 2018

@Habbie

This comment has been minimized.

Member

Habbie commented Nov 1, 2018

The 4.1.x version of this (with default '0') was merged. Changed milestone back to 4.2.0.

@Habbie Habbie merged commit 6bd6f07 into PowerDNS:master Nov 6, 2018

4 checks passed

LGTM analysis: C/C++ No alert changes
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mind04 mind04 deleted the mind04:offset branch Nov 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment