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

Error when parsing some real world OCSPResponses in Extensions (OctetString received instead of ParsableOctetString) #85

Open
erny opened this issue Apr 5, 2018 · 7 comments

Comments

@erny
Copy link

erny commented Apr 5, 2018

I got errors when parsing OCSP Responses in CADES / CMS attributes and running .native.
It comes down to the definition of 'extn_value' as 'ParsableOctetString' in ResponseDataExtension:

class ResponseDataExtension(Sequence):
    _fields = [
        ('extn_id', ResponseDataExtensionId),
        ('critical', Boolean, {'default': False}),
        ('extn_value', ParsableOctetString),       # Replace with Any type makes error go away
    ]

    _oid_pair = ('extn_id', 'extn_value')
    _oid_specs = {
        'nonce': OctetString,
        'extended_revoke': Null,
    }

The error I get:

>>> unsigned_attrs['certificate_revocation_values'][0]['ocsp_vals'][0]['tbs_response_data']['response_extensions'][0]['extn_value']
*** ValueError: Insufficient data - 115 bytes requested but only 16 available
    while parsing asn1crypto.core.ParsableOctetString
    while parsing asn1crypto.ocsp.ResponseDataExtension

When looking at the data we can see that it's not an DER encoded OctetString (not ParsableOctetString) but a normal octet string:

>>> unsigned_attrs['certificate_revocation_values'][0]['ocsp_vals'][0]['tbs_response_data']['response_extensions'][0].dump()
'0\x1d\x06\t+\x06\x01\x05\x05\x070\x01\x02\x04\x10%q\x93;j\xe5\xcd&\x9a\x80a\x8e\xdf\x85\x85\xcd'

The interesting part here is the extn_value:

'x04\x10%q\x93;j\xe5\xcd&\x9a\x80a\x8e\xdf\x85\x85\xcd'

How could I ignore the error or make an option to avoid it? e.g.:

ContentInfo.load(..., relax_parsable_octet_strings=True)

E.g., whenever an OctetString is requested in ParsableOctetString.parse, self.bytes() must start with '\x04'. If it's not the case, the self._header could be prepended.

Any suggestions?
Thanks in advance.

erny added a commit to erny/asn1crypto that referenced this issue Apr 5, 2018
erny added a commit to erny/asn1crypto that referenced this issue Apr 5, 2018
erny added a commit to erny/asn1crypto that referenced this issue Apr 5, 2018
@wbond
Copy link
Owner

wbond commented Apr 17, 2018

This looks very similar to #56. I do believe the ASN.1 data you are reading is technically invalid, but it seems that there is at least one popular OCSP responder out there that is encoding the data incorrectly, so we'll probably have to find a way around that.

@erny
Copy link
Author

erny commented May 11, 2018

Hi. It would be cool if we had a way to register error handlers for parsing errors.

register_error_handler(ObjectType_or_ObjectTypeTuple, function)

The function should be called with something like this:

function(asn1data, ObjectTypeWhichHadParsingErrors)

Some of the outcomes could be:

  • Retry with X type
  • Retry with corrected asn1data
  • I'll parse this on my own, returns an already parsed object
  • Raise user exception

@joernheissler
Copy link
Collaborator

Best to track down which software generates the bad ASN.1 and have it fixed. Adding workaround to decoders only encourages encoders to implement even more bugs, forcing other decoders to add the workaround too ;-)
If it's a private CA, fix it yourself.
If it's a public CA and they refuse to fix it, complain to CAB forum or browser vendors because BRs might be violated.

Would such an error handler be reliable? Could there be no decoding error by accident but still a wrong result?

It might be possible to add a generic callback that's called every time a value is about to be decoded. This can be used to work around all types of mistakes.

Or you can decode your ocsp response as a Sequence, fix the problem and parse again as OCSPResponse.

@erny
Copy link
Author

erny commented Jul 3, 2018

@joernheissler I'll try to investigate it a bit more, when I find the time. We receive the response through an intermediate service (of spanish's government). I guess that's a problem of one of approved Trust Service Providers.

IMHO, I consider it very interesting to have a custom error handler to avoid the need of patching the library.

@sheepsy90
Copy link

sheepsy90 commented Sep 19, 2018

I came across something similar now in the swedish BankID system. If I am not mistaken when trying to parse and go through the OCSP response on calling

basic_ocsp_response['tbs_response_data']['response_extensions'][0]['extn_value']

it crashes with

ValueError: Insufficient data - 1599800026821371669022217068972140223656264339565228101802544028419084878 bytes requested but only 32 available while parsing asn1crypto.core.ParsableOctetString while parsing asn1crypto.ocsp.ResponseDataExtension

@sheepsy90
Copy link

Actually when looking at the raw data it actually seems to be correct.
This is the byte stream (in ints that I analyzed myself)
It always includes the TAG, LENGTH, PAYLOAD
[6, 9, 43, 6, 1, 5, 5, 7, 48, 1, 2] [1, 1, 255] [4, 32, 102, 247, 231, 203, 225, 136, 203, 52, 189, 140, 20, 211, 111, 117, 38, 130, 58, 85, 148, 254, 152, 24, 35, 238, 115, 16, 215, 227, 144, 123, 209, 213]

@erny
Copy link
Author

erny commented Sep 25, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants