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

RSAKeyValue tag (Modulus and Exponent) #52

Closed
Danisan opened this issue May 21, 2016 · 7 comments
Closed

RSAKeyValue tag (Modulus and Exponent) #52

Danisan opened this issue May 21, 2016 · 7 comments

Comments

@Danisan
Copy link

Danisan commented May 21, 2016

Since these values are not provided in the sign() return value, which should be the best way to add them?

Thanks

@kislyuk
Copy link
Member

kislyuk commented May 21, 2016

They should be. How are you calling sign()?

@Danisan
Copy link
Author

Danisan commented May 22, 2016

They are missing in the result. this way:

def sign_full_xml(self, message, privkey, cert, uri):
        # canocalization REC-xml-c14n-20010315
        # Algorithm = u'http://www.w3.org/TR/2001/REC-xml-c14n-20010315'
        doc = etree.fromstring(message)
        signed_node = xmldsig(
            doc, digest_algorithm=u'sha1').sign(
            method=methods.enveloped, algorithm=u'rsa-sha1',
            c14n_algorithm=u'http://www.w3.org/TR/2001/REC-xml-c14n-20010315',
            reference_uri='#'+uri,
            key=privkey.encode('ascii'),
            cert=cert.encode('ascii'))

@kislyuk
Copy link
Member

kislyuk commented May 23, 2016

The reason is that you're giving it a value for cert. The method currently assumes that if you're passing cert, then you intend to serialize its contents using X509Data, not using RSAKeyValue. The x.509 base64 encoded cert contains the key info encoded in it (and is generally what people use nowadays instead of RSAKeyValue).

Try not passing cert and see if that accomplishes your goal. Otherwise, please describe your use case and I'll see if I can accommodate it.

@Danisan
Copy link
Author

Danisan commented May 24, 2016

Ok I'll try... will tell you what happen very soon. It is related to en electronic invoice requirement. Thanks a lot.

@Danisan
Copy link
Author

Danisan commented May 25, 2016

Andrey, I called "sign()" without cert. Now there come "modulus" and "exponent". I still did not reach the fire-test anyway, but think I should pass the verify first..
These are the ways I tried calling verify() method, unsuccessfully:
verified_data = xmldsig(signed_node).verify()
verified_data = xmldsig(signed_node).verify().signed_xml
verified_data = xmldsig(signed_node).verify(x509_cert = cert)
or
verified_data = xmldsig(signed_node).verify(x509_cert = cert).signed_xml
I think the x509 certificate should be passed, but is not clear to me what "signed_xml" stands for, since in the readme, it is not previously refferenced as a variable.

@Gallaecio
Copy link

Gallaecio commented Aug 30, 2016

According to the XSD of XML signatures of the SII (Chile's IRS), both elements should be on the signature, KeyValue and X509Data.

@kislyuk
Copy link
Member

kislyuk commented Aug 30, 2016

@Gallaecio, to your original question, the XML Security spec does not say anything about the requirement for both elements to be present. On the other hand, having both elements be present results in an ambiguity and/or extra work for the validator, and makes it easier for vulnerabilities to arise - the public key used to sign the document is already encoded in the cert (which is in X509Data). So the verifier must either ignore KeyValue or make sure it matches what's in the cert.

kislyuk added a commit that referenced this issue Nov 30, 2019
- XMLSigner.sign(): add always_add_key_value kwarg to include both
  X509Data and KeyValue for ill-defined signing applications

- XMLVerifier.verify(): reject signatures that contain both X509Data
  and KeyValue by default; add ignore_ambiguous_key_info kwarg to
  bypass

- Add security warnings in docs

Fixes #52
Fixes #65
Fixes #64
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

No branches or pull requests

3 participants