Skip to content

Accept mnemonics in zonefile scanner for int_enum_str_decimal types - #704

Open
SebastiaanYN wants to merge 2 commits into
NLnetLabs:mainfrom
SebastiaanYN:zonefile-mnemonic
Open

Accept mnemonics in zonefile scanner for int_enum_str_decimal types#704
SebastiaanYN wants to merge 2 commits into
NLnetLabs:mainfrom
SebastiaanYN:zonefile-mnemonic

Conversation

@SebastiaanYN

Copy link
Copy Markdown
Contributor

scan_impl! for int_enum_str_decimal types only accepted decimal numbers via FromStr. Add from_mnemonic as a fallback so zone files can use RSASHA256 instead of 8 for SecurityAlgorithm.

`scan_impl!` for `int_enum_str_decimal` types only accepted decimal
numbers via `FromStr`. Add `from_mnemonic` as a fallback so zone files
can use `RSASHA256` instead of `8` for `SecurityAlgorithm`.
@withjannisNLnetLabs

Copy link
Copy Markdown
Contributor

Hello @SebastiaanYN

Thank you very much for this PR and all the others.

While this change would certainly resolve the issue for SecurityAlgorithm, it would also enable parsing from the mnemonic names for all other types, including those that do not allow it. Therefore, we refrain from merging it.

For example, NSEC3PARAM does not allow mnemonics in the presentation format, as specified by Section 4.3 in RFC 5155.

We have encountered this issue ourselves before and tried to solve it. Unfortunately, we were unable to do so without breaking SecurityAlgorithms interface and with a reasonable amount of work.

In case you are interested in our approach:

diff --git a/src/base/iana/secalg.rs b/src/base/iana/secalg.rs
index 82d222395..d19e30e15 100644
--- a/src/base/iana/secalg.rs
+++ b/src/base/iana/secalg.rs
@@ -116,5 +116,9 @@ int_enum! {
     (PRIVATEOID => 254, "PRIVATEOID")
 }
 
-int_enum_str_decimal!(SecurityAlgorithm, u8);
+int_enum_str_with_decimal!(
+    SecurityAlgorithm,
+    u8,
+    "unknown security algorithm"
+);

The above change resolves the parsing issue by modifying the implementation. However, this change affects the Display and serde::Serialize output, which is undesired. Solving this issue is certainly possible, but from what we currently know, it requires some larger code changes.

@SebastiaanYN

Copy link
Copy Markdown
Contributor Author

Thanks for taking a look at the PR. I did overlook that only SecurityAlgorithm can accept mnemonics

I pushed another commit that modifies the int_enum_str_decimal macro to accept an optional scan mode, which allows scan_impl to accept mnemonics for only SecurityAlgorithm, without affecting serde and Display

Hopefully this is a reasonable solution without doing larger refactoring

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.

2 participants