Subject: Re: [smime] draft-housley-ct-keypackage-receipt-n-error-00

From: Russ Housley <[email-masked]>
Date: Thu, 2 May 2013 10:07:51 -0400

Jim:

Thank you for the review.

> 1.  What is SIR - not defined

Source Intermediary Recipient (SIR) entity name

> 2.  Is it your expectation that name types which do not have ASN.1 values
> are going to be created?  If not then why is there an OCTET STRING wrapper
> for nameValue.  This choice should be justified in the document.

The nameValue is an OCTET STRING, which allows the canonical form of any name to be carried.  Two names of the same type are considered equal if the octet strings are the same length and contain the same string of octets.

> 3.  Should you define a relationship for relating nameType and nameValue
> information?  Automated packages would find it useful, it also makes the
> fact that you are use Name rather than possibly GeneralName explicit in the
> module.

I am not totally sure what you are suggesting.  Let me know if I got it right.

  SIR-ENTITY-NAME ::= CLASS {
      &SIRNameType  OBJECT IDENTIFIER UNIQUE,
      &SIRNameValue
  } WITH SYNTAX {
      SYNTAX &SIRNameValue IDENTIFIED BY &SIRNameType
  }

  SIRNames{SIR-ENTITY-NAME:SIRNameSet} ::=
      SEQUENCE SIZE (1..MAX) OF SIRName{{SIRNameSet}}

  SIRName{SIR-ENTITY-NAME:SIRNameSet} ::= SEQUENCE {
      sirNameType      SIR-ENTITY-NAME.&SIRNameType({SIRNameSet}),
      sirNameValue   OCTET STRING (CONTAINING
                  SIR-ENTITY-NAME.&SIRNameValue({SIRNameSet}{@sirNameType}))

> 4.  You should probably give a reference to where signed, authenticated and
> content attributes are found.  I am familiar with the first two since I do a
> lot of CMS work, however the last one really needs to be tied to an RFC and
> a specific content type.

This attribute can appear as a signed, authenticated, and content attribute.  Signed attributes are carried in the CMS Signed-data content type described in Section 5 of [RFC5652].  Authenticated attributes are carried in the CMS Authenticated-data content type described in Section 9 of [RFC5652] or in the CMS Authenticated-enveloped-data content type described in Section 2 of [RFC5083].  Content attributes are carried in the Content-with-attributes content type described in Section 3 of [RFC4073].

> 5.  Can the key package identifier and receipt request attribute have
> multiple values or is a single value attribute?

Even though the ATTRIBUTE syntax is defined as a SET OF AttributeValue, a key-package-identifier-and-receipt-request attribute MUST have a single attribute value; zero or multiple instances of AttributeValue are not permitted.

> 6.  We can rehash this discussion.  First I don't see any reason for
> incrementing the version number unless you are going to re-assign the same
> OID for the new structure as you did for the old one.  If a new OID is used
> there is more than enough information to distinguish between the two
> different structures.  Second, I am not a fan of assigning version numbers
> to these structures because they do not help any encoding/decoding systems.
> The structure will be encoded or decoded based on the ASN and not on the
> version number.

I'd rather not rehash this discussion,  Instead, I'd like to follow the convention used in CMS.

> 7.  What is the purpose of the (1..MAX) on the definition of KeyPkgVersion.
> Are you just trying to say that it cannot be a negative value?  It would be
> more helpful if you used a smaller version such as 2^32-1 so that a compiler
> would know that it fit into an int32 value.  I would also note that this is
> a change from the old definition of the field in RFC 6031

Yes, the reader is warned that this definition is different.  This is also a reason not to import the definition:

-- Revised definition of KeyPkgVersion from [RFC6031]
KeyPkgVersion ::= INTEGER  { v1(1), v2(2) } (1 .. MAX)

I think a maximum of 65535 is very safe.

> 8.  Is there a requirement that systems should accept
> KeyPkgIdentifier.attribute values that they do not understand as it can be
> reflected in the receipt without having to decode it?

As with all CMS processing, unrecognized attributes are ignored.  I'm not sure this needs to be repeated further.  It comes up here:

       * badUnsignedAttrs is used to indicate that the unsignedAttrs
         within SignerInfo contains one or more attributes.  Since
         unrecognized attributes are ignored, this error code is used
         when the object identifier for the attribute is recognized, but
         the value is malformed or internally inconsistent.
 
> 9.  Why is there a requirement that the content types have to be encoded
> using the DER rules?  This is not a general requirement imposed by CMS and
> therefore needs some justification text.

I am not sure that justification needs to be added to the document, but I'll share what I was thinking.  This content could be processed inside a security boundary.  Using DER allows more straightforward processing, including the use of templates.

> 10.  I find the field names errorOf and errorBy to be obtuse - but that is
> not a strong reason to change them if they have a meaning that is not
> documented.

The field names make sense to me.  Do you find the explanation wanting?

> 11. does badContentInfo apply to embedded content types (i.e. eContent) as
> well as the ContentInfo structure?

I was thinking that a failure at an embedded layer would result in a badEncapContent error code.

> 12.  Why should there be a problem with having more than one entry in the
> digestAlgorithms field?  I can potentially understand complaining if there
> is one that is not understood but not if there is more than one.

I'm trying to keep it simple by requiring a single algorithm used by a single signer.

> 13.  I must have missed the rule that says that there is a problem if you
> have a signed attribute that is unknown and not ignored.  Is that part of
> the badSignedAttrs field or is this only an issue with the ASN.1 encoding?

AsI said above, I am following the normal CMS processing, unrecognized attributes are ignored.

> 14.  notAuthroized - this description seems off for this content type.  Is
> the issue that the TA is not authorized or the TA does not root the
> authorization.  It would not be the signer itself in this case one presumes.

TAMP (RFC 5934) can be used to associate a list of authorized content types with a TA.

> 15.  put in a reference for the content-decryption-key-identifier attribute.

Good idea:

       * noDecryptKey indicates that the receiver does not have the key
         named in the content-decryption-key-identifier attribute (see 
         [RFC6032]).

> 16.  Is there a reason for the badKeyTransportRecipientInfo item being
> absent?

Added ...

       * badKeyTransRecipientInfo indicates that the
         KeyTransRecipientInfo syntax is invalid or the version is
         unknown or unsupported.

> 17.  Do you want to distinguish between a decryptFailre and a failure
> processing a key management item?  This is the basis of some online attacks
> to get a key when dealing with RSA v1.5 and RSA OEAP.  This merits a
> security consideration notice all by itself.

Please see my response to comment 29 below.

> 18.  Are there any security attacks that occur by differentiating between
> decryptFailure and invalidMAC for AuthEnvelopedData?

I do not think so.  AES-CCM and AES-GCM do not return any plaintext if there is a integrity failure.

> 19.  For mismatchedDigestAlg - are you comparing with the signature
> algorithm or with the content digest algorithm?

I think this one is clear.  It indicates that the digest algorithm in digestAlgorithms field within SignedData does not match the digest algorithm used by the content signer.

> 20.  You need some text to distinguish missingCertificate and noTrustAnchor
> if keep the "using a trust anchor" text.

TAMP (RFC 5934) tells how a TA can sign the content directly.  That applies here too.

> 21.  tooManySigners - where is this restriction imposed?

I think this makes sense in most key distribution scenarios.  How about this text?

       * tooManySigners indicates that a SignedData content contained
         more than one SignerInfo for a content type that requires only
         one signer.

> 22.  can the missingSignedAttributes be used if there are attributes that
> are required to be present but are absent - even if there are some attibutes
> present?

Yes.

> 23.  I don't understand the reason for the missingContentHints.  This
> attribute would be an encrypted structure not a signed structure.  Why would
> a content hint make any difference in terms of the processing?

This is related to CMS Content Constraints (CCC) defined in RFC 6010.

> 24.  Are there any security attacks that are uncovered by the use of the
> badMessageDigest error code rather than just saying that the signature
> failed to validate?

Please see my response to comment 29 below.

> 25.  badAttributes should be modified to say that this is an error only if
> the attribute is defined to say that it is not legal for that attribute.
> For some attributes having multiple attributes or multiple values is legal.

How about this?

       * badAttributes indicates that an attribute collection contained
         either multiple instances of the same attribute type that
         allows only one instance or contained an attribute instance
         with multiple values in an attribute that allows only one
         value.

> 26.  The description of unsupportedAsymmetricKeyPackage does not make sense
> - There is a difference between not prepared and unsupported

How about this?

       * unsupportedSymmetricKeyPackage indicates that the
         implementation does not support symmetric key packages
         [RFC6031].

       * unsupportedAsymmetricKeyPackage indicates that the
         implementation does not support asymmetric key packages
         [RFC5958].

> 27.  Should I be able to return more than one errorCode if I find more than
> one error during processing?

No.  I'd like to keep it simple.

> 28.  Given the number of times that AuthenticatedData was mentioned in the
> text, is there a reason it is omitted from section 6?

Added ...

     o AuthenticatedData can be used to integrity protect the content
       type with message authentication algorithms that support
       authenticated encryption, where key management information is
       handled in a manner similar to EnvelopedData.

> 29.  Security consideration on the cost benefits of using a generic vs a
> specific error code.  Some specific codes might leak security information.

Does the following text capture your point?

   In some situations, returning very detailed error information can
   provide an attacker with insight into the security processing.  Where
   this is a concern, the implementation should return the most generic
   error code that is appropriate.  However, detailed error codes are
   very helpful during development, debugging, and interoperability
   testing.  For this reason, implementations may want to have a way to
   configure the use of a generic error code or a detailed one.

> 30.  Do you really want to make the KeyPkgVersion revised or is it a totally
> independent thing?  As currently setup it will be a different thing since it
> is in a different module.  If you want it to be a change on what is in the
> original module then you need to re-publish that module as well.

I do not see this as a problem.  The v2 value is not defined in RFC 6031, otherwise an IMPORT might be appropriate.

Again, thanks very much for the review.

Russ

---

From: "Jim Schaad" <[email-masked]>
Date: Thu, 2 May 2013 13:04:41 -0700

> -----Original Message-----
> From: Russ Housley [mailto:[email-masked]]
> Sent: Thursday, May 02, 2013 7:08 AM
> To: Jim Schaad
> Cc: 'IETF SMIME'
> Subject: Re: [smime] draft-housley-ct-keypackage-receipt-n-error-00
> 
> Jim:
> 

Good

> 
> > 2.  Is it your expectation that name types which do not have ASN.1
> > values are going to be created?  If not then why is there an OCTET
> > STRING wrapper for nameValue.  This choice should be justified in the
> document.
> 
> The nameValue is an OCTET STRING, which allows the canonical form of any
> name to be carried.  Two names of the same type are considered equal if
the
> octet strings are the same length and contain the same string of octets.

Good

> 
> > 3.  Should you define a relationship for relating nameType and
> > nameValue information?  Automated packages would find it useful, it
> > also makes the fact that you are use Name rather than possibly
> > GeneralName explicit in the module.
> 

Yes that looks correct.  You could use a fixed name set if you wanted to
rather than having it be a parameter.  This would depend on how you are
planning to use it.

> 
> > 4.  You should probably give a reference to where signed,
> > authenticated and content attributes are found.  I am familiar with
> > the first two since I do a lot of CMS work, however the last one
> > really needs to be tied to an RFC and a specific content type.
> 
> This attribute can appear as a signed, authenticated, and content
attribute.
> Signed attributes are carried in the CMS Signed-data content type
described
> in Section 5 of [RFC5652].  Authenticated attributes are carried in the
CMS
> Authenticated-data content type described in Section 9 of [RFC5652] or in
the
> CMS Authenticated-enveloped-data content type described in Section 2 of
> [RFC5083].  Content attributes are carried in the Content-with-attributes
> content type described in Section 3 of [RFC4073].
> 

Good

> Even though the ATTRIBUTE syntax is defined as a SET OF AttributeValue, a
> key-package-identifier-and-receipt-request attribute MUST have a single
> attribute value; zero or multiple instances of AttributeValue are not
> permitted.
> 

Good

> > incrementing the version number unless you are going to re-assign the
> > same OID for the new structure as you did for the old one.  If a new
> > OID is used there is more than enough information to distinguish
> > between the two different structures.  Second, I am not a fan of
> > assigning version numbers to these structures because they do not help
any
> encoding/decoding systems.
> > The structure will be encoded or decoded based on the ASN and not on
> > the version number.
> 
> I'd rather not rehash this discussion,  Instead, I'd like to follow the
convention
> used in CMS.

I don't need to rehash the discussion either.  I will just always raise it.

> > Are you just trying to say that it cannot be a negative value?  It
> > would be more helpful if you used a smaller version such as 2^32-1 so
> > that a compiler would know that it fit into an int32 value.  I would
> > also note that this is a change from the old definition of the field
> > in RFC 6031
> 
> Yes, the reader is warned that this definition is different.  This is also
a reason
> not to import the definition:
> 
> -- Revised definition of KeyPkgVersion from [RFC6031] KeyPkgVersion ::=
> INTEGER  { v1(1), v2(2) } (1 .. MAX)
> 

I agree that is probably more than sufficient.

> > KeyPkgIdentifier.attribute values that they do not understand as it
> > can be reflected in the receipt without having to decode it?
> 
> As with all CMS processing, unrecognized attributes are ignored.  I'm not
sure
> this needs to be repeated further.  It comes up here:
> 

I don't think that this is an acceptable solution ore response at this
point.

If I send you 

Key package id and receipt request ::= {
   pkgID = { random OID you never heard of, binary value }
  receiptReq = {
   encryptReceipt FALSE,
   receiptsFrom - absent
   receiptsTo = {Me}
}}

You have three options:

1 - say that the signed attribute is bad because you do not understand a
piece if it and neither process nor receipt the package
2 - say that you don't care that the signed attribute is bad and process it
and return a receipt because you do not need to understand the key package
identifier
3 - say that you ignore things you do not understand and process the package
but do not return a receipt.

> 
> > 9.  Why is there a requirement that the content types have to be
> > encoded using the DER rules?  This is not a general requirement
> > imposed by CMS and therefore needs some justification text.
> 
> I am not sure that justification needs to be added to the document, but
I'll
> share what I was thinking.  This content could be processed inside a
security
> boundary.  Using DER allows more straightforward processing, including the
> use of templates.

That's fine - I thought it might be something like that.  

> 
> > 10.  I find the field names errorOf and errorBy to be obtuse - but
> > that is not a strong reason to change them if they have a meaning that
> > is not documented.
> 

The explanations are just fine.   I merely find the names odd.  I would have
expected something more along the lines of

errorOnPackage - name of package
errorReportedBy -

That is longer more descriptive names.  But as I said - I don't really care
that much.

> 
> > 11. does badContentInfo apply to embedded content types (i.e.
> > eContent) as well as the ContentInfo structure?
> 

Ok - that is what is reflected.

> 
> > 12.  Why should there be a problem with having more than one entry in
> > the digestAlgorithms field?  I can potentially understand complaining
> > if there is one that is not understood but not if there is more than
one.
> 
> I'm trying to keep it simple by requiring a single algorithm used by a
single
> signer.

Ok - I can see this as being reasonable.

> 
> > 13.  I must have missed the rule that says that there is a problem if
> > you have a signed attribute that is unknown and not ignored.  Is that
> > part of the badSignedAttrs field or is this only an issue with the ASN.1
> encoding?
> 

Ok - may not be correct behavior in all cases, but that can be application
specific and that is the rule you are using in this application.

> 
> > 14.  notAuthroized - this description seems off for this content type.
> > Is the issue that the TA is not authorized or the TA does not root the
> TAMP (RFC 5934) can be used to associate a list of authorized content
types
> with a TA.

It also allows for a list of authorized contents to be associated with a
signer certificate as well I thought.  If so then this description would
seem to not allow for that case.  That is the current signing certificate
can be more restrictive than the TA is.

>          named in the content-decryption-key-identifier attribute (see

Good

>          KeyTransRecipientInfo syntax is invalid or the version is

Good

> > processing a key management item?  This is the basis of some online
> > attacks to get a key when dealing with RSA v1.5 and RSA OEAP.  This
> > merits a security consideration notice all by itself.
> 

Yes that deals with the issue.

> > 18.  Are there any security attacks that occur by differentiating
> > between decryptFailure and invalidMAC for AuthEnvelopedData?
> 
> I do not think so.  AES-CCM and AES-GCM do not return any plaintext if
there
> is a integrity failure.

I was thinking in terms of something similar to the attacks that exist for
the RSA v1.5/OAEP differences.  There may be something that may leak
information to an attacker that could be useful.  However I think that the
comment on 29 probably addresses this issue as well.

> I think this one is clear.  It indicates that the digest algorithm in
> digestAlgorithms field within SignedData does not match the digest
algorithm
> used by the content signer.

I might prefer s/used by the content signer/used in the signature algorithm/
However this was a double check so the current text is probably sufficient.

> 
> > 20.  You need some text to distinguish missingCertificate and
> > noTrustAnchor if keep the "using a trust anchor" text.
> 
> TAMP (RFC 5934) tells how a TA can sign the content directly.  That
applies
> here too.

Ok - I can see this

> I think this makes sense in most key distribution scenarios.  How about
this
> text?

Good

> > 22.  can the missingSignedAttributes be used if there are attributes
> > that are required to be present but are absent - even if there are
> > some attibutes present?
> 
> Yes.

Ok

> > attribute would be an encrypted structure not a signed structure.  Why
> > would a content hint make any difference in terms of the processing?
> 

Yes - I had forgotten that type.

> > 24.  Are there any security attacks that are uncovered by the use of
> > the badMessageDigest error code rather than just saying that the
> > signature failed to validate?
> 

Again - that covers the issue

> 
> > 25.  badAttributes should be modified to say that this is an error
> > only if the attribute is defined to say that it is not legal for that
attribute.
>          with multiple values in an attribute that allows only one

Yes that works

>          implementation does not support asymmetric key packages

Yes that deals with the issue

> > 27.  Should I be able to return more than one errorCode if I find more
> > than one error during processing?
> 

That is fine with me, I was just making sure that you had considered the
issue.

> 
> > 28.  Given the number of times that AuthenticatedData was mentioned in
> > the text, is there a reason it is omitted from section 6?
> 
> Added ...
> 

Looks good

> > 29.  Security consideration on the cost benefits of using a generic vs
> > a specific error code.  Some specific codes might leak security
information.
>    configure the use of a generic error code or a detailed one.

Yes that addresses the issues that I was looking at

> 
> > 30.  Do you really want to make the KeyPkgVersion revised or is it a
> > totally independent thing?  As currently setup it will be a different
> > thing since it is in a different module.  If you want it to be a
> > change on what is in the original module then you need to re-publish
that
> module as well.
> 
> I do not see this as a problem.  The v2 value is not defined in RFC 6031,
> otherwise an IMPORT might be appropriate.
> 

Ok - no big deal.  

> Again, thanks very much for the review.

Welcome - jim

---

From: Russ Housley <[email-masked]>
Date: Fri, 17 May 2013 09:34:27 -0400

> rather than having it be a parameter.  This would depend on how you are

I have not made this change yet.  It seems I would need a '88 and an '02 module.

I'd appreciate advice on the fixed name set vs. the parameter.

Russ

---

From: Russ Housley <[email-masked]>
Date: Fri, 17 May 2013 09:36:48 -0400

> but do not return a receipt.

Does this text resolve you concern?

      * badUnsignedAttrs is used to indicate that the unsignedAttrs
        within SignerInfo contains one or more attributes.  Since
        unrecognized attributes are ignored, this error code is used
        when the object identifier for the attribute is recognized, but
        the value is malformed or internally inconsistent.  In
        addition, this error code can be used when policy prohibits an
        implementation from supporting unsigned attributes.

Russ

---

From: "Jim Schaad" <[email-masked]>
Date: Fri, 17 May 2013 18:45:56 +0100

The use of things such as CONTENT-TYPE are already pushing you to the point
of needing an '88 ASN.1 module in the event that is a requirement.  The
addition of the SIR-ENTITY-NAME class does not change anything from the
current world.  As such I do not believe that this is a change that would
affect a decision on providing an '88 module.

One would use a parameterized class definition if one believe that the class
would be used in multiple locations with different sets of parameters.  Thus
it makes sense in the case of CMS to define a single class and type for
signed attributes, unsigned attributes, authenticated attributes,
unauthenticated attributes as a parameterized set.  The same basic type
structure is used in each of these locations but with a different set of
possible values that can go into each location.

One would use a global/fixed name set in the event that something is used in
exactly one location and there is no reason to expect that it would be
imported into a different module and used with a different set of possible
values.  Thus we use a single global set in the update to 5272 for the
definition of Cmc-Control-Set.

I would say that if you expect this to be used in  a different document then
using a parameter makes sense.  Otherwise I would use a fixed object set in
this location.

Jim

> Sent: Friday, May 17, 2013 2:34 PM
> To: Jim Schaad
> Cc: 'IETF SMIME'
> >> I am not totally sure what you are suggesting.  Let me know if I got
> >> it
> > right.
> >>
> >>      SYNTAX &SIRNameValue IDENTIFIED BY &SIRNameType  }
> >>
> > Yes that looks correct.  You could use a fixed name set if you wanted
> > to rather than having it be a parameter.  This would depend on how you
> > are planning to use it.
>

---

From: "Jim Schaad" <[email-masked]>
Date: Fri, 17 May 2013 18:48:33 +0100

> Sent: Friday, May 17, 2013 2:37 PM
> To: Jim Schaad
> Cc: 'IETF SMIME'
> >   pkgID = { random OID you never heard of, binary value }  receiptReq
> > = {
> >   encryptReceipt FALSE,
> >   receiptsFrom - absent
> >   receiptsTo = {Me}
> > }}
> >
> > You have three options:
> >
> > 1 - say that the signed attribute is bad because you do not understand
> > a piece if it and neither process nor receipt the package
> > 2 - say that you don't care that the signed attribute is bad and
> > process it and return a receipt because you do not need to understand
> > the key package identifier
> > 3 - say that you ignore things you do not understand and process the
> > package but do not return a receipt.
> 

No.  I think this is going to need a F2F conversation to resolve.

> Russ=