Re: Gen-ART Review of draft-ietf-smime-sha2-03.txt

2008-02-29 Thread Spencer Dawkins
Hi, Sean,

This is much better than I feared. There were just too many places in -03 
where I was guessing what was intended, for me to say "ready for 
publication", or even "almost ready with nits".

I know you don't make the decisions about when drafts are last-called, but 
when you talk to your shepherd, you might suggest looking at diffs for the 
version posted for last call. that popped up a lot of the concerns i had 
(which were also concerns that denis had).

Best wishes with the draft.

Spencer


> Spencer,
>
> Thanks for taking the time to read the draft. Responses are inline.
>
> spt
>
>>-Original Message-
>>From: Spencer Dawkins [mailto:[EMAIL PROTECTED]
>>Sent: Friday, February 29, 2008 12:27 AM
>>To: General Area Review Team
>>Cc: Sean Turner; Blake Ramsdell; ietf@ietf.org; [EMAIL PROTECTED]
>>Subject: Gen-ART Review of draft-ietf-smime-sha2-03.txt
>>
>>I have been selected as the General Area Review Team (Gen-ART)
>>reviewer for this draft (for background on Gen-ART, please see
>>http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).
>>
>>Please resolve these comments along with any other Last Call
>>comments you may receive.
>
> Will do.
>
>>Document: draft-ietf-smime-sha2-03.txt
>>Reviewer: Spencer Dawkins
>>Review Date: 2008 02 28
>>IETF LC End Date: 2008-03-07
>>IESG Telechat date: (if known)
>>
>>Summary: This document isn't ready for publication as a
>>Proposed Standard - it's got enough cut-and-paste errors and
>>apparently-omitted text that I'd think twice about trying to
>>implement it. And if it has a note that says "normative
>>reference still in progress, should be brought in line with
>>normative reference before publishing as an RFC", I'm not sure
>>why it's being last called now (of course, that decision is
>>above my pay grade).
>
> Wrt the reference, that draft is being worked in PKIX. Hopefully, it will
> progress quickly - I'm hoping for this summer (or sooner) for it to 
> complete
> WG LC and IETF LC.  Also, the reference is for object identifiers all of
> which were assigned years ago and are stable.
>
>>Comments:
>>
>>Please include any nits listed in
>>http://www.ietf.org/mail-archive/web/ietf/current/msg50518.html
>> that I may have missed in this review, by reference :-(
>
> Will do.
>
>>When one of the last call comments is "There are obvious
>>errors (intentionnaly left by the editor in order to know how
>>many people read the document)", this does not inspire
>>confidence. I note there is no shepherd writeup in the tracker
>>yet. The "of for" typo below has been in every version since
>>00. Who else has read this draft?
>
> This was, I believe, his attempt at humor. See my response to Denis' 
> email.
>
>>Abstract
>>
>>   This document describes the conventions for using the message digest
>>   algorithms SHA-224, SHA-256, SHA-384, SHA-512, as defined in FIPS
>>
>>Nit - I'm not sure what passes for normal in security drafts,
>>but I'd expect to see SHA and FIPS expanded on first use in
>>the abstract, and in the introduction of the document. Ditto
>>for DSA, RSA, and ECDSA.
>
> I will expand the acronyms.
>
>>   180-3, with the Cryptographic Message Syntax (CMS). It also
>>describes
>>   the conventions for using these algorithms with CMS and the
>>DSA, RSA,
>>   and ECDSA signature algorithms.
>>
>>Conventions used in this document
>>
>>Nit - it is odd to see this section as part of the abstract...
>
> For some reason the tool picks up this section as part of the abstract. 
> It's
> got a heading title so I don't think it's in the abstract.
>
>>1. Introduction
>>
>>   This document specifies the algorithm identifiers and specifies
>>   parameters for the message digest algorithms SHA-224, SHA-256, SHA-
>>   384, and SHA-512 for use with the Cryptographic Message Syntax (CMS)
>>   [RFC3852].  The message digest algorithms are defined in and [SHS].
>>
>>Concern: "in and" seems to be missing something.
>
> Denis caught this too. Will fix by removing the "and".
>
>>   If an implementation chooses to support one of the algorithms
>>   discussed in this document, then the implementation MUST do so as
>>   described in this document.
>>
>>Concern: this MUST (and the parallel MUST in the next
>>paragraph) seem odd - do you need to say this?
>
> Hmmm ... I guess not.  I'll remove both sentences.
>
>>   Note that [RFC4231] specifies the conventions for use of for the
>>
>>Concern: "of for" seems to be missing something.
>
> I will remove "for use of" so the sentence will read: Note that [RFC4231]
> specifies the conventions for the message authentication code (MAC)
> algorithms 
>
>>   message authentication code (MAC) algorithms: HMAC with
>>SHA-224, HMAC
>>   with SHA-256, HMAC with SHA-384, and HMAC with SHA-512.
>>
>>2. Message Digest Algorithms
>>
>>   The following addresses the parameters field:
>>
>>Nit - this sentence isn't clear and isn't required. I'd strike it.
>
> Removed.
>
>>   There are two possible encodings for the SHA AlgorithmIdentifier
>>

RE: Gen-ART Review of draft-ietf-smime-sha2-03.txt

2008-03-01 Thread Turner, Sean P.
Spencer,

Thanks for taking the time to read the draft. Responses are inline.

spt 

>-Original Message-
>From: Spencer Dawkins [mailto:[EMAIL PROTECTED] 
>Sent: Friday, February 29, 2008 12:27 AM
>To: General Area Review Team
>Cc: Sean Turner; Blake Ramsdell; ietf@ietf.org; [EMAIL PROTECTED]
>Subject: Gen-ART Review of draft-ietf-smime-sha2-03.txt
>
>I have been selected as the General Area Review Team (Gen-ART) 
>reviewer for this draft (for background on Gen-ART, please see 
>http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).
>
>Please resolve these comments along with any other Last Call 
>comments you may receive.

Will do.

>Document: draft-ietf-smime-sha2-03.txt
>Reviewer: Spencer Dawkins
>Review Date: 2008 02 28
>IETF LC End Date: 2008-03-07
>IESG Telechat date: (if known)
>
>Summary: This document isn't ready for publication as a 
>Proposed Standard - it's got enough cut-and-paste errors and 
>apparently-omitted text that I'd think twice about trying to 
>implement it. And if it has a note that says "normative 
>reference still in progress, should be brought in line with 
>normative reference before publishing as an RFC", I'm not sure 
>why it's being last called now (of course, that decision is 
>above my pay grade).

Wrt the reference, that draft is being worked in PKIX. Hopefully, it will
progress quickly - I'm hoping for this summer (or sooner) for it to complete
WG LC and IETF LC.  Also, the reference is for object identifiers all of
which were assigned years ago and are stable.

>Comments:
>
>Please include any nits listed in
>http://www.ietf.org/mail-archive/web/ietf/current/msg50518.html
> that I may have missed in this review, by reference :-(

Will do.

>When one of the last call comments is "There are obvious 
>errors (intentionnaly left by the editor in order to know how 
>many people read the document)", this does not inspire 
>confidence. I note there is no shepherd writeup in the tracker 
>yet. The "of for" typo below has been in every version since 
>00. Who else has read this draft?

This was, I believe, his attempt at humor. See my response to Denis' email.

>Abstract
>
>   This document describes the conventions for using the message digest
>   algorithms SHA-224, SHA-256, SHA-384, SHA-512, as defined in FIPS
>
>Nit - I'm not sure what passes for normal in security drafts, 
>but I'd expect to see SHA and FIPS expanded on first use in 
>the abstract, and in the introduction of the document. Ditto 
>for DSA, RSA, and ECDSA.

I will expand the acronyms.

>   180-3, with the Cryptographic Message Syntax (CMS). It also 
>describes
>   the conventions for using these algorithms with CMS and the 
>DSA, RSA,
>   and ECDSA signature algorithms.
>
>Conventions used in this document
>
>Nit - it is odd to see this section as part of the abstract...

For some reason the tool picks up this section as part of the abstract. It's
got a heading title so I don't think it's in the abstract.

>1. Introduction
>
>   This document specifies the algorithm identifiers and specifies
>   parameters for the message digest algorithms SHA-224, SHA-256, SHA-
>   384, and SHA-512 for use with the Cryptographic Message Syntax (CMS)
>   [RFC3852].  The message digest algorithms are defined in and [SHS].
>
>Concern: "in and" seems to be missing something.

Denis caught this too. Will fix by removing the "and".

>   If an implementation chooses to support one of the algorithms
>   discussed in this document, then the implementation MUST do so as
>   described in this document.
>
>Concern: this MUST (and the parallel MUST in the next 
>paragraph) seem odd - do you need to say this?

Hmmm ... I guess not.  I'll remove both sentences.

>   Note that [RFC4231] specifies the conventions for use of for the
>
>Concern: "of for" seems to be missing something.

I will remove "for use of" so the sentence will read: Note that [RFC4231]
specifies the conventions for the message authentication code (MAC)
algorithms 

>   message authentication code (MAC) algorithms: HMAC with 
>SHA-224, HMAC
>   with SHA-256, HMAC with SHA-384, and HMAC with SHA-512.
>
>2. Message Digest Algorithms
>
>   The following addresses the parameters field:
>
>Nit - this sentence isn't clear and isn't required. I'd strike it.

Removed.

>   There are two possible encodings for the SHA AlgorithmIdentifier
>   parameters field.  The two alternatives arise from the fact 
>that when
>   the 1988 syntax for AlgorithmIdentifier was translated into the 1997
>   syntax, the OPTIONAL associated with the AlgorithmIdentifier
>   parameters got lost.  Later the OPTIONAL was recovered via a defect
>   report, but by then many people thought that algorithm parameters
>   were mandatory.  Because of this history some implementations encode
>   parameters as a NULL element and others omit them entirely.  The
>   correct encoding is to omit the parameters field; however,
>   implementations MUST also handle a SHA AlgorithmIdentifier 
>parameters
>