Re: Best Practices comments

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

Received on Tuesday, 10 June 2008 18:14:04 UTC