Skip to content

Conversation

@dariocast
Copy link
Contributor

At the moment if an issuerSigned object contains issuerAuth whose unprotected header contains a list of certificate the verification fails.

Specifically the problem is related on how pycose handle the list and how it create the MsoVerifier.object.uhdr element. I report here the actual pycose method to parse the headers used while creating MsoVerifier:

@classmethod
    def _parse_header(cls, hdr, allow_unknown_attributes: bool) -> dict:
        decoded_hdr = {}
        for k, v in hdr.items():
            attr = CoseHeaderAttribute.from_id(k, allow_unknown_attributes)

            if hasattr(attr, 'value_parser'):
                decoded_hdr[attr] = attr.value_parser(v)
            else:
                decoded_hdr[attr] = v

        return decoded_hdr

This parsing results in a dictionary with one Pycose X5Chain element with the list of certificates as value. This breaks the actual implementation of pyMdocCbor to load raw public keys.

This commit support the pycose parsing and solves issue #23

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for verifying Issuer Signed objects that contain certificate chains in their unprotected headers. The main issue was that pycose's header parsing creates dictionary structures that broke the existing implementation's ability to extract raw public keys from X.509 certificates.

  • Handles different data types (bytes, list, dict) when extracting public keys from certificate chains
  • Updates the method for merging protected and unprotected headers to avoid union operation issues
  • Adds cbor2 dependency for enhanced CBOR parsing capabilities

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.

File Description
requirements-dev.txt Adds cbor2 dependency with version constraints
pymdoccbor/mso/verifier.py Updates raw_public_keys method to handle multiple certificate data types and fixes header merging

for h, v in _mixed_heads:
if h.identifier == 33:
return list(self.object.uhdr.values())
# return list(self.object.uhdr.values())
Copy link

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented-out code instead of leaving it in the codebase. This clutters the code and reduces readability.

Suggested change
# return list(self.object.uhdr.values())

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@PascalDR PascalDR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me.

Can you please apply the changes suggested by copilot, in particular removing the commented code?

@dariocast
Copy link
Contributor Author

I added commit to solve issues arisen by copilot

@dariocast dariocast requested a review from PascalDR July 23, 2025 16:02
@peppelinux peppelinux merged commit 4b007be into IdentityPython:main Jul 24, 2025
3 checks passed
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.

3 participants