Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide an option to ignore missing URIs #67

Closed
dominykas opened this issue Aug 15, 2015 · 5 comments
Closed

Provide an option to ignore missing URIs #67

dominykas opened this issue Aug 15, 2015 · 5 comments

Comments

@dominykas
Copy link

I'm working on a library to verify ASiC containers - they are essentially a ZIP with a signatures.xml. The signatures.xml contains XAdES which can happily be verified using this library, except that it contains URIs which are outside of the document itself, i.e. the URIs do not reference the IDs inside the XML, but reference the files inside the ZIP. Since URIs with and without the starting # can be treated as reference IDs, I don't see an easy way to automagically determine which is which, therefore I need a way to tell this library to ignore missing elements. I think a good way to achieve that would be to have an option that instead of failing here: https://github.com/yaronn/xml-crypto/blob/master/lib/signed-xml.js#L284 would rather stuff the information into some "missing URIs" property, that can later be used to verify the documents externally.

In short - I'd like to see a way to treat missing URIs as warnings, not as failures.

I'm happy to submit a PR, if we can agree on the contract - where to add the option and how to name the "missing URIs" property. What do you think about options.failOnMissingUris [default: true] and SignedXml.validationWarnings (array of objects with warningType and custom data which differs per type) or a simpler SignedXml.validationMissingUris?

Reference:
ASiC: http://www.etsi.org/deliver/etsi_ts/102900_102999/102918/01.03.01_60/ts_102918v010301p.pdf
XAdES: http://www.etsi.org/deliver/etsi_ts%5C101900_101999%5C101903%5C01.04.02_60%5Cts_101903v010402p.pdf

@bjrmatos
Copy link
Contributor

basically what you're asking is support for validate detached signatures, external signatures that are not part of the signed document.

this feature is planned, see #66

checkSignature will support a new argument to specify the external signature document, and the validation will resolve the references correctly (we are investigating how to make this)

your solution seems a little confusing to use correctly, so until we can support detached signatures the best you can do is to modify xml-crypto to meet your needs :(

@dominykas
Copy link
Author

It's not exactly an "external signature document" per se - there are multiple files to be checksumed, and they're all inside the ASiC container (which means there's multiple streams to be handled [careful not to cross them], and could potentially be done over network, etc etc) - I'm not sure it's really a good idea to make xml-crypto aware of such (or any other) containers, or at least it feels to me that it wouldn't be the node way :)

As for using correctly - I'm suggesting to keep the default as is. As for non-default behavior - a lot of the "using correctly" part would come from properly naming the options - maybe strictReferenceValidation, allowExternalUris, ignoreExternalUris, ignoreMissingReferences is all that's needed. As for the second part - getting a list of what was ignored - for my current purposes actually the current approach is OK - all information gets stuffed into validationErrors anyways.

Even the implementation is not that tricky:

Like I said - I'm happy to PR (and I'd be somewhat less happy to maintain my own fork just for this...)

@dominykas
Copy link
Author

Oh, another way to achieve what I need would be to have some configurable getHashByUri method - in which case I could provide the result from my program (by reading and checksumming the container streams that I have).

@bjrmatos
Copy link
Contributor

I need to read the ASiC and XAdES specifications to see if this is the best solution.

But maybe @yaronn knows more about this than me

@yaronn would you like to comment on this?

@cjbarth
Copy link
Contributor

cjbarth commented May 29, 2023

Closing due to inactivity; reply to reopen.

@cjbarth cjbarth closed this as completed May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants