Re: [sfc] Benjamin Kaduk's No Objection on draft-ietf-sfc-nsh-integrity-08: (with COMMENT)

[Benjamin Kaduk <[e-mail]>]

Hi Med,

Thanks for taking the PR and making the further edits.

I don't see any changes in §7.5, though -- is it really the expectation
that any node that implements this specification at all is going to be
required to expect that all incoming NSH-bearing messages include the new
context header?  That doesn't really seem deployable.

-Ben

On Wed, Sep 15, 2021 at 12:00:17PM +0000, [e-mail] wrote:
> Hi Ben,
> 
> The suggestions look good to me. 
> 
> I made some additional edits + added a pointer to the cfrg I-D. The full diff can be seen at: https://tinyurl.com/nsh-integrity-latest 
> 
> Will proceed with the submission of this version tomorrow.
> 
> Many thanks for your effort. 
> 
> Cheers,
> Med
> 
> > -----Message d'origine-----
> > De : Benjamin Kaduk via Datatracker [mailto:[e-mail]]
> > Envoyé : mardi 14 septembre 2021 23:45
> > À : The IESG <[e-mail]>
> > Cc : [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail]
> > Objet : Benjamin Kaduk's No Objection on draft-ietf-sfc-nsh-integrity-
> > 08: (with COMMENT)
> > 
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-sfc-nsh-integrity-08: No Objection
> > 
> > When responding, please keep the subject line intact and reply to all
> > email addresses included in the To and CC lines. (Feel free to cut this
> > introductory paragraph, however.)
> > 
> > 
> > Please refer to https://www.ietf.org/iesg/statement/discuss-
> > criteria.html
> > for more information about DISCUSS and COMMENT positions.
> > 
> > 
> > The document, along with other ballot positions, can be found here:
> > https://datatracker.ietf.org/doc/draft-ietf-sfc-nsh-integrity/
> > 
> > 
> > 
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> > 
> > Thanks for the updates in the -07 and -08; I think we found a reasonable
> > way to address the issues that came up in my earlier review.
> > 
> > I turned most of my comments into a github PR against
> > https://github.com/boucadair/draft-ietf-sfc-nsh-integrity/ since that
> > repo was referenced in previous responses to comments.  The tip of the
> > branch I forked off seems to have corresponded to only the -07, though,
> > so I'm not sure if I should have gone somewhere else.  (I made a
> > separate first commit that just syncs down the -08 from the
> > datatracker.)
> > The PR is
> > https://github.com/boucadair/draft-ietf-sfc-nsh-integrity/pull/6 and the
> > actual "new" (non-08) changes are viewable via
> > https://github.com/boucadair/draft-ietf-sfc-nsh-
> > integrity/compare/99c6484fbe85af7179086ab243c88813e2b47b74..kaduk:kaduk-
> > review?expand=1
> > 
> > Section 4.4
> > 
> >    In order to accommodate deployments relying upon keying material per
> >    SFC/SFP and also the need to update keys after encrypting NSH data
> >    for a certain amount of time, this document uses key identifiers to
> >    unambiguously identify the appropriate keying material.  Doing so
> >    addresses the problem of synchronization of keying material.
> > 
> > I included a suggestion in my pull request, but I want to call out as a
> > potentially significant change that the key identifier should identify
> > not just the key material but also the algorithms that they key is to be
> > used with.
> > 
> > Section 5.1
> > 
> >    Nonce Length:  Carries the length of the Nonce.  If the Context
> >       Headers are only integrity protected, "Nonce Length" is set to
> >       zero (that is, no "Nonce" is included).  The "Nonce Length" can be
> >       set to zero depending on the encryption algorithm used to encrypt
> >       the Context Headers.
> > 
> > I included this in my PR, but want to call out that this last sentence
> > seems harmful.  It seems vastly preferrable to have "the nonce length
> > field is zero" indicate one specific thing, rather than having two
> > different possibilities for that situation, as this text appears to do.
> > I know that my earlier comments proposed a scenario where an AEAD might
> > validly encrypt with a zero-length nonce, but I don't think we should
> > support that at the cost of losing the clear signal that the encrypted
> > context headers are (not) present.
> > 
> > Section 7.3
> > 
> >    Using the secret key (ENC_KEY) and authenticated encryption
> >    algorithm, the NSH imposer encrypts the Context Headers (as set, for
> >    example, in Section 3) and inserts the resulting payload in the "MAC
> >    and Encrypted Metadata" Context Header (Section 5).  The entire
> >    Context Header carrying a sensitive metadata is encrypted (that is,
> >    including the MD Class, Type, Length, and associated metadata of each
> >    Context Header).
> > 
> > I included some text in my pull request, but want to call out as
> > important that this description does not specify what is to be used as
> > the "additional authenticated data" AEAD input.  (I assume the empty
> > string is intended, though other choices are valid.)
> > 
> > Section 7.5
> > 
> >    When an SFC data plane element conforms to this specification, it
> >    MUST ensure that a "MAC and Encrypted Metadata" Context Header is
> >    included in a received NSH packet.  [...]
> > 
> > This doesn't quite seem like the right condition -- this text sounds
> > like once you implement this context header you have to require that it
> > appears in all incoming packets, which breaks interoperability with
> > senders that don't produc this contxt header.
> > 
> > Section 9
> > 
> >    The secret key (K) must have an expiration time assigned as the
> >    latest point in time before which the key may be used for integrity
> >    protection of NSH data and encryption of Context Headers.  Prior to
> >    the expiration of the secret key, all participating NSH-aware nodes
> >    SHOULD have the control plane distribute a new key identifier and
> >    associated keying material so that when the secret key is expired,
> >    those nodes are prepared with the new secret key.  This allows the
> >    NSH imposer to switch to the new key identifier as soon as necessary.
> >    It is RECOMMENDED that the next key identifier and associated keying
> >    material be distributed by the control plane well prior to the secret
> >    key expiration time.
> > 
> > I note that draft-irtf-cfrg-aead-limits offers some guidance on how
> > often to rekey (though it gives data-based criteria, not time-based
> > ones), if that is potentially relevant.
> > 
> > 
> 
> 
> _________________________________________________________________________________________________________________________
> 
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
> 
> This message and its attachments may contain confidential or privileged information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
> Thank you.
>