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

Update pkcs7 library as a fix for ber2der error #32

Merged
merged 2 commits into from
Sep 27, 2021

Conversation

daemonsy
Copy link
Contributor

@daemonsy daemonsy commented Sep 18, 2021

What's up?

Been getting the occasional error for certain unlucky enrollments with certain Mdm-Signature.

err=ber2der: BER tag length is more than available data

When this happens, PUT /mdm returns a 400 response.

The fix?

Not sure, but after upgrading the PKCS7 library, replaying the same request works. Maybe it's this? mozilla-services/pkcs7#61

Regardless, reproduced that error happens in pkcs7.Parse() in main and upgrading the library solved it.

@jessepeterson
Copy link
Member

jessepeterson commented Sep 20, 2021

Thanks for this! Yes I've seen these errors intermittently, too. In my case it was interittent within a normal enrollment: i.e. if an enrolled device just checked-in again it would be fine. Was there any pattern that you were able to discern from those with the issues and those without? Discussing this with @w0de I suspect this is very similar to the issues mentioned micromdm/scep#79 that we've been tracking for a long time. My complete guess is that it's related to longer signatures that trigger this (because the fixes are all related to encoding lengths), but again: just a guess. It also may have been solved in omorsi/pkcs7#2 (the fork we were originally using).

@daemonsy
Copy link
Contributor Author

daemonsy commented Sep 20, 2021

Hey @jessepeterson, on my local machine, the enrollment that bumped into this had all the longer query command acknowledgements failing. So it does seem somewhat length related to length (assuming bigger payloads == bigger signatures, don't really know much about cryptography).

I was using a test machine with Monterrey when I bumped into this. Thought I had the cause, but when I generated a new CA for SCEP and tried it again, the error is gone. Tried re-enrollmment a few times, haven't bumped into it again :(

With the test in place, if you have the same result whereby it fails on the main branch, then maybe we can bisect the actual commit on https://github.com/mozilla-services/pkcs7 that fixes it?

@jessepeterson
Copy link
Member

jessepeterson commented Sep 20, 2021

I was using a test machine with Monterrey when I bumped into this. Thought I had the cause, but when I generated a new CA for SCEP and tried it again, the error is gone. Tried re-enrollmment a few times, haven't bumped into it again :(

Yes, my encounters with this were with iOS 15 beta and Monterey.

Yeah, again though even within the same enrollment I couldn't reliably reproduce this. I.e. just the next check-in or command or whatever it'd happily not reproduce it. I'm glad you captured a problematic instance though! Was on my todo list to write some sort of header capture tool. So, thanks!

With the test in place, if you have the same result whereby it fails on the main branch, then maybe we can bisect the actual commit on https://github.com/mozilla-services/pkcs7 that fixes it?

Go for it if you want. It's almost certainly the PR you suggested. Here's the changes between the diff you submitted and the version we were running; it's like 6 commits: mozilla-services/pkcs7@7259124...33d0574

I'm comfortable merging this. Give me a little bit of time. I'll also have to apply it to MicroMDM, too.

@daemonsy
Copy link
Contributor Author

Hey @jessepeterson how's it going with this? Wondering if you bumped into any time sinks / blockers with this. If so, whatever I can help :)

@jessepeterson jessepeterson merged commit fa8b812 into micromdm:main Sep 27, 2021
@jessepeterson
Copy link
Member

Apologies on the delay!

@daemonsy daemonsy deleted the ber2der branch September 28, 2021 01:03
@daemonsy
Copy link
Contributor Author

Thank you!

w0de pushed a commit to w0de/nanomdm that referenced this pull request Dec 14, 2021
* Update pkcs7 library as a fix for ber2der error
* Add a test case to confirm that the library upgrade worked
w0de pushed a commit to w0de/nanomdm that referenced this pull request Mar 17, 2022
* Update pkcs7 library as a fix for ber2der error
* Add a test case to confirm that the library upgrade worked
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.

2 participants