Subject: Re: [Last-Call] Opsdir last call review of draft-hodges-webauthn-registries-05

From: Benjamin Kaduk <[email-masked]>
Date: Sun, 3 May 2020 15:02:20 -0700

Hi Sarah,

On Tue, Apr 28, 2020 at 10:41:39AM -0700, Sarah Banks via Datatracker wrote:
> Reviewer: Sarah Banks
> Review result: Has Issues
> 
> Hello,
>      I too share the concerns the GENART reviewer does. In addition, a few
>      things:
> 
> 1. As a personal nit, I'm slightly annoyed as a reader that the draft defines
> the registries, but another doc has the default values. Just ann FYI, and I
> realize this is a style choice.

I think it's important to note that the initial registry contents are to
list a non-IETF organization as the change controller.  To me, it seems
more appropriate to have that organization produce a document to effectuate
the registrations they desire, rather than having the IETF publish a
document that tries to allocate things on their behalf.

> 2. In section 2.1, it states: "Each attestation
> statement format identifier added to this registry MUST be unique amongst the
> set of registered attestation statement format identifiers.", and that they are
> case sensitive. Did you really intend to allow a conceptual overload where a
> string of "string" and "STRING" would be allowed?

Yes.  See also the note about "may not match [...] in a case-insensitive
manner unless the DEs determine that there is a compelling reason to allow
an exception".

> 3. In a few spots it's
> written (see 2.2.2 for example): " As noted in Section 2.2.1, WebAuthn
> extension identifiers are registered using the Specification Required policy,
> implying review  and approval by a designated expert.". Implied doesn't seem to
> be normative. Given the follow on text here, did you explictly NOT want to make
> this a normative requirement?

It is normative, though -- we use the RFC 8126 policy of "Specification
Required", which by definition includes expert review.  Do you have an
alternate phrasing than "implying" to use for this situation?

Thanks for the review,

Ben

---

From: Mike Jones <[email-masked]>
Date: Wed, 13 May 2020 23:49:56 +0000

Thanks for the review, Sarah.  My replies are inline below, prefixed by "Mike>".

-----Original Message-----
From: Sarah Banks via Datatracker <[email-masked]> 
Sent: Tuesday, April 28, 2020 10:42 AM
To: [email-masked]
Cc: [email-masked]; [email-masked]
Subject: Opsdir last call review of draft-hodges-webauthn-registries-05

Reviewer: Sarah Banks

Hello,

Mike> We've decided to delete the language about defining additional fields.  (This language was copied from RFC 8288 but we decided that it wasn't needed for the purposes of this specification.)

1. As a personal nit, I'm slightly annoyed as a reader that the draft defines the registries, but another doc has the default values. Just ann FYI, and I realize this is a style choice.

Mike> The WebAuthn specification from which the initial values come https://www.w3.org/TR/2019/REC-webauthn-1-20190304/ already defined these values.  As an editorial choice, we decided that it would be less error prone to reference them there, than to repeat them here.

2. In section 2.1, it states: "Each attestation statement format identifier added to this registry MUST be unique amongst the set of registered attestation statement format identifiers.", and that they are case sensitive. Did you really intend to allow a conceptual overload where a string of "string" and "STRING" would be allowed?

Mike> Fair question.  You'll also notice that the instructions to the designated experts include this text: "Extension identifiers may not match other registered names in a case-insensitive manner unless the Designated Experts determine that there is a compelling reason to allow an exception."  This is the same instruction text used by JOSE and OAuth RFCs, such as RFC 7515 and RFC 7519.

3. In a few spots it's written (see 2.2.2 for example): " As noted in Section 2.2.1, WebAuthn extension identifiers are registered using the Specification Required policy, implying review  and approval by a designated expert.". Implied doesn't seem to be normative. Given the follow on text here, did you explictly NOT want to make this a normative requirement?

Mike>  I've deleted the "implied" text where it occurred.  RFC 8126 already requires expert review when the Specification Required policy is used, so we didn't need to repeat it here.

Mike> You can see proposed updated source for -06 at https://github.com/w3c/webauthn/pull/1415.

Thanks,
Sarah

				Thanks again,
				-- Mike