Subject: [lamps] Benjamin Kaduk's No Objection on draft-ietf-lamps-cms-update-alg-id-protect-03: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 25 Aug 2020 16:00:43 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-lamps-cms-update-alg-id-protect-03: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-lamps-cms-update-alg-id-protect/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thanks for this; it's perhaps a bit overdue.  Section-by-section comments below.

Section 1

   In an algorithm substitution attack, the attacker looks for a
   different algorithm that produces the same result as the algorithm
   used by the originator.  As an example, if the signer of a message
   used SHA-256 [SHS] as the digest algorithm to hash the message
   content, then the attacker looks for a weaker hash algorithm that
   produces a result that is of the same length.  The attacker's goal is
   to find a different message that results in the same hash value,
   which is commonly called a collision.  [...]

The described scenario seems to be a cross-algorithm collision, which is
not, in my experience, the most common usage of the unqualified term
"collision".  In some sense, it seems that the task for an attacker is
to find (within the context of the "weak" algorithm) a first preimage
for the digest value that is computed by the honest participant (and is
likely to be using a "strong" algorithm).

   Further, when a digest algorithm produces a larger result than is
   needed by a digital signature algorithm, the digest value is reduced
   to the size needed by the signature algorithm.  This can be done both
   by truncation and modulo operations, with the simplest being
   straightforward truncation.  [...]

(But which of truncation and modulo is to be used is fixed by the
algorithm ID, right?  Perhaps a slight rewording to avoid indicating
that the attacker has a free choice is in order.)

   This document makes two updates to CMS to provide similar protection
   for the algorithm identifier.  [...]

nit: the discussion of how X.509 protects the algorithm
identifier/parameters was four paragraphs ago, already; I'd suggest a
bit more exposition about what we're providing "similar protection" as.

Section 3.1

The preexisting text allows implementations to fail to validate
signatures in some cases (when using a digest algorithm not included in
the SignedData digestAlgorithms set); do we want to say anything about
allowing (or requiring?) implementations to fail to validate signatures
if the two digest algorithms are different?

Section 3.2

      When the signedAttrs field is present, the same digest algorithm
      MUST be used to compute the digest of the encapContentInfo
      eContent OCTET STRING, which is carried in the message-digest
      attribute, and the collection of attributes that are signed.

nit: there may be a grammar nit here, relating to the parallelism of
"compute the digest of" -- I think "the collection of attributes that
are signed" should also have an "of" or "digest of" prefix.

Section 3.5

   When producing the TimeStampToken, the TSA MUST use same digest
   algorithm to compute the digest of the encapContentInfo eContent,
   which is an OCTET STRING that contains the TSTInfo, and the message-
   digest attribute within the SignerInfo.

(There's an implicit "in order to comply with the requirement introduced
above" in here, right?)

   To ensure that TimeStampToken values that were generated before this
   update remain valid, no requirement is placed on a TSA to ensure that
   the digest algorithm for the TimeStampToken matches the digest
   algorithm for the MessageImprint embedded within the TSTTokenInfo.

I assume that "TSTTokenInfo" is a typo for "TSTInfo"?

Section 4

I like this quote from RFC 6211:

%                     There is a convention, undocumented as far as I
% can tell, that the same hash algorithm should be used for both the
% content digest and the signature digest.  [...]

It seems we are now documenting this as more than just convention :)

   This section updates [RFC5652] to recommend that the originator
   include the CMSAlgorithmProtection attribute [RFC6211] whenever
   signed attributes or authenticated attributes are present.

Why is the recommendation scoped to only the case when protected
attributes are already present?  Is the recommendation not generically
applicable even when this would be the only protected attribute?

Section 6

   The security properties of the CMS [RFC5652] signed-data and
   authenticated-data content types are updated to ensure that algorithm
   identifiers are adequately protected, which makes algorithm
   substitution attacks significantly more difficult.

Is "ensure" the right word when we only recommend (not require) the use
of the CMSAlgorithmProtection attribute?

   Therefore, it remains important that a signer have a way to signal to
   a recipient which digest algorithms are allowed to be used in
   conjunction with the verification of an overall signature.  This
   signalling can be done as part of the specification of the signature
   algorithm in an X.509v3 certificate extension [RFC5280], or some

I'm not entirely sure I'm picturing what is intended by "part of the
specification of the signature algorithm in an X.509v3 certificate
extension" -- how is the signature algorithm relying on an X.509v3
extension?

   The CMSAlgorithmProtection attribute [RFC6211] offers protection for
   the algorithm identifiers used in the signed-data and authenticated-
   data content types.  However, no protection is provided for the
   algorithm identifiers in the enveloped-data, digested-data, or
   encrypted-data content types.  Likewise, The CMSAlgorithmProtection
   attribute provides no protection for the algorithm identifiers used
   in the authenticated-enveloped-data content type defined in
   [RFC5083].

I feel like we should say something about why we do not provide
protection for those content types (e.g., why it is believed to be safe
to not have such protection).

Section 8.2

The reference to RFC 3161 in Section 3.5 is facially adding a new
MUST-level requirement for processing of the structures from RFC 3161,
which would qualify as a normative reference in my interpretation of
https://www.ietf.org/about/groups/iesg/statements/normative-informative-references/
.  (However, I believe that the "MUST" in that section is just repeating
the requirement from a previous section in the more-specific context, so
could safely be rewritten to not have the appearance of a new normative
requirement, in which case RFC 3161 could properly remain as an
informative reference.)

---

From: Russ Housley <[email-masked]>
Date: Wed, 26 Aug 2020 13:23:14 -0400

Hi Ben.

Would you be more happy with the following?

   ... which is called a cross-algorithm collision.

> that the attacker has a free choice is in order.)

Yes, truncation or modular reduction is selected by the algorithm identifier, but the point here is that the algorithm identifier is not itself protected.  By changing the algorithm identifier, the attacker does get to pick what the verifier will do.

> bit more exposition about what we're providing "similar protection" as.

Maybe just drop "similar".

> if the two digest algorithms are different?

I do not understand your suggestion.

The structure here is:

      SignedData ::= SEQUENCE {
        version CMSVersion,
        digestAlgorithms DigestAlgorithmIdentifiers,
        encapContentInfo EncapsulatedContentInfo,
        certificates [0] IMPLICIT CertificateSet OPTIONAL,
        crls [1] IMPLICIT RevocationInfoChoices OPTIONAL,
        signerInfos SignerInfos }

      DigestAlgorithmIdentifiers ::= SET OF DigestAlgorithmIdentifier

      SignerInfos ::= SET OF SignerInfo

      SignerInfo ::= SEQUENCE {
        version CMSVersion,
        sid SignerIdentifier,
        digestAlgorithm DigestAlgorithmIdentifier,
        signedAttrs [0] IMPLICIT SignedAttributes OPTIONAL,
        signatureAlgorithm SignatureAlgorithmIdentifier,
        signature SignatureValue,
        unsignedAttrs [1] IMPLICIT UnsignedAttributes OPTIONAL }

The preexisting text is also included in the NEW text.  It is saying that the verifier can fail the signature validation if SignerInfo.digestAlgorithm in not in the SET carried in SignedData.digestAlgorithms.

There can be many signatures over the same content.  A SignerInfo is included for each signature.  Each signature can use a different digest algorithm, so I am not sure what "two digest algorithms" you mean.

> are signed" should also have an "of" or "digest of" prefix.

I changed it to:

   When the signedAttrs field is present, the same digest algorithm
   MUST be used to compute the digest of the encapContentInfo
   eContent OCTET STRING, which is carried in the message-digest
   attribute, and the digest of the collection of attributes that
   are signed.

> (There's an implicit "in order to comply with the requirement introduced

Yes, I think the previous paragraph makes that clear.

> I assume that "TSTTokenInfo" is a typo for "TSTInfo"?

Thanks for catching that one.  It has been there a long time...

> It seems we are now documenting this as more than just convention :)

Yes.

> applicable even when this would be the only protected attribute?

Signed-data content type computes the signature differently if there are no protected attributes.  In this case, there is only one digest calculation.  So, this is not adding any attributes if there would not have been any in the first place.

> of the CMSAlgorithmProtection attribute?

I suggest:

   The security properties of the CMS [RFC5652] signed-data and
   authenticated-data content types are updated to offer protection for
   algorithm identifiers, which makes algorithm substitution attacks
   significantly more difficult.

> extension" -- how is the signature algorithm relying on an X.509v3

A certificate extension could be specified to says, only used this public key with a particular digest algorithm.

> to not have such protection).

This attribute does not fit in that structure.  It is work for the future.

I suggest:

   The CMSAlgorithmProtection attribute [RFC6211] offers protection for
   the algorithm identifiers used in the signed-data and authenticated-
   data content types.  However, no protection is provided for the
   algorithm identifiers in the enveloped-data, digested-data, or
   encrypted-data content types.  Likewise, The CMSAlgorithmProtection
   attribute provides no protection for the algorithm identifiers used
   in the authenticated-enveloped-data content type defined in
   [RFC5083].  A mechanism for algorithm identifier protection for these
   content types is work for the future.

> ..  (However, I believe that the "MUST" in that section is just repeating

I will make it a normative reference.  My thinking was that RFC 3161 already had a normative reference to CMS, and this document is updating CMS...

Russ

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 27 Aug 2020 09:41:47 -0700

Hi Russ,

>    ... which is called a cross-algorithm collision.

Yes

> Yes, truncation or modular reduction is selected by the algorithm identifier, but the point here is that the algorithm identifier is not itself protected.  By changing the algorithm identifier, the…

True.
I was thinking maybe something like "Depending on the particular
algorithm, this is done by truncation or by modulo operation", but it's up
to you.

> Maybe just drop "similar".

Sure.

> There can be many signatures over the same content.  A SignerInfo is included for each signature.  Each signature can use a different digest algorithm, so I am not sure what "two digest algorithms"…

Basically, I was noting that the preexisting (and retained) text says
something about failing signature validation in some cases.  The new text
adds a MUST relating two digest algorithms (when signedAttrs is present),
but it is perhaps ambiguous as to whether the requirement is only on those
producing signatures or also applies to those verifying signatures.  Having
a specific mention about failing signature validation when the newly added
MUST does not hold true (but worded appropriately) would be consistent with
the preexisting text that discusses when signature validation could fail,
and would clarify whether the newly added MUST applies to validation as
well as generation of signatures.

>    attribute, and the digest of the collection of attributes that

Thanks.

> Signed-data content type computes the signature differently if there are no protected attributes.  In this case, there is only one digest calculation.  So, this is not adding any attributes if ther…

I understand that it would change the mechanics of how the signature is
computed ... why does that matter for whether the new procedure is the
right thing to do, though?  Is the cost of the second digest calculation
a significant burden?

>    significantly more difficult.

Thanks.

> A certificate extension could be specified to says, only used this public key with a particular digest algorithm.

Ah, I see; thanks.

>    content types is work for the future.

Thank you for clarifying the state of affairs.

> I will make it a normative reference.  My thinking was that RFC 3161 already had a normative reference to CMS, and this document is updating CMS...

Okay.

Thanks for all the updates and clarifications,

Ben

---

From: Russ Housley <[email-masked]>
Date: Thu, 27 Aug 2020 14:31:44 -0400

Ben:

I think there are just two comments that have not reached closure.

> well as generation of signatures.

The MUST applies to the signature generation and validation.  The digestAlgorithm identifies the message digest algorithm, and any associated parameters, used by the signer.  So, I think it is obvious that verification involves the same digest algorithm.

Further, this is the (long) sentence you are talking about.  I do not think that adding "the signer and verifier" will make it more clear.

      If the signedAttrs field is present in the SignerInfo, then the
      same digest algorithm MUST be used to compute both the digest of
      the SignedData encapContentInfo eContent, which is carried in the
      message-digest attribute, and the digest of the DER-encoded
      signedAttrs, which is passed to the signature algorithm.

Since there is only on digest algorithm identifier in the SignerInfo, I cannot think of any additional check to add.

> right thing to do, though?  Is the cost of the second digest calculation

There is no desire to break previous signature, especially since we were unable to find any implementations that did not always use the same digest algorithm.

We did no investigation about the impact of recommending that signed attributes alway be present going forward, so I am reluctant to make such a change at this point in the process.

Russ

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 27 Aug 2020 12:39:03 -0700

Hi Russ,

> I think there are just two comments that have not reached closure.

That sounds right.

> Since there is only on digest algorithm identifier in the SignerInfo, I cannot think of any additional check to add.

Okay, thank you for talking through it; I am fine with no change here.

> We did no investigation about the impact of recommending that signed attributes alway be present going forward, so I am reluctant to make such a change at this point in the process.

So the concern is more about breaking at validation than at
signature-generation time, okay.  That makes more sense than how I was
thinking about it ("of course you wouldn't just randomly require validators
to make a breaking change"), and I can accept your reasoning for not
wanting to make changes at this time without a stronger justification.

Thanks again,

Ben