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

Problems parsing some BER encoding #11

Closed
isnotnick opened this issue Oct 3, 2016 · 9 comments
Closed

Problems parsing some BER encoding #11

isnotnick opened this issue Oct 3, 2016 · 9 comments

Comments

@isnotnick
Copy link

Thanks so much for this package, it's honestly saved me a lot of headache!

However - I've come across some BER encodings that it's having problems with, notably those with indefinite length encodings. The bottom-up search for the EOC blocks seems to fail, I think.

I've attached a sample - it's a SCEP request. (Generated, I think, by the JSCEP library - I've been toying with building a SCEP server for some time, and most other clients like iOS, SSCEP and so on don't seem to do the encoding quite like this does. The request is actually from a major MDM solution, but given the errors I've seen, I'm confident it's using JSCEP).

BER.txt

@fullsailor
Copy link
Owner

Hey Nick, your kind message means a lot. Thanks.

As for BER... yes, its problematic. lstoll also brought up an issue (#10) that sounds suspiciously the same. He wasn't able to provide a sample, but using yours I can reproduce and work out a fix. I can't recall why the code does a bottom-up search, but you may of pin-pointed the issue.

Thanks again!

@isnotnick
Copy link
Author

Wanted to add an update here, as I've been beating my head against this for a couple of weeks now!

I did hack something together that seems to work at handling the indefinite-length objects, but I've not tested enough yet.
Essentially I run 'readObject' twice - the first 'pass' only really cares about finding the indefinite-length markers and EOCs.

The total number of indef markers and their offsets, as well as the EOCs and offsets are stored in two slices.

If an indef is found, but no matching EOC is (bytes.LastIndex call) then instead of returning an error, it searches 'back', at lower offsets than the indef offset.

Then, the slices are re-ordered to pair up indef-length indicators and EOC indicators.

Finally, the main 'readObject' is run, and each time an indef is encountered, the offset of the next EOC is used.

This seems to work, but I've now got another problem...

asn1.Unmarshal doesn't seem to correctly handle compound octet-strings.
Line 170 in pkcs7.go attempts to unmarshal everything, but I think it's only doing the first octet-string that's generally 1000 bytes.

@isnotnick
Copy link
Author

Another update.

My final statement in that asn1.Unmarshal doesn't appear to handle 'compound' octet-strings appears to be true.
I put the two octet-strings comprising the pkcs7-data with 1200+ bytes in and get only the first 1000 out.

Your code correctly 'detects' the compound, so I'm testing a hack which - if there's only 1000 bytes returned and the input is larger - unmarshals and appends the rest of the octet-string.
It seems to work, and my testing with the major MDM provider is proving successful.

I'll tidy up the code for the BER-to-DER conversion and the unmarshaling of compound octet-strings and send you a pull request to review over the next few days.

@fullsailor
Copy link
Owner

Interesting that it detects the compound but doesn't rewrite it. I don't think DER allows compound strings, so that would be why Go's asn1 package won't parse it.

Any love you can give to the BER-to-DER code would be much appreciated.

@erning
Copy link

erning commented Dec 13, 2016

any update on this issue?

@Vespira
Copy link

Vespira commented Dec 21, 2016

Hi there,

I'm using micromdm/scep server and it uses pkcs7 lib as a dependency. At some point, when I send a CSR to enrol my Android client into the scep server, I have a ber2der: Invalid BER format error.

I created an issue (which is maybe a bug, maybe not ? we don't really know) here : micromdm/scep#20

And if someone make any progress on this, can you please post an answer ? it would be great !
I'm not a cryptography-pro, I try to learn, and dive deep to debug the unmarshalling process, and why it see it as invalid format, but so far I didn't find a cause.

@fullsailor
Copy link
Owner

I've merged #17 which we expect fixes this issue with indefinite lengths in BER encoding.

@mikkeloscar
Copy link

As reported by others we also saw AWS identity documents sometimes failing with the error: ber2der: BER tag length is more than available data.

After updating to latest master version it no longer fails on the one test document I had 👍

Thanks!

@fullsailor
Copy link
Owner

Super! Thank you for testing.

chrisccoulson pushed a commit to chrisccoulson/pkcs7 that referenced this issue Apr 25, 2020
Add unsigned attributes support
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

5 participants