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

Support for signing non XML data #238

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

mwegrzynek
Copy link

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.

Copy link
Member

@kislyuk kislyuk left a 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:
Copy link
Member

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.

Copy link
Author

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.

Copy link
Member

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.

Copy link
Author

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.

@mwegrzynek
Copy link
Author

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

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)

@kislyuk
Copy link
Member

kislyuk commented Jan 26, 2024

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.

@mwegrzynek
Copy link
Author

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?

@kislyuk
Copy link
Member

kislyuk commented Jan 26, 2024

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.

@kislyuk kislyuk force-pushed the develop branch 4 times, most recently from 7e7f504 to b3de531 Compare September 20, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants