Detecting duplicate IDs is really orthagonal to the real question: "Is the stuff I care about covered by the signature?". So what I recommended below is really just focused on providing the means to answer that question.
If was wish to provide duplicate ID checking, in addition to this, I think that would be a fine tool to make available to people. On 12/21/11 2:47 PM, Sean Mullan wrote: > Are you still thinking we should also scan the document for duplicate IDs? > > --Sean > > On 12/21/11 7:14 AM, Chad La Joie wrote: >> So, last night I spent some time digging in to the new code and >> discussing with some things with Scott. Here's where we ended up. >> >> The IdResolver is not designed in a manner that would allow alternative >> implementations to be used. Though, given that it is only used by the >> ResourceResolver implementations, those could be replaced by the user of >> the library >> >> The new code does use the Document.getElementById method to look up IDs >> in the general case. This may be overridden by explicitly placing >> ID-to-Element mappings into the IdResolver, but since doing this is >> neither documented nor shown in the examples it is fairly safe to assume >> this isn't happening out in the wild. >> >> Given the above, this code does not guard against any issues that may >> arise from duplicate IDs. >> >> In the case of signatures, there seems to be nothing that a library can >> do in order to ensure that the "right thing" is covered by the signature >> since the "right thing" is use-case specific. >> >> In the case of encryption, there is no way for the application to know >> whether the dereferenced encrypted encryption key is what the document >> author actually intended. If it's not, however, the worst that would >> happen is that encrypted data would not decrypt. >> >> Given this, I believe the following things should be done: >> - The use of Document.getElementById and the potential issue caused by >> the lack of a strong guarantee on ID uniqueness should be documented. >> - The documentation for XMLSignature and all the examples should be >> updated to make it clear that the user of the library needs to check >> that what they thought was covered by the signature actually was. >> - Attempting to update the examples in this way will make it clear that >> better APIs need to be provided to expose the content indicated by >> <Reference>s. >> >> In addition, given the manner in which the IdResolver currently works, I >> believe it could be done away with entirely. The >> IdResolver.registerElementById method could simply be replaced by the >> appropriate Element.setIdAttribute* method. Doing so would remove a >> potentially large in-memory data structure as well as various >> synchronization points that currently prevent concurrent ID resolutions. >> -- Chad La Joie www.itumi.biz trusted identities, delivered
