Early Review of draft-ietf-anima-brski-async-enroll-03
review-ietf-anima-brski-async-enroll-03-yangdoctors-early-rahman-2021-08-15-00

Major comments on this document:
- The YANG module has errors. Please validate it first e.g. by using
https://yangvalidator.com/yangvalidator or local tools. Also if you follow
guidelines @ https://datatracker.ietf.org/doc/html/rfc8407#section-3.2, you
will see errors present, if any, on datatracker. See below for the modified
YANG module which is valid, please check whether it is correct. Changes consist
of removing extra "; in author list, adding a revision date and replacing
augment "voucher-request" by augment voucher. - Include a tree diagram as per
https://datatracker.ietf.org/doc/html/rfc8407#section-3.4 - For the security
considerations, please add information as requested in
https://datatracker.ietf.org/doc/html/rfc8407#section-3.7

New assertion-type:
This is regarding issue #18, i.e.
https://github.com/anima-wg/anima-brski-async-enroll/issues/18, the need for
8366bis etc. Email threads:
https://mailarchive.ietf.org/arch/msg/netmod/Om6QOZL-bupgEblNLDL3S0PnaPY/ and
https://mailarchive.ietf.org/arch/msg/netmod/OrJYk01en82VVG-Ncud1YWxCBtg/

It was correctly pointed out that the enumeration for "leaf assertion" in
RFC8366 can not be augmented. If my understanding is correct, there is a
suggestion to do a IANA-maintained module for the assertion and republish a new
YANG module revision when a new assertion is added. However, I don't believe
the "assertion values" are actually IANA-maintained. So I don't think that
doing a IANA-maintained module is good in this case (disclaimer: I won't
pretend to be an expert on IANA-maintained modules). As comparision point, the
IANA BFD module at
https://datatracker.ietf.org/doc/html/draft-ietf-bfd-yang-17#section-2.12 is
for BFD registries maintained by IANA
(https://datatracker.ietf.org/doc/html/rfc5880#section-8).

Since 8366bis is being worked on, can the enum be changed to an identity? That
way, when a new assertion is needed, a new identity is added. Identities would
also enable to support "multiple inheritance" as was asked here:
https://mailarchive.ietf.org/arch/msg/netmod/dNGvcvckwuS_pBmkVg_Te8bedZs/. For
an example, see https://datatracker.ietf.org/doc/html/rfc7950#section-7.18.3

Other comments:
- rc:yang-data (RFC8040) is used. While this seems to be fine, if the
voucher-request-async-artifact template needs to be extended in the future, my
understanding is that it is not possible with yang-data. However, you could use
"structure" and (eventually) "augment-structure" from RFC8791 for this. -
Prefix "ivr" is used for ietf-voucher-request although RFC8995 has "vcr". While
this is valid, I am curious why. - Please take a look at
https://datatracker.ietf.org/doc/html/rfc8407#appendix-B for a module remplate.
e.g. data definition statements usualy go after grouping definitions.

Valid YANG module:

   module ietf-async-voucher-request {
     yang-version 1.1;

     namespace
       "urn:ietf:params:xml:ns:yang:ietf-async-voucher-request";
     prefix "constrained";

     import ietf-restconf {
       prefix rc;
       description
         "This import statement is only present to access
          the yang-data extension defined in RFC 8040.";
       reference "RFC 8040: RESTCONF Protocol";
     }

     import ietf-voucher-request {
       prefix ivr;
       description
         "This module defines the format for a voucher request,
             which is produced by a pledge as part of the RFC8995
             onboarding process.";
       reference
         "RFC 8995: Bootstrapping Remote Secure Key Infrastructure";
     }

     organization
      "IETF ANIMA Working Group";

     contact
      "WG Web:   <http://tools.ietf.org/wg/anima/>
       WG List:  <mailto:anima@ietf.org>
       Author:   Steffen Fries
                 <mailto:steffen.fries@siemens.com>
       Author:   Hendrik Brockhaus
                 <mailto: hendrik.brockhaus@siemens.com>
       Author:   Eliot Lear
                 <mailto: lear@cisco.com>
       Author:   Thomas Werner
                 <mailto: thomas-werner@siemens.com>";
     description
      "This module defines an extension of the RFC8995 voucher
       request to permit a registrar-agent to convey the adjacency
       relationship from the registrar-agent to the registrar.

       The key words 'MUST', 'MUST NOT', 'REQUIRED', 'SHALL',
       'SHALL NOT', 'SHOULD', 'SHOULD NOT', 'RECOMMENDED', 'MAY',
       and 'OPTIONAL' in the module text are to be interpreted as
       described in RFC 2119.";
     revision 2021-08-13 {
       description
        "Initial version";
       reference
        "RFC XXXX: Voucher Request for Asynchronous Enrollment";
     }
     rc:yang-data voucher-request-async-artifact {
       // YANG data template for a voucher.
       uses voucher-request-async-grouping;
     }
     // Grouping defined for future usage
     grouping voucher-request-async-grouping {
       description
         "Grouping to allow reuse/extensions in future work.";
       uses ivr:voucher-request-grouping {

         augment voucher {
           description "Base the constrained voucher-request upon the
             regular one";
           leaf agent-signed-data {
             type binary;
             description
               "The agent-signed-data field contains a JOSE [RFC7515]
                object provided by the Registrar-Agent to the Pledge.

                This artifact is signed by the Registrar-Agent
                and contains a copy of the pledge's serial-number.";
           }

           leaf agent-provided-proximity-registrar-cert {
             type binary;
             description
               "An X.509 v3 certificate structure, as specified by
                RFC 5280, Section 4, encoded using the ASN.1
                distinguished encoding rules (DER), as specified
                in ITU X.690.
                The first certificate in the registrar TLS server
                certificate_list sequence (the end-entity TLS
                certificate; see RFC 8446) presented by the
                registrar to the registrar-agent and provided to
                the pledge.
                This MUST be populated in a pledge's voucher-request
                when an agent-proximity assertion is requested.";
             reference
               "ITU X.690: Information Technology - ASN.1 encoding
                rules: Specification of Basic Encoding Rules (BER),
                Canonical Encoding Rules (CER) and Distinguished
                Encoding Rules (DER)
                RFC 5280: Internet X.509 Public Key Infrastructure
                Certificate and Certificate Revocation List (CRL)
                Profile
                RFC 8446: The Transport Layer Security (TLS)
                Protocol Version 1.3";
           }

           leaf agent-sign-cert {
             type binary;
             description
               "An X.509 v3 certificate structure, as specified by
                RFC 5280, Section 4, encoded using the ASN.1
                distinguished encoding rules (DER), as specified
                in ITU X.690.
                This certificate can be used by the pledge,
                the registrar, and the MASA to verify the signature
                of agent-signed-data. It is an optional component
                for the pledge-voucher request.
                This MUST be populated in a registrar's
                voucher-request when an agent-proximity assertion
                is requested.";
             reference
               "ITU X.690: Information Technology - ASN.1 encoding
                rules: Specification of Basic Encoding Rules (BER),
                Canonical Encoding Rules (CER) and Distinguished
                Encoding Rules (DER)
                RFC 5280: Internet X.509 Public Key Infrastructure
                Certificate and Certificate Revocation List (CRL)
                Profile";
           }
         }
       }
     }
   }