ietf-smime
[Top] [All Lists]

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

2013-05-02 15:05:36


-----Original Message-----
From: Russ Housley [mailto:housley(_at_)vigilsec(_dot_)com]
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:

Thank you for the review.

1.  What is SIR - not defined

Source Intermediary Recipient (SIR) entity name

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.

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}))

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

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.


Good

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.

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


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.

I agree that is probably more than sufficient.



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.


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 field names make sense to me.  Do you find the explanation wanting?

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?

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

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?

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


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
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.

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.



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]).


Good

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.

Good


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.


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.


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.

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


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.


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


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.


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?

Please see my response to comment 29 below.

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.
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.


Yes that works


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].


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?

No.  I'd like to keep it simple.

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 ...

     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.


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.

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.

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


Russ

_______________________________________________
smime mailing list
smime(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/smime