RobotsAndPencils / buford

A push notification delivery engine for the new HTTP/2 APNS service.
MIT License
475 stars 52 forks source link

SSL Certificate decode error (pkcs12: expected exactly two safe bags in the PFX PDU) #8

Open tipycalFlow opened 8 years ago

tipycalFlow commented 8 years ago

Hey,

I seem to have run into a certificate issue as I get the error: pkcs12: expected exactly two safe bags in the PFX PDU. I'm generating the Production Apple Push Notification Service SSL cert as described in Apple's docs and I had simply exported the certificate/private-key pair from my keychain to get a .p12 file and I'm using this p12 file's path as filename in line cert, err := certificate.Load(filename, password) of the package's example. The error is returned by pkcs12 package in the Decode method of cert.go and specifically, by pkcs12.Decode. Would you happen to know why this error occurs or maybe what steps need to be taken to get the correct p12 file?

nathany commented 8 years ago

@idyll What needs to be done to remove the duplicates from a p12 file?

nathany commented 8 years ago

When you export the p12 from Keychain, be sure to select both the certificate and the private key.

If I only select the private key, I get the following error: pkcs12: expected exactly two items in the authenticated safe

Does this resolve the issue for you?

Reference: Installing a Client SSL Signing Identity on the Server https://developer.apple.com/library/watchos/documentation/IDEs/Conceptual/AppDistributionGuide/AddingCapabilities/AddingCapabilities.html

tipycalFlow commented 8 years ago

I tried recreating the cert from scratch and it still logs the same error. I edited the file pkcs12.go and logged the number of safe bags and surprisingly got 4 instead of the expected 2!

I checked the cert from my command line: keytool -list -storetype pkcs12 -keystore app_id_1.p12 And got this reply: Keystore type: PKCS12 Keystore provider: SunJSSE

Your keystore contains 1 entry

aakash chaudhary, Jan 7, 2016, PrivateKeyEntry, Certificate fingerprint (SHA1): xxxxxxxxxxxxxxxxx

tipycalFlow commented 8 years ago

Well this is weird! I edited the pkcs12.go file locally and commented out the safe bags' validation (line 219) just to test: //if len(bags) != 2 { // err = errors.New("pkcs12: expected exactly two safe bags in the PFX PDU") // return //}

and voila! Notification sent! Maybe the certificate Apple generates is not compliant with pkcs12 but I think I'll use the edited version of pkcs12.go for the time being. Now the only thing left to test is whether the same TLS connection is being reused or not? Apple have specifically mentioned this in their docs to prevent being considered as a DDOS attack, so this is important too.

nathany commented 8 years ago

Glad to hear you found a workaround, though I'm still not sure why it was necessary.

tipycalFlow commented 8 years ago

I forgot to mention that I had later tried to use only the public certificate (and not public cert and private key exported together as mentioned in Apple's docs) and got it to work without commenting out the safe-bags validation in crypto/pkcs12.go. The weird thing is that Apple has mentioned (Step 4: Select both the certificate and the key, and choose File > Export Items. under Installing a Client SSL Signing Identity on the Server) to export both public cert and private key while the tests suggest that the private key is not required for authenticating the requests! I thought I'd add this info for other developers who will likely stumble upon this if following Apple's docs. I've raised a bug with the crypto team as well: https://github.com/golang/go/issues/14015

nathany commented 8 years ago

Thanks for logging a bug with the crypto team.

It's odd that I had a somewhat opposite experience https://github.com/RobotsAndPencils/buford/issues/8#issuecomment-169464471

KirkAtLarc commented 8 years ago

Same thing that tipycalFlow reported here. Following the Apple Guide and exporting both the cert and private key resulted in the same two bags error but exporting just the cert got past that.

I noticed that a couple of the buford source files, e.g. push/client.go and certificate/cert.go, import "golang.org/x/net/http2" or "golang.org/x/crypto/pkcs12". I'd expect that the Go 1.6 release includes those packages without the "x/" part in their paths. Is that done to explicitly enable the features for older Go versions?

nathany commented 8 years ago

There is an issue to support more than two bags for pkcs12, but I haven't had a chance to work on it yet. https://github.com/golang/go/issues/14015

As to your other question, x/crypto/pkcs12 is not included in Go 1.6 and still must be imported from there. There aren't any plans to bring it into the standard library.

In my tests the current x/net/http2 client doesn't work with Go 1.5, so we're not importing it for that.

We are pulling in x/net/http2 because of a bug in Go 1.6 (see #26). Though the bug is fixed for Go 1.6.1, we will probably still need to pull in x/net/http2 because we are using an Apple signed certificate to establish the HTTP/2 connection (rather than the default certs). This requires that we reassure net/http that we want HTTP/2, and right now that requires x/net/http2.

Maybe API could be added to Go 1.7 to allow the equivalent of http2.ConfigureTransport(transport) or maybe there is a way to do this already that I'm not aware of. /cc @bradfitz.

element-of-surprise commented 5 years ago

This still seems like it is a problem. I've been using a system that output pks12 and it has the same problem.

In my case, I found that if in Decode() I:

I get what I'm looking for. In the same way Apple does, I seem to be getting 4 bags.

Here's exactly what I did: `/* if len(bags) != 2 {

         err = errors.New("pkcs12: expected exactly two safe bags in the PFX PDU")
            return
    }
    */

for i := 0; i < 2; i++ { bag := bags[i] .... `

I don't know enough about how PKCS12 works to understand the problem between what is in the file and this library to know what is going on. But I wanted to indicate this is a problem for other users.

nathany commented 5 years ago

Maybe should switch to https://github.com/SSLMate/go-pkcs12?

fpawel commented 4 years ago

Same problem and same solution. Thanks to @element-of-surprise and @tipycalFlow I wonder - what could it be? Incorrect interpretation of rfc?

rr-tomas-henriquez commented 2 years ago

if using https://github.com/SSLMate/go-pkcs12 you should be probably using the DecodeChain instead, the Decode alone is not expecting the whole chain but only the key and main cert.