Re: [Gen-art] Genart last call review of draft-ietf-cose-countersign-06

[Russ Housley]

Elwyn:

Thank you for the careful review.  My earlier message responded to the nits.  
This message responds to your more significant comment.

> Summary: Almost ready with one minor issue and several nits.  I do not
> understand how it is decided what the count of bstr fields is which is needed
> to determine if the other_fields mechanism is invoked.  Are all the standard
> fields included?  And could other_fields be included in an example please?
> Constructing an example would be helpful for both author and users I think.
> 
> Major issues:
> None
> 
> Minor issues:
> s3.3, description of 'other_fields':  I am confused as to which bstr's count
> towards the 'only two' condition.  All the fields after 'context' are encoded
> as bstr so are all these involved in the count?  Also I couldn't see an 
> example
> which appeared to showcase how 'other_fields' is used.  This might well have
> helped.

In the first paragraph of Section 3.3, the countersignature target structure is 
defined to be one of these: COSE_Signature, COSE_Sign1, COSE_Sign, COSE_Mac, 
COSE_Mac0, COSE_Encrypt, or COSE_Encrypt0.

Then, the other_fields description says:

   other_fields:  If there are only two bstr fields in the target
  structure, this field is omitted.  The field is an array of all
  bstr fields after the second.  As an example, this would be an
  array of one element for the COSE_Sign1 structure containing the
  signature value.

Would it help to say "countersignature target structure" instead of the 
abbreviated "target structure"?

Russ
___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

Re: [Gen-art] Genart last call review of draft-ietf-cose-countersign-06

[Russ Housley]

Elwyn:

This message responds to the Nits.  I'll respond to your comment about 
'other_fields' separately.

> Nits/editorial comments:
> Abstract:  Idnits is thoroughly confused by the document claiming to update 
> RFC
> 8152 when it is actually updating an RFC that hasn't been published yet.  
> Given
> that rfc8152bis (RFC-to-be 9052) hasn't been published yet, I wonder if a note
> about countersigning could be added into that document. But in any case  this
> document updates RFC 9052.

This decision was made a long time ago by the COSE WG.  Given that 
draft-ietf-cose-rfc8152bis-struct is in AUTH48, it seems like a bad idea to 
pull it back at this point.

> General: Use of " rather than ' for quoted strings. [s1.3 (6 places), s3.3 (2
> places)]

This seems to be Jim's style.  In soon-to-be-RFC-9052, the RFC Editor has 
changed the single quotes to double quotes.  I'll match that.

> s1.3: s/Byte is a synonym for octet./"Byte" is a synonym for "octet" in this
> document./

Agree.

> s1, para 3: I think this needs a little expansion:  "the inclusion of more of
> values in the countersignature".  At least s/of more of values/of the content
> of additional fields/  (if I understand correctly).

It is just trying to say what is different from the old algorithm.  I suggest:

   The new algorithm requires the inclusion of more values for the
   countersignature computation.

> s2, para 3: s/Details on version 2/Details of version 2/

Agree.

> s3, para 2: s/This is same structure/This is of the same structure/

I suggest a bigger change:

   Full countersignatures use the structure COSE_Countersignature,
   which has the same structure as COSE_Signature. Thus, full
   countersignatures can have protected and unprotected attributes,
   including chained countersignatures.

> s3.3, para 1: s/takes in countersignature/takes in the countersignature/

Agree.

> s5.2, last para: s/"(Deprecated by [[This Document]]"./"(Deprecated by [[This
> Document]])"./ [Missing closing bracket.]

Agree.

> s7.1: For the record there seems to be some lack of clarity as to whether 
> there
> are two or three different languages supported.  The 'Languages' line says 3
> languages but only mentions Java and C#.  Further on in 'Testing', Java, C# 
> and
> C are mentioned.  Since this section will be removed before publication it is
> not of great importance but would be good to get it right.  I couldn't see a C
> implementation in the cose-wg repository.

I see no reason to change this since it will be deleted.

Russ

___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

[Gen-art] Genart last call review of draft-ietf-cose-countersign-06

[Elwyn Davies via Datatracker]

Reviewer: Elwyn Davies
Review result: Ready with Issues

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

.

Document: draft-ietf-cose-countersign-06
Reviewer: Elwyn Davies
Review Date: 2022-07-22
IETF LC End Date: 2022-08-10
IESG Telechat date: Not scheduled for a telechat

Summary: Almost ready with one minor issue and several nits.  I do not
understand how it is decided what the count of bstr fields is which is needed
to determine if the other_fields mechanism is invoked.  Are all the standard
fields included?  And could other_fields be included in an example please?
Constructing an example would be helpful for both author and users I think.

Major issues:
None

Minor issues:
s3.3, description of 'other_fields':  I am confused as to which bstr's count
towards the 'only two' condition.  All the fields after 'context' are encoded
as bstr so are all these involved in the count?  Also I couldn't see an example
which appeared to showcase how 'other_fields' is used.  This might well have
helped.

Nits/editorial comments:
Abstract:  Idnits is thoroughly confused by the document claiming to update RFC
8152 when it is actually updating an RFC that hasn't been published yet.  Given
that rfc8152bis (RFC-to-be 9052) hasn't been published yet, I wonder if a note
about countersigning could be added into that document. But in any case  this
document updates RFC 9052.

General: Use of " rather than ' for quoted strings. [s1.3 (6 places), s3.3 (2
places)]

s1.3: s/Byte is a synonym for octet./"Byte" is a synonym for "octet" in this
document./

s1, para 3: I think this needs a little expansion:  "the inclusion of more of
values in the countersignature".  At least s/of more of values/of the content
of additional fields/  (if I understand correctly).

s2, para 3: s/Details on version 2/Details of version 2/

s3, para 2: s/This is same structure/This is of the same structure/

s3.3, para 1: s/takes in countersignature/takes in the countersignature/

s5.2, last para: s/"(Deprecated by [[This Document]]"./"(Deprecated by [[This
Document]])"./ [Missing closing bracket.]

s7.1: For the record there seems to be some lack of clarity as to whether there
are two or three different languages supported.  The 'Languages' line says 3
languages but only mentions Java and C#.  Further on in 'Testing', Java, C# and
C are mentioned.  Since this section will be removed before publication it is
not of great importance but would be good to get it right.  I couldn't see a C
implementation in the cose-wg repository.



___
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art