Last Call Review of draft-ietf-jmap-smime-08
review-ietf-jmap-smime-08-artart-lc-paine-2021-09-22-00

I'm the assigned reviewer for the ART area. Apologies for the delay in my
review - I was on holiday.

In summary, I think the draft could do with being clearer in a few places and
having a more logical ordering in Section 4. That said, most of my comments are
nits or common ART review issues, so I think the draft is essentially Ready.

Abstract
•       it could be worth including the RFC reference to the protocol being
extended here, or expanding the acronym "JMAP". S/MIME is probably well-known
enough by now though :)

Introduction
•       RFC8621 specifies "JMAP for email", JMAP is specified by RFC8620. So
should this be "JMAP for Email [RFC8621]" or "JMAP [RFC8620]" rather than "JMAP
[RFC8620]" as it currently is? •       The first sentence in the second
paragraph in the introduction could benefit from more specific references too.
As an example, you could rephrase like "when the multipart/signed media type is
used [RFC1847], the client is not required to download… and when the
application/pkcs7-mime media type is used [RFC8551 Sec. 3.2]" •       Missing
words: "assumes that *the/a* JMAP client trusts *the/a* JMAP server…" •      
I'm not sure that "this JMAP extension assumes" makes much sense – is it really
the extension that assumes this trust relationship? Perhaps more accurately,
the use of the extension implies the client trusts the server's configuration
and code… Or that when this extension is in use, the threat model must assume
that the client trusts the server's configuration and signature verification
code.

Section 2:
•       I see this document follows the conventions in Section 1.1 of RFC8621.
However, if there are particular meanings for words and asterisks later in the
document, it might be helpful to reference Section 1.1 at the point of use too,
so that the specific normative meaning isn't lost on those unfamiliar with the
editorial conventions (e.g. for "server-set" in Section 4).

Section 3:
•       It might be worth highlighting where exactly in RFC8620 one can find
details about the capabilities object, since it's a 90-page RFC and it's not a
section header. The capabilities object and Session object referenced are
defined in Section 2 of RFC8620. •       Is this a hidden requirement: "The
value of this property is an empty object"? For example, you could reword to
say "the value of the urn:ietf:params:jmap:smimeverify property MUST be an
empty object" or "must be set to" or similar.

Section 4:
•       In the second sentence of Section 4, "This document" refers to the
current document you're writing in, not RFC8621, if I understand correctly. Try
to make that clearer :) •       There's no sentence introducing the unknown,
signed, signed/verified and signed/failed messages, and they don't form a
bullet point list as you have above, so I found this difficult to parse. It
could be a good idea to include something like "The smimeStatus value can only
be one of the following four messages". This would help readability, and it
might be worth adding a colon after these messages or indenting more, because
it's not very clear, e.g. "unknown:   S/MIME message" read to me as "unknown
S/MIME message…" •       For unknown, you use the present tense "is" but for
the other smimeStatus responses, you use "was". I would suggest making it the
same for them all. •       You probably don't need to say "compliant with this
document", just say "JMAP servers SHOULD…" •       Structurally for Section 4,
it might be helpful to have sub-headings for the four different property values
and then group all the text about it in one place. And then another section for
the FilterCondition object too perhaps. •       I find it odd that the bullet
points are in a different order to the text that follows, e.g. the bullets are
smimeStatus, smimeErrors, smimeVerifiedAt, smimeStatusAtDelivery; the text
orders it smimeStatus, smimeStatusAtDelivery, smimeErrors, smimeVerifiedAt. •  
    The "string" values that are allowed for smimeStatus and smimeErrors do not
explicitly mention the allowable charsets (standard ART area guidance). In IETF
protocols, UTF-8 is preferred - see RFC 2277 (BCP 18), Sections 3.1 and 3.2.
Please reference this and make it explicit what characters are allowed. •      
Similarly, this draft has protocol elements that contain human-readable text,
so you need to specify language tags for those fields, or justify why language
tagging is not needed. There is text that alludes to this in smimeErrors,
saying "in the language specified in the Content-Language header field, if any"
– but there's no language stipulation for others, e.g. smimeStatus where it
says "Servers MAY return other values not defined below." For details, see RFC
2277 (BCP 18), Section 4.2. •       Suggest adding some words for readability:
*In one* example, the signing certificate might be expired and the message From
email address might not correspond to any of the email addresses in the signing
certificate. *In another example*, the certificate might be expired and the
JMAP server might be unable to retrieve a CRL for the certificate. •       The
smimeVerifiedAt value is described as "server-set". I was unsure if all of
these four are server-set, on reflection this could be because I missed the
particular definition of 'server-set' in RFC8621. •       Re: "UTCDate", please
make sure you define the details of its syntax (standard ART area guidance).
Even if you reference something like RFC 3339 (which is generally recommended
for IETF specifications), make sure that you document all the details about
various options, such as use of non-UTC time zones and fractional seconds. •   
   "(but see below about result caching)" – do you mean in the Security
Considerations or later in this paragraph? I'd suggest just removing this
bracket, it doesn't help with understanding nor does it contradict that the
value is calculated when the request was processed. •       Missing word(?): In
all cases "smimeVerifiedAt" is set to *the* time when "smimeStatus" and
"smimeErrors" were last updated. •       Should the IDs match? You have
f123u987 in the request but f123u457 in the response (even if they don't need
to match, I'm still twitching that "f123u457" isn't "f123u456"…)

Section 5.1
•       should this be "smimeverify" and not "smime", i.e. instead of "IANA is
requested to register the "smime" JMAP Capability as follows", it should be
"IANA is requested to register the "smimeverify" JMAP Capability as follows"

Section 6
•       I think you're missing a word - "server verification code" ought to be
"server *signature* verification code" as it is in the Introduction. •       I
suggest it's more of a trade-off than described currently, it's more subtle
than "the client trusts the server's configuration". The client trusts the
server's configuration and signature verification code *more than* the
inefficiency/work/bandwidth required to download the signature body part and
all signed body parts or to download and decode CMS. •       I'm not sure why
10 minutes specifically is recommended – it seems to be landed there without
rationale and without conveying why it's not longer/shorter.

I hope this review is helpful to the authors and, finally, thanks for your work
and I wish you all the best for this draft as it continues its IETF journey.