Key2ds #2

tertsdiepraam · 2024-09-10T12:25:27Z

No description provided.

src/commands/mod.rs

src/commands/key2ds.rs

mozzieongit · 2024-10-29T13:24:17Z

src/commands/key2ds.rs

+            | SecAlg::ECDSAP256SHA256 => DigestAlg::SHA256,
+            SecAlg::ECDSAP384SHA384 => DigestAlg::SHA384,
+            SecAlg::ECC_GOST => DigestAlg::GOST,
+            _ => DigestAlg::SHA1,


Is using SHA-1 as the fallback a good idea? If new algorithms were to get introduced and we forget to update the above list, they would fallback to using SHA-1, right?

It's the current ldns behaviour: https://github.com/NLnetLabs/ldns/blob/b87b8e1d212b43f225891570ebc5045c6331dd8a/examples/ldns-key2ds.c#L48C1-L48C14

If we want to change it, let's take it to a separate PR, where we can discuss this in depth.

src/commands/mod.rs

src/commands/key2ds.rs

bal-e · 2024-10-31T12:26:40Z

src/commands/key2ds.rs

+                let sec_alg = sec_alg.to_int();
+                let filename =
+                    format!("K{owner}+{sec_alg:03}+{key_tag:05}.ds");
+                let mut out_file = File::create(&filename).map_err(|e| {


Would it make sense to use File::create_new() here?

Possibly, but ldns-key2ds doesn't care I think.

That is, if we have consensus that we want to break compat here, I'd be happy to change it.

Now that we have different argument parsing for ldns and dnst, and we can use different defaults, we can probably also introduce differences here. Maybe a simple running_as_ldns: bool (clap-hidden) member is enough or should it be an enum like MultiCallMode::{LDNS,DNST}/CompatMode::{None/DNST,LDNS} (or similar)?

tertsdiepraam · 2024-11-18T11:42:13Z

Little update: I changed the arguments for the dnst version so that we can use --force/-f for overwriting files and turn the ignoring sep option into the more descriptive --ignore-sep. I also added file opening/creation into Env, which can now be set up with a temp dir. Adds a bit of complexity, but not too much since we already have to use Env everywhere and the tests are quite nice if I do say so myself.

Cargo.toml

src/commands/key2ds.rs

mozzieongit

Apart from the minor comments, it looks good to me. Also, very thorough testing.

src/env/fake.rs

tertsdiepraam added 3 commits September 10, 2024 14:27

implement key2ds

19072fe

key2ds: better errror handling and --algorithm

119a6e5

key2ds: write filename of the output

c0d8ffa

tertsdiepraam force-pushed the key2ds branch from 4fd47ff to c0d8ffa Compare September 10, 2024 12:30

mozzieongit mentioned this pull request Sep 11, 2024

Reimplement (some) ldns sub commands #3

Open

12 tasks

ximon18 added the needs-review label Oct 16, 2024

ximon18 requested a review from a team October 16, 2024 09:59

key2ds: use zonefilefmt instead of display

498264a

mozzieongit requested changes Oct 29, 2024

View reviewed changes

key2ds: improve docs

56870bc

tertsdiepraam requested a review from mozzieongit October 30, 2024 10:34

mozzieongit reviewed Oct 30, 2024

View reviewed changes

src/commands/mod.rs Outdated Show resolved Hide resolved

key2ds: fix some more docstrings

e858fcc

mozzieongit approved these changes Oct 30, 2024

View reviewed changes

ximon18 removed the needs-review label Oct 30, 2024

ximon18 reviewed Oct 30, 2024

View reviewed changes

src/commands/key2ds.rs Outdated Show resolved Hide resolved

ximon18 reviewed Oct 30, 2024

View reviewed changes

src/commands/key2ds.rs Outdated Show resolved Hide resolved

ximon18 reviewed Oct 30, 2024

View reviewed changes

src/commands/key2ds.rs Show resolved Hide resolved

mozzieongit mentioned this pull request Oct 30, 2024

Notify #7

Merged

key2ds: split imports and improve error message

f9a5c73

bal-e reviewed Oct 31, 2024

View reviewed changes

tertsdiepraam added 6 commits November 12, 2024 16:43

Merge branch 'main' into key2ds

11abdcc

key2ds: separate ldns arg parsing

f675f8f

Merge branch 'main' into key2ds

e1fb799

WIP

82d6ea6

Merge branch 'main' into key2ds

d756c51

add some tests for key2ds

370dc3e

tertsdiepraam added 2 commits November 18, 2024 12:42

fmt

0cd1bdc

test a bit of the parsing of key2ds

8c7d188

tertsdiepraam added 3 commits November 18, 2024 14:55

fmt!

4cb13ce

even more key2ds parsing tests

a71d0f9

oops

94fe5fb

mozzieongit reviewed Nov 19, 2024

View reviewed changes

Cargo.toml Show resolved Hide resolved

src/commands/key2ds.rs Outdated Show resolved Hide resolved

mozzieongit reviewed Nov 19, 2024

View reviewed changes

src/env/fake.rs Outdated Show resolved Hide resolved

simplify env handing of tmp directory

0d8cc5d

tertsdiepraam force-pushed the key2ds branch from 6f570da to 0d8cc5d Compare November 19, 2024 12:38

tertsdiepraam requested a review from mozzieongit November 19, 2024 12:38

overwritten -> overwriting

761c8cc

mozzieongit approved these changes Nov 19, 2024

View reviewed changes

tertsdiepraam merged commit 511c8f1 into main Nov 19, 2024
29 checks passed

mozzieongit mentioned this pull request Nov 20, 2024

Change fallback for digest algorithm in dnst key2ds from SHA-1 to SHA-256? #32

Closed

Uh oh!

Key2ds #2

Key2ds #2

Uh oh!

Conversation

tertsdiepraam commented Sep 10, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tertsdiepraam commented Nov 18, 2024

Uh oh!

Uh oh!

Uh oh!

mozzieongit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants