Re: [saag] AD review of draft-moskowitz-ipsecme-ipseckey-eddsa-02

[Robert Moskowitz <[e-mail]>]

On 10/30/22 08:36, Roman Danyliw wrote:
> Hi SAAG: as a reminder, I agreed to AD sponsor this document per https://mailarchive.ietf.org/arch/msg/saag/2JJo0lzaVlfFcBKJNfbXBLS5L44/.
>
> Hi Bob and Tero!
>
> I performed an AD review of draft-moskowitz-ipsecme-ipseckey-eddsa-02.  My feedback is as follows:
>
>
> ** Section 1.  Editorial.  Please introduce the protocol, provide pointers and expand names/use the formal names.
> OLD
>     The IPSECKEY IANA Registry specifically enumerates the various
>     Algorithm Types used.  This document adds support for the EdDSA
>     algorithm's Public Keys in IPSECKEY.
>
>     The IPSECKEY RR [RFC4025] defines the 'Algorithm Type' for specifying
>     the PK Algorithm.  Herein we are adding the EdDSA algorithm.
>
> NEW
> IPSECKEY [RFC4025) is a resource record (RR) for the Domain Name System (DNS) that is used to store public keys for use in IP security (IPsec) systems. The IPSECKEY RR relies on the IPSECKEY Algorithm Type Field registry [IANA-IPSECKEY] to enumerate the permissible formats for the public keys.
>
> This documents adds support for Edwards-Curve Digital Security Algorithm (EdDSA) public keys in the format defined in [RFC8080] to the IPSECKEY RR.

done

> ** Section 2.  Please remove this section as there aren't an RFC2119 key words used.

I guess I had it in as a place holder just in case.  Well time to..
Cut

> ** Section 3. Editorial.
>
> OLD
> The new EdDSA value uses [RFC8080] for the IPSECKEY encoding:
>
> NEW
> Use of an EdDSA public key encoded in the format specified in [RFC8080] in an IPSECKEY RR is indicated as follows:
>
> ** Section 4. Editorial
>
> OLD
>     This document requests IANA to clarify the text in the "Algorithm
>     Type Field" subregistry of the "IPSECKEY Resource Record Parameters"
>     [IANA-IPSECKEY] registry to explicitly state this is for "Public"
>     keys:
>
> NEW
> This document requests IANA to the "Description" field in existing entries of the "Algorithm Type Field" registry of the "IPSECKEY Resource Record Parameters" [IANA-IPSECKEY] to explicitly state that these keys are "public keys."

...to update the "Description"...
???

Anyway I think that is what you wanted.

> ** Section 4.  Do you want to also s/0, No key is present/0, No Public Key is present/?

If no public key is present, this RR will not be present!  By definition 
this is a RR for public keys.  No public keys, no RR so no defining a 
value of 0.

Or that is the way I see it.

> ** Section 4.  The proposed update to values 1 - 3 is too long and generates errors.

But it does produce a draft with the list with no indent.

> OLD
> Value  Description                                                      Reference
>
> 1    A DSA Public Key is present, in the format defined in [RFC2536]    [RFC4025]
> 2    A RSA Public Key is present, in the format defined in [RFC3110]    [RFC4025]
> 3    An ECDSA Public Key is present, in the format defined in [RFC6605] [RFC8005]
>
>
> NEW
> Value  Description
>
> 1    A DSA Public Key is present, in the format defined in [RFC2536]
> 2    A RSA Public Key is present, in the format defined in [RFC3110]
> 3    An ECDSA Public Key is present, in the format defined in [RFC6605]

I can remove the reference column?  It seems this is always called for.  
So either we accept the build errors that still result in a usable 
draft, or we make these entries two lines like:

       Value Description                               Reference

       1    A DSA Public Key is present,
                  in the format defined in [RFC2536]    [RFC4025]
2    A RSA Public Key is present,
                  in the format defined in [RFC3110]    [RFC4025]
3    An ECDSA Public Key is present,
                  in the format defined in [RFC6605]    [RFC8005]

Please advise.

> ** Section 4.  Typo. s/Futher/Further/

done

> ** Section 4.  Drop the following redundant text:
>
>     IPSECKEY:
>        This document defines the new IPSECKEY value TBD1 (suggested: 4)
>        (Section 3) in the "Algorithm Type Field" subregistry of the
>        "IPSECKEY Resource Record Parameters" registry.

How is this redundant?  Sec 3 (now 2) is the general description. Here 
it is instructions to IANA.

I had been "told" to use this format in ietf-drip-rid as it follows that 
used in other drafts/rfcs.

> ** Section 4.  s/[This]/[ThisRFC]/ and say somewhere what [ThisRFC] is.

Again, [This] seems to be the "accepted" method in drafts and then the 
RFC Editor replaces it with the actually assigned rfc number.  I can 
find for you a number of drafts that used [This] as the instruction to 
the RFC Editor.

How should I proceed?  Continue as past drafts have done: [This]? 
Introduce: [ThisRFC]?

> ** Section 6.  Please make [IANA-IPSECKEY] and [RFC8080] normative references.

Interesting.  I have not made IANA reference normative elsewhere, but 
will move it.  OK with 8080.

>    Please drop [RFC2119] and [RFC8174].

And 4025 which results in no Informative references.  OK.


What should I do about Security Considerations section?  I really don't 
have anything to say here.  Should not leave it TBD.  Drop it or say "No 
security considerations"?

> Regards,
> Roman

I have -03 ready pending any additions you recommend in response to this 
message.

Bob