-
-
Notifications
You must be signed in to change notification settings - Fork 107
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
Support for signing non XML data #238
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution. To continue, we would need some documentation with examples, references to relevant parts of the XML Signature standard, test cases, and a reference to an interop counterparty implementation (another implementation of XML Signature that you intend to use this with and can confirm that it works with the result).
xml_node = etree.fromstring(data, parser=self.parser, **kwargs) | ||
for entity in xml_node.iter(etree.Entity): | ||
raise InvalidInput("Entities are not supported in XML input") | ||
except etree.XMLSyntaxError as e: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not safe. In general, we should not ignore XML syntax errors, should not try to parse arbitrary binary data as XML, and should not return from an except block.
Any configuration that references a binary data blob should be direct and explicit, not implicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get it. But I'm struggling to find a way to mark the data as a particular content type, so it would be treated properly in various places of the code.
So what would you think about adding a non_xml
boolean parameter to sign
method and propagating it through _unpack
, get_root
, etc. methods to _fromstring
invocations? I don't see a clear way to attach that info to the data variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll think about the best way to organize this more over the weekend and come back with a recommendation here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just thought about another way of solving this problem. What would you think about adding digest
parameter? You would sign with either data
or digest
filled out. With digest
set, code would skip all the canonicalization and digesting logic and just use the passed value in reference.
That way it would be feasible to sign even huge documents, with digests obtained externally.
Hello! Right now I'm trying to sign arbitrary ASCII data needed to prepare requests for a bank SOAP API. If this is out of scope for the library, please let me know :) Unfortunately, this API is not public. From the scraps of documentation I gathered, it's implemented in .net (but the guys I'm working with don't seem to know specifics either). In a nutshell, I'm trying to get a detached XAdES signature for a provided byte-stream (https://www.w3.org/TR/xmldsig-core1/#def-SignatureDetached) |
OK, thanks for those details. Yes, this application is generally within the scope of this library and I understand the constraints regarding the non-public API, and that you may not know which XML Signature implementation the counterparty uses. However, we still need examples and test cases to proceed. |
I understand, I'll add them. Is it better to keep this PR as it is now and slowly fix it or should I retract it and submit it later, in a more complete form? |
I don't mind keeping this PR open, but I'm fine either way. Please do what works best for your workflow. Looking forward to adding this change. Thanks. |
7e7f504
to
b3de531
Compare
SignXML only supports signing XML files. This pull-request adds support for signing non-XML data, useful for example for signing binary files or arbitrary byte streams.