Skip to content

Keyset faketime#146

Merged
Philip-NLnetLabs merged 97 commits intomainfrom
keyset-faketime
Mar 2, 2026
Merged

Keyset faketime#146
Philip-NLnetLabs merged 97 commits intomainfrom
keyset-faketime

Conversation

@Philip-NLnetLabs
Copy link
Member

@Philip-NLnetLabs Philip-NLnetLabs commented Dec 16, 2025

Add fake-time to supported testing. This works fine for generating signatures with specific inception and expiration times. At the moment keyset in domain has no support for faketime. So key rolls cannot be tested with virtual time.

Philip-NLnetLabs and others added 30 commits April 17, 2025 13:37
Co-authored-by: Terts Diepraam <terts.diepraam@gmail.com>
- Adds KMIP server based key generation, signing and destruction, equivalent to the existing Ring/OpenSSL functionality.
- Adds new kmip subcommands for managing KMIP server configurations.
- Adds support for referring to KMIP keys by a new KMIP URL scheme. 
- Add a feature for the KMIP crypto backend just like the Ring and OpenSSL crypto backends.
- Adds support for storing sensitive credentials in files separate to the KMIP server configuration.
…port to the proposed channel at packages.nlnetlabs.nl.
* Restructure roll commands.

* Import public keys.

* Import a public/private key pair from files.

* Add a default TTL to config. Use that for DNSKEY/CDS/CDNSKEY/DS RRsets.

* Cargo.lock.

* Support for importing KMIP keys.

* Import public/private keys in decoupled state

* Add --private-key option to importing a public/private key pair from files.

* Add remove-key command.
@Philip-NLnetLabs Philip-NLnetLabs marked this pull request as ready for review February 27, 2026 15:40
let now = Timestamp::now().into_int();
let inception = (now - self.config.dnskey_inception_offset.as_secs() as u32).into();
let expiration = (now + self.config.dnskey_signature_lifetime.as_secs() as u32).into();
let now_u32 = Into::<Duration>::into(now).as_secs() as u32;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there is no other use of now in this function below the use of now_u32 here, why create a second now_u32 variable instead of replacing the existing now variable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid confusion. now_u32 is a truncated version of now.

let now = Timestamp::now().into_int();
let inception = (now - self.config.cds_inception_offset.as_secs() as u32).into();
let expiration = (now + self.config.cds_signature_lifetime.as_secs() as u32).into();
let now_u32 = Into::<Duration>::into(now).as_secs() as u32;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there is no other use of now in this function below the use of now_u32 here, why create a second now_u32 variable instead of replacing the existing now variable?

Co-authored-by: Ximon Eighteen <3304436+ximon18@users.noreply.github.com>
/// Collect all keys where present() returns true and sign the DNSKEY RRset
/// with all KSK and CSK (KSK state) where signer() returns true.
fn update_dnskey_rrset(&mut self, env: &impl Env, verbose: bool) -> Result<(), Error> {
let now = self.faketime_or_now();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In normal use (without fake time) this timestamp could differ to the "now" used elsewhere, but when using fake time those two timestamps will be the same I assume. Why is it okay/required for them to differ in normal use but not when using fake time?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fine if they are different. The question is whether it would improve code if now was stored and the code would use self.now. Hard to estimate. I'll leave it like this for now.

}
AutoReportActionsResult::Report(max_ttl)
}
/// Create CDS and CDNSKEY RRsets.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing line break?

Copy link
Member

@ximon18 ximon18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly fine, am confused by some changes regarding TTLs that appear unrelated to fake time, is that some weird artifact of the GH diff view?

@Philip-NLnetLabs
Copy link
Member Author

I think some of the weird changes are GH getting confused. One function got moved. I moved it back to its original location and the diffs are gone.

@Philip-NLnetLabs Philip-NLnetLabs merged commit ff82cf3 into main Mar 2, 2026
20 checks passed
@Philip-NLnetLabs Philip-NLnetLabs deleted the keyset-faketime branch March 2, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants