> One general comment. I think we should avoid making strict
> recommendations. I would like to use the words such as "Consider
> avoiding XSLT" or "Take care when processing RetrievalMethod". I
> think we should leave the final decision up to the reader, and we
> should just be advisors, and not make strict rules
+1
regards, Frederick
Frederick Hirsch
Nokia
On Jun 10, 2008, at 1:12 PM, ext Sean Mullan wrote:
>
> Hi Pratik,
>
> Thanks for incorporating my feedback. I have some more comments,
> and they are mostly on specific text or editorial related.
>
> One general comment. I think we should avoid making strict
> recommendations. I would like to use the words such as "Consider
> avoiding XSLT" or "Take care when processing RetrievalMethod". I
> think we should leave the final decision up to the reader, and we
> should just be advisors, and not make strict rules. Just my two
> cents. I wonder how others feel about that.
>
> * 1 Overview
>
> Although much of the best practices is related to mitigating
> attacks, some are not (such as the namespace section). So I would
> recommend rewording this to state that the best practices cover
> more than just attacks, perhaps simply we can change this sentence:
>
> "This flexibility has the downside of increasing the number of
> possible attacks."
>
> to:
>
> "One of the significant downsides of this flexibility is that it
> can increase the number of possible attacks."
>
> * 2.1 For Implementors: Reduce the opportunities for denial of
> service attacks
>
> Step 1 get the verification key
>
> I suggest changing this to "establish trust in the verification key"
>
> * Best Practice 2: Avoid RetrievalMethod.
>
> Very constrained RetrievalMethods are a lot safer. There may be
> some legitimate advantages to RetrievalMethods, perhaps if you have
> multiple signatures in the same document and you want to avoid
> duplicate KeyInfo certificate entries. So maybe we want to change
> this BP to "Take care when processing RetrievalMethod".
>
> * Best Practice #3: "In addition to verifying an XML Signature,
> also check that the key is valid."
>
> The way this is worded it sound likes you could validate the key
> after you validate the signature, so I suggest just changing this
> to "Establish trust in the verification/validation key". (You
> already mentioned the BP about order of steps so I don't think you
> need to repeat that here).
>
> A couple of other comments on the previous paragraph:
>
> "If the KeyInfo is a X509Certificate element, the certificate needs
> to be validated by checking the signature of the certificate's
> issuer, the expiry date, purpose of the certificate, and also make
> sure that it has not been revoked."
>
> These are just some of the checks, not an exhaustive list. I might
> reference RFC 3280/5280 or simply change this to "If the KeyInfo is
> a X509Certificate element, the certificate needs to be validated.
> This involves verifying information in the certificate (for
> example, the expiration date, the purpose of the certificate,
> checking that it is not revoked, etc), and potentially building and
> validating a chain of certificates to a trusted certificate
> authority. See RFC 3280 for more information."
>
> "However if the KeyInfo is a RSA or DSA KeyValue, then there is no
> way to validate the key, so these should not be normally trusted."
>
> That's not totally true, as the keys may have been exchanged and
> verified OOB. I agree on avoiding KeyValues, but unless we
> elaborate on this some more, I would probably omit this sentence.
>
> Also, validation of the key is typically more than an
> implementation issue, and often involves application specific
> information. I'm not sure if we want to mention that though. I
> think the point is clear enough.
>
> * 2.1.1
>
> s/The following XSLT transform does 4 levels of nested loops/The
> following XSLT transform contains 4 levels of nested loops
>
> s/and in each loop it goes over all the nodes/and for each loop it
> iterates over all the nodes
>
> s/an application server to spend hours processing this it/an
> application server to spend hours processing it
>
> In the sentence:
>
> "To totally eliminate this kind of attack an implementation can
> choose to not support XSLT at all."
>
> I suggest also adding this text:
>
> or provide a mechanism to allow the application to optionally
> disable support for it.
>
> (This puts the burden on the application developer to know what
> they are doing, but making it optional or providing some way to
> enable/disable it is more API/toolkit-friendly). It could also be
> disabled out of the box, but need to be explicitly enabled.
>
> * 2.1.2
>
> s/The following XPath Transforms/The following XPath Transform
>
> I would add the same text above in this section about also
> providing a way to optionally enable/disable the XPath Transform.
>
> This section should also recommend using the XPath Filter Transform
> instead, which I believe avoids the DOS issue in the example. This
> is more of an application issue, but I think it should still
> mention it, as it was designed to address these expensive
> processing issues with the XPath transform.
>
> 2.1.3
>
> so it is best to not support RetrievalMethod at all.
>
> As mentioned earlier, I think that is too strong of a recommendation.
>
>
> more comments later ...
>
>
> --Sean
>
> Pratik Datta wrote:
>> I have updated the best practices document to reflect the
>> discussion that Sean and I were having.
>> http://www.w3.org/2007/xmlsec/Drafts/xmldsig-bestpractices/
>> The main changes are
>> * reorganization by audience - implementors or app developers.
>> * put in a lot of "why" before each best practice.
>> I have not yet changed the timestamp section to incorporate Juan
>> Carlos's comments.
>> Pratik
>> Sean Mullan wrote:
>>>
>>> Pratik Datta wrote:
>>>>
>>>> Sean,
>>>>
>>>> I am not trying to cover any policy related issues here.
>>>
>>> But it is kind of a policy issue, right? This is similar to WS
>>> Security Policy where there are constraints that are required to
>>> be satisfied. One of those constraints is to specify which parts
>>> of the document must be signed. So a validating (and signing)
>>> implementation must ensure those requirements are satisfied. Am I
>>> off?
>>>
>>>> The main issue, as you said, is just to see what was signed.
>>>
>>> I now think they are related but a little different. "See what is
>>> signed" is more applicable to something processing and acting on
>>> the actual data that has been signed (and doesn't necessarily
>>> care what parts of the document were signed), whereas ensuring
>>> that certain elements are signed is more of a lower-level
>>> security related issue where the user isn't really involved (as
>>> you say below it is more of a programmatic issue). Anyway, I
>>> think they are both subject to the same sort of attacks when
>>> using arbitrary transforms. So the resulting best practice should
>>> cover both issues :)
>>>
>>>>
>>>> I would like to know what kind of hooks can be put in the
>>>> implementation code to cache and return data.
>>>>
>>>> One idea is to get the bytes just before digesting and return
>>>> that as an array of bytes. However this approach is only useful
>>>> for visual comparison, and can't be used for programmatic
>>>> comparison - how would you produce the other array of bytes to
>>>> compare with? Also this approach is susceptible to wrapping
>>>> attacks.
>>>>
>>>> The other approach that I can think of (and this is what the
>>>> Oracle implementation uses) is to run all the transforms except
>>>> the last C14N transform, which may be implicit - the output of
>>>> this should be nodeset. Then one can use the DOM3 method
>>>> isSameNode() to compare each node of the expected nodeset with
>>>> the nodeset that is being signed. The expected nodeset should be
>>>> a subset of the nodeset being signed. The restriction with
>>>> approach is that there should not be any sequences of
>>>> transforms that converts from xml to binary and then back to
>>>> xml, because in that case signed nodeset belongs to a different
>>>> document, and will not match with the expected nodeset.
>>>
>>> So maybe another best practice would be to advise that
>>> implementations have APIs to return this data (pre-digested and
>>> pre-c14n data).
>>>
>>> --Sean
>>>
>
>