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

Jump to bottom

crypto/x509: Support critical Apple extension for Development and Submission signing #10459

Closed
nathany opened this issue Apr 14, 2015 · 9 comments
Closed

crypto/x509: Support critical Apple extension for Development and Submission signing #10459

nathany opened this issue Apr 14, 2015 · 9 comments
Labels
FrozenDueToAge

Comments

@nathany
Copy link
Contributor

nathany commented Apr 14, 2015

What version of Go are you using (go version)?

go version devel +eced964 Tue Apr 14 17:19:36 2015 +0000 darwin/amd64

What operating system and processor architecture are you using?

darwin/amd64

What did you do?

I am using the Azure go-pkcs12 package to load a .p12 file exported from my Mac keychain. The PEM data from this library looks good and is without error, but X509KeyPair fails with:

x509: unhandled critical extension

package main

import (
    "crypto/tls"
    "encoding/pem"
    "io/ioutil"
    "log"

    "github.com/Azure/go-pkcs12"
)

func main() {
    p12, err := ioutil.ReadFile("Certificates.p12")
    if err != nil {
        log.Fatal(err)
    }

    blocks, err := pkcs12.ConvertToPEM(p12, "password")
    if err != nil {
        log.Fatal(err)
    }

    pemData := []byte{}
    for _, b := range blocks {
        pemData = append(pemData, pem.EncodeToMemory(b)...)
    }

    log.Println(string(pemData))

    // then use PEM data for tls to construct tls certificate:

    cert, err := tls.X509KeyPair(pemData, pemData)
    if err != nil {
        log.Fatal(err)
    }

    _ = cert
}

What did you expect to see?

A valid tls.X509KeyPair without error.

I would like to be able to load .p12 files directly to use with https://github.com/timehop/apns for push notifications. Currently I need to use OpenSSL (bindings) to do the conversion to a format Go understands.

What did you see instead?

x509: unhandled critical extension

This is failing because there are "Unknown extensions cause an error if marked as critical." (src)

In this case, the failing extension is 1.2.840.113635.100.6.1.2.0.0 APPLE_EXTENSION_ADC_APPLE_SIGNING which is flagged as critical.
@minux
Copy link
Member

minux commented Apr 14, 2015 via email

@nathany
Copy link
Contributor Author

nathany commented Apr 14, 2015

In that case, maybe there is a way I can strip out some extensions before sending it through to tls.X509KeyPair.

Or what would need to be done to support these extensions?

@bradfitz
Copy link
Contributor

Where is the definition of APPLE_EXTENSION_ADC_APPLE_SIGNING?

/cc @agl

@nathany
Copy link
Contributor Author

nathany commented Apr 14, 2015

Googling for "1.2.840.113635.100.6.1.2.0.0" turned up this source file.

I'll see if I can find some better info.

@nathany
Copy link
Contributor Author

nathany commented Apr 14, 2015

Here we go: http://images.apple.com/certificateauthority/pdf/Apple_WWDR_CPS_v1.0.pdf

Update: newer version of that document http://images.apple.com/certificateauthority/pdf/Apple_WWDR_CPS_v1.12.pdf

iPhone Software Development Signing (1.2.840.113635.100.6.1.2), critical (Development
Certificates)

I'm assuming it's marked critical because it is flagging this as a development certificate.

@nathany nathany changed the title crypto/x509: Support or skip Apple extensions? crypto/x509: Support or critical Apple extension for Development and Submission signing Apr 14, 2015
@nathany nathany changed the title crypto/x509: Support or critical Apple extension for Development and Submission signing crypto/x509: Support critical Apple extension for Development and Submission signing Apr 14, 2015
@agl agl self-assigned this Apr 14, 2015
@nathany
Copy link
Contributor Author

nathany commented Apr 14, 2015

In addition to critical extensions for iPhone certs:

iPhone Software Submission Signing (1.2.840.113635.100.6.1.4)
iPhone Software Development Signing (1.2.840.113635.100.6.1.2)

There are critical extensions for Safari, Mac, etc. listed in that document.

Extended Key Usage - Safari Extension Signing (1.2.840.113635.100.4.8)
Custom Extensions - Safari Extension Signing (1.2.840.113635.100.6.1.5)

But I'm not seeing information on what the values mean. My development cert contained Value:[]uint8{0x5, 0x0} but I don't know what that means. 😦

@agl
Copy link
Contributor

agl commented Apr 14, 2015

I think a reasonable change here might be for X509KeyPair to ignore errors in the certificate. As a server, it doesn't really matter to you what's in the certificate that you send to the clients except for sanity checks and for SNI matching.

Would that be sufficient for you?

Another option would be that unknown critical extensions were no longer a parse-time error, but rather caused validation to fail.

@nathany
Copy link
Contributor Author

nathany commented Apr 16, 2015

Upon further investigation, I think I was using the wrong cert, as a push notification cert doesn't have critical extensions based on the docs. I need to check with a colleague that I have the right certs and double check if this is still an issue for me. [Update: Confirmed, this isn't an issue for me right now, but it would be something nice to resolve for future use.]

Also, I think the error handling could be improved. If instead of:

x509: unhandled critical extension

It could include the ID

x509: unhandled critical extension (1.2.840.113635.100.6.1.2.0.0)

It would've been a lot easier to hunt down the issue.

@gopherbot
Copy link

CL https://golang.org/cl/9390 mentions this issue.

@agl agl closed this as completed in d942737 Apr 28, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
@rsc rsc unassigned agl Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@bradfitz @nathany @agl @minux @gopherbot