[lamps] AD Review of draft-ietf-lamps-rfc5751-bis-06.txt

[Eric Rescorla <[e-mail]>]

See https://mozphab-ietf.devsvcdev.mozaws.net/D8 for a more full-featured
version of this review.


This document appears to be in generally good shape. I have marked a number
of issues below.

Please spell check this document. I found a rather large number of typos.

*INLINE COMMENTS*
View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-265>
draft-ietf-lamps-rfc5751-bis.txt:230
Certificate: A type that binds an entity's name to a public key
with a digital signature.

Nit: I'm not sure "type" is the cleanest word here. I suppose this comes
from ASN.1 but it's really more like a message or a credential

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-266>
draft-ietf-lamps-rfc5751-bis.txt:263
Data Integrity Service: A security service that protects againist
unauthorized changes to data by ensuring that

Typo: against

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-267>
draft-ietf-lamps-rfc5751-bis.txt:267
Data Confidentiality: The property that data is not discolsed to
system entities unless they have been authorized

Typo: disclosed

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-268>
draft-ietf-lamps-rfc5751-bis.txt:272
Data Origination: The corroboration that the source of the data
received is as claimed. [RFC4949].

This phrase does not seem to appear in the document, and this is an odd
definition.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-269>
draft-ietf-lamps-rfc5751-bis.txt:305
S/MIME version 4.0 agents ought to attempt to have the greatest
interoperability possible with agents for prior versions of S/MIME.

Should "ought to" be SHOULD

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-270>
draft-ietf-lamps-rfc5751-bis.txt:390
Section 3.4.3.2: Replace micalg parameter for SHA-1 with sha-1.

This might be easier to read with quotes.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-271>
draft-ietf-lamps-rfc5751-bis.txt:466
- MUST- support RSA with SHA-256.

I think it would be clearest when you have RSA PKCS#1 1.5 next to PSS if
you clearly stated 1.5. I think it's fine elsewhere.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-275>
draft-ietf-lamps-rfc5751-bis.txt:482
- MUST NOT support EdDSA using the pre-hash mode.

Can you explain why you are prohibiting this mode? I'm not endorsing it,
but I don't see a prohibition on (for instance) secp256k1 though that's
presumably also inadvisable.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-272>
draft-ietf-lamps-rfc5751-bis.txt:488
section of the community which believes that there might be a
backdoor to these curves. The EdDSA curves were, in part, created in
response to this feeling. However, there are still significant

This seems like it's kind of controversial for no good reason. These curves
are more modern, faster, and were standardized in IETF. Isn't that a good
enough reason?

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-273>
draft-ietf-lamps-rfc5751-bis.txt:491
sections of the industry which need to have NIST approved algorithms.
For this reason, both sets of curves are represented in the recieving
agent list, but there is only a requirement for curve in the sending

Typo: receiving

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-274>
draft-ietf-lamps-rfc5751-bis.txt:495
See Section 4.1 for information on key size and algorithm references.

It might be worth noting that these requirements are intended to guarantee
that any pair of sender and receiver can interoperate.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-276>
draft-ietf-lamps-rfc5751-bis.txt:518
with AES-128 content encryption algorithm). As both 128 and 256 bit
AES modes are mandatory-to-implment as content encryption algorithms
(Section 2.7), both the AES-128 and AES-256 key wrap algorithms MUST

Typo: implement.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-277>
draft-ietf-lamps-rfc5751-bis.txt:546
2.4.2. SignedData Content Type

Note to self: order of signature and encryption

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-279>
draft-ietf-lamps-rfc5751-bis.txt:640
implies that the sender can deal with the algorithm as well as
undertanding the ASN.1 structures associated with that algorithm.
There are also several identifiers that indicate support for other

Typo: understanding.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-280>
draft-ietf-lamps-rfc5751-bis.txt:649
If present, the SMIMECapabilities attribute MUST be a
SignedAttribute; it MUST NOT be an UnsignedAttribute. CMS defines
SignedAttributes as a SET OF Attribute. The SignedAttributes in a
IMPORTANT: there are a number of conformance requirements here. You need to
specify how one should behave if you receive a misformatted message.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-281>
draft-ietf-lamps-rfc5751-bis.txt:653
SMIMECapabilities attribute. CMS defines the ASN.1 syntax for
Attribute to include attrValues SET OF AttributeValue. A
SMIMECapabilities attribute MUST only include a single instance of

This seems ungrammatical. Perhaps "as a SET OF"

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-282>
draft-ietf-lamps-rfc5751-bis.txt:656
AttributeValue. There MUST NOT be zero or multiple instances of
AttributeValue present in the attrValues SET OF AttributeValue.

How should I behave in this case as well?

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-283>
draft-ietf-lamps-rfc5751-bis.txt:671
matches. For instance, the DER-encoding for the SMIMECapability for
AES-128 CBC MUST be identically encoded regardless of the
implementation. Because of the requirement for identical encoding,

Is this a normative reference or a statement of fact?

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-284>
draft-ietf-lamps-rfc5751-bis.txt:717
signerInfo MUST NOT include multiple instances of the
SMIMEEncryptionKeyPreference attribute. CMS defines the ASN.1 syntax
for Attribute to include attrValues SET OF AttributeValue. A

It would be clearer if here and above you said "Although CMS defines... the
SignedAttributes ... MUST NOT include multiple" to make clear you are
narrowing the CMS rules.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-285>
draft-ietf-lamps-rfc5751-bis.txt:745
In order to determine the key management certificate to be used when
sending a future CMS EnvelopedData message for a particular

"key management" seems like an unclear name. Perhaps "encryption key" to
match 2.5.3?

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-286>
draft-ietf-lamps-rfc5751-bis.txt:758
the same subject name as the signer of a X.509 certificate that
can be used for key management.
IMPORTANT: Does this certificate itself have to be valid?

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-287>
draft-ietf-lamps-rfc5751-bis.txt:863
agent chooses not to use AES-256 GCM in this step, it SHOULD use
AES-128 CBC.

Why not AES-128 GCM, as that is also a MUST.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-288>
draft-ietf-lamps-rfc5751-bis.txt:965
fields. It is up to the receiving client to decide how to present
this "inner" header along with the unprotected "outer" header.
IMPORTANT: I think some guidance here is probably needed, at least to
explain that if you intermix them it causes confusion. We have seen plenty
of issues here.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-290>
draft-ietf-lamps-rfc5751-bis.txt:1013
S/MIME implementations SHOULD however use transfer encoding described
in Section 3.1.3 for all MIME entities they secure. The reason for
securing only 7-bit MIME entities, even for enveloped data that are

This seems to conflict with the next paragraph. Maybe "In general" here and
"In the case where" in front of the next would help it read better.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-289>
draft-ietf-lamps-rfc5751-bis.txt:1029
(such as base64) would unnecessarily expand the message size.
Implementations MAY "know" that recipient implementations are capable
of handling inner binary MIME entities either by interpreting the id-

If you change "know" to "determine" here you can lose the scare quotes.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-291>
draft-ietf-lamps-rfc5751-bis.txt:1072
multipart/signed message with 8-bit or binary data in the first part,
it would have to return the message to the sender as undeliverable.

Nit: "encounters" and "would have to" disagree. I believe you want
"encountered"

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-292>
draft-ietf-lamps-rfc5751-bis.txt:1208
assigned, then this can be omitted. Thus, since "certs-only" can
only be signed, "signed-" is omitted.

Isn't it "verification" and "decryption" which are applied?

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-293>
draft-ietf-lamps-rfc5751-bis.txt:1232
possible to replace ciphertext in such a way that the processed
message will still be valid, but the meaning can be altered.
IMPORTANT: this is not correct if an AEAD algorithm is used.

I'm just getting into this, but it appears that you have the following
choices:

   - EnvelopedData with an encryption algorithm
   - EnvelopedData with an AEAD algorithm
   - AuthEnvelopedData with an encryption algorithm and a MAC
   - AuthEnvelopedData with an AEAD algorithm

The second and fourth of these seem to differ in whether you have
authenticated attributes.

Or is #2 prohibited? It wasn't entirely clear to me from 5083 and I haven't
chased it yet.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-294>
draft-ietf-lamps-rfc5751-bis.txt:1276
valid, but the meaning can be altered. However this is substantially
more difficult than it is for an enveloped-only message as the

Can you elaborate more on this attack? It seems like you could just swap in
your own ciphertext, but that's like the same as just crafting a new
message, right?

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-295>
draft-ietf-lamps-rfc5751-bis.txt:1589
it will not yield significant compression. Base64 encrypted data
could very well benefit, however.

Incidentally, this is not strictly true. Consider the case where you use
AES-GCM to encrypt data which you know to itself be 7-bit clean. In that
case you can easily obtain a 12.5% compression ratio by stripping the high
bit and replacing it with zeros on decompression. For the more general case
see: https://pdfs.semanticscholar.org/6072/b10c4ab2851047464fe4fff766693a
4184e6.pdf

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-296>
draft-ietf-lamps-rfc5751-bis.txt:1661
automatically generate a message to an intended recipient requesting
that recipient's certificate in a signed return message. Receiving
and sending agents SHOULD also provide a mechanism to allow a user to

This is just like textually requesting it, right? Or is there an indicator
for this?

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-297>
draft-ietf-lamps-rfc5751-bis.txt:1878
Using weak cryptography in S/MIME offers little actual security over
sending plaintext. However, other features of S/MIME, such as the

This seems inconsistent with the definition of "weak" as <= 112 bits. That
still offers significant security

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-298>
draft-ietf-lamps-rfc5751-bis.txt:1937
Many people assume that the use of an authenticated encryption
algorithm is all that is needed to be in a situtation where the
sender of the message will be authenticated. In almost all cases

Typo: situation

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-264>
draft-ietf-lamps-rfc5751-bis.txt:1989
a concern need to provide some type of padding so that the length of
the message does not provide this information.
IMPORTANT: You also need to talk about compression oracles.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-299>
draft-ietf-lamps-rfc5751-bis.txt:2343
Appendix A. ASN.1 Module

Note: I did not review this on the premise that it was done so in 3851.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-300>
draft-ietf-lamps-rfc5751-bis.txt:2456
statement on SHA-1 can be found in [RFC6194] but it is out-of-date
relative to the most recient advances.

Typo: recent.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-301>
draft-ietf-lamps-rfc5751-bis.txt:2498
the binding of the public key to the identity that signed the
message as well.

You should note that because these weaknesses are recent (at least in the
public sector) if you know you have had the message for a long time, it may
be safer to accept it.

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-302>
draft-ietf-lamps-rfc5751-bis.txt:2517
some issues have been created that can cause interopatability
problems:

Typos: mandatory, interoperability

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-303>
draft-ietf-lamps-rfc5751-bis.txt:2565
to encrypt new emails.
B.4. KeyEncryptionAlgorithmIdentifier

Did you mean to use RFC 2119 normative language here?

View Inline <https://mozphab-ietf.devsvcdev.mozaws.net/D8#inline-304>
draft-ietf-lamps-rfc5751-bis.txt:2589
it is recommended that RFC 2311 [SMIMEv2] be moved to Historic
status.

Are you actually moving it, or just suggesting that the IESG do so?

*REPOSITORY*
rIETFREVIEW ietf-review

*REVISION DETAIL*
https://mozphab-ietf.devsvcdev.mozaws.net/D8

*EMAIL PREFERENCES*
https://mozphab-ietf.devsvcdev.mozaws.net/settings/panel/emailpreferences/

*To: *ekr-moz, ekr