Skip to content

Conversation

@Gallaecio
Copy link

@Gallaecio Gallaecio commented Aug 31, 2016

Fixes #64. #52 could probably be considered fixed by this as well.

List of changes:

  • XMLSigner gains a new method, key_value_serialization_is_required(), which
    may be overriden in subclasses to force key value serialization regardless of
    the existence of a certificate chain.
  • The C14N algorithm transform is generated regardless of the signing method.
  • XMLProcessor becomes a new-style class.

These changes make it possible to define an SII-compliant XMLSigner subclass as
follows:

from signxml import methods, XMLSigner

class Signer(XMLSigner):

    def __init__(self):
        super(Signer, self).__init__(
            method=methods.detached,
            signature_algorithm='rsa-sha1',
            digest_algorithm='sha1',
            c14n_algorithm='http://www.w3.org/TR/2001/REC-xml-c14n-20010315')

    def key_value_serialization_is_required(self, cert_chain):
        return True

List of changes:
- XMLSigner gains a new method, key_value_serialization_is_required(), which
  may be overriden in subclasses to force key value serialization regardless of
  the existence of a certificate chain.
- The C14N algorithm transform is generated regardless of the signing method.
- XMLProcessor becomes a new-style class.

These changes make it possible to define an SII-compliant XMLSigner subclass as
follows:

    from signxml import methods, XMLSigner

    class Signer(XMLSigner):

        def __init__(self):
            super(Signer, self).__init__(
                method=methods.detached,
                signature_algorithm='rsa-sha1',
                digest_algorithm='sha1',
                c14n_algorithm='http://www.w3.org/TR/2001/REC-xml-c14n-20010315')

        def key_value_serialization_is_required(self, cert_chain):
            return True
@codecov-io
Copy link

codecov-io commented Aug 31, 2016

Current coverage is 94.94% (diff: 100%)

Merging #65 into master will decrease coverage by 0.46%

@@             master        #65   diff @@
==========================================
  Files             3          3          
  Lines           588        514    -74   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            561        488    -73   
+ Misses           27         26     -1   
  Partials          0          0          

Powered by Codecov. Last update f921c09...e1f287f

@kislyuk kislyuk force-pushed the master branch 2 times, most recently from fc2e4cf to 636ed68 Compare September 1, 2016 21:20
@kislyuk
Copy link
Member

kislyuk commented Sep 1, 2016

Thanks. Please add a test where key_value_serialization_is_required is overridden in a subclass.

@kislyuk
Copy link
Member

kislyuk commented Nov 30, 2019

Closing due to lack of response.

@kislyuk kislyuk closed this Nov 30, 2019
kislyuk added a commit that referenced this pull request 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

Successfully merging this pull request may close these issues.

Incompabilities with the SII schema

3 participants