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

crypto/x509: unable to parse certificate parsable by Java #33259

Open
tomjamescn opened this issue Jul 24, 2019 · 4 comments
Open

crypto/x509: unable to parse certificate parsable by Java #33259

tomjamescn opened this issue Jul 24, 2019 · 4 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@tomjamescn
Copy link

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

$ go version
go version go1.11.5 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/godev/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/godev/go_workspace"
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/golang"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/golang/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build357830024=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I have a pem which can parsed correctly in java, such as code below:

package com.mytest;


import java.io.ByteArrayInputStream;
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;


public class Test {
    public static void main(String[] args) {

        try {


            String pem = "-----BEGIN CERTIFICATE-----\n" +
                    "MIID7jCCAtigAwIBAgIBATALBgkqhkiG9w0BAQswHTEbMBkGA1UEAxMSSHVhd2Vp\n" +
                    "IEtleVN0b3JlICAgMB4XDTE5MDcyNDA4NTQ0NFoXDTI5MDcyNDA4NTQ0NFowGjEY\n" +
                    "MBYGA1UEAxMPQSBLZXltYXN0ZXIgS2V5MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A\n" +
                    "MIIBCgKCAQEA8WS+DnC9JzGytDPpe3/GVY4xQx0bsPVP1Drf0n3eD+wq6U91QnjX\n" +
                    "vhyRXtguDBu8OM5Qc5h8wFIOAUTD4+U/QGLQ3pZN+DXVmwlSJjbx8yMjuiUwInhG\n" +
                    "uJ+xzhuEsNFCdxdyaNPmGhUycu09olL2/mcgDQFxusfr5jnnhU9VFv3/x1Y+7mVh\n" +
                    "kICFnCE3YQ4ufHOHroQIE0kxnfJF+DkgK1tkdIMHEvX5rJvaqtd0M7RY/PRrE0dE\n" +
                    "bYSlAlSAVqdjfLb0LF8tRTcjnIh4lg5NhqCWeAkGw8egUaNW1vFX6SeXrW6uJv2N\n" +
                    "OeHxi9VFG7ymaOqB4URS0wGQ6dzoYsx/iwIDAQABo4IBPjCCATowCwYDVR0PBAQD\n" +
                    "AgAAMAgGA1UdHwQBADCB9QYKKwYBBAHWeQIBEQSB5jCB4wIBAgoBAQIBAwoBAQR3\n" +
                    "eyJjcHVfaWQiOiJIVUFXRUlfSFdITUFfNmYwNzk3ZTQtMWFjNS00YjM3LWEyZTgt\n" +
                    "YzY2ZjQ2MTM0YjhlLTIyMzE5ZjcxIiwiY291bnRlciI6MTMsInVpZCI6IjEwNDE3\n" +
                    "IiwicnNhX3Bzc19zYWx0bGVuIjozMn0EADASv4N3AgUAv4U9CAIGAWwjMJ1KMEah\n" +
                    "CTEHAgUA/wEAAaIDAgEBowQCAggApQUxAwIBBKYFMQMCAQO/gUgFAgMBAAG/hT4D\n" +
                    "AgEAv4VBBQIDAV+Qv4VCBQIDAxSxMCkGCSsGAQQBj1seAgEBAAQZMBcCAQCiAwEB\n" +
                    "Ab+BSAswCaEHAwUABoAAgDALBgkqhkiG9w0BAQsDggEBAFZRjVpDqujJrwaZqycw\n" +
                    "VgrM/J1b2VcVKUPJ39eJXs2S/ur7yUlgSxRcpOufa3IF0XekOUyHTNIroWUz/xLb\n" +
                    "X6pv32PCMeavI/6ldl/zEJyy11PKX8ZrVfE05WiWUIJ6BwmDX+RtNjJSJ/xmwfDu\n" +
                    "dn0CAx5apWsCMYpcGXQ2g8DRGQpYVdJS/aOTlDHGdSdSesx0TbGL39gjfdDb851L\n" +
                    "spVFtcdoxw5nb0obwRItPBF+gHIh3xsYGGDN/EKSNN9YMja4MzgjTeLjNXjs1pXO\n" +
                    "f4Fm3OiOfSFnTJuJk/rKQ0TiW+p3EvuZ+tRT+iffJvhdvDAIp7I3pJjaoZw4xwHH\n" +
                    "Tr8=\n" +
                    "-----END CERTIFICATE-----";

            CertificateFactory factory = CertificateFactory.getInstance("X.509");
            X509Certificate askCertificate = (X509Certificate) factory.generateCertificate(new ByteArrayInputStream(pem.getBytes()));

        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

But in Golang, the code below will return err:

package main

import (
	"crypto/x509"
	"encoding/pem"
	"fmt"
)

func main() {
	certPEM := `-----BEGIN CERTIFICATE-----
MIID7jCCAtigAwIBAgIBATALBgkqhkiG9w0BAQswHTEbMBkGA1UEAxMSSHVhd2Vp
IEtleVN0b3JlICAgMB4XDTE5MDcyNDA4NTQ0NFoXDTI5MDcyNDA4NTQ0NFowGjEY
MBYGA1UEAxMPQSBLZXltYXN0ZXIgS2V5MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A
MIIBCgKCAQEA8WS+DnC9JzGytDPpe3/GVY4xQx0bsPVP1Drf0n3eD+wq6U91QnjX
vhyRXtguDBu8OM5Qc5h8wFIOAUTD4+U/QGLQ3pZN+DXVmwlSJjbx8yMjuiUwInhG
uJ+xzhuEsNFCdxdyaNPmGhUycu09olL2/mcgDQFxusfr5jnnhU9VFv3/x1Y+7mVh
kICFnCE3YQ4ufHOHroQIE0kxnfJF+DkgK1tkdIMHEvX5rJvaqtd0M7RY/PRrE0dE
bYSlAlSAVqdjfLb0LF8tRTcjnIh4lg5NhqCWeAkGw8egUaNW1vFX6SeXrW6uJv2N
OeHxi9VFG7ymaOqB4URS0wGQ6dzoYsx/iwIDAQABo4IBPjCCATowCwYDVR0PBAQD
AgAAMAgGA1UdHwQBADCB9QYKKwYBBAHWeQIBEQSB5jCB4wIBAgoBAQIBAwoBAQR3
eyJjcHVfaWQiOiJIVUFXRUlfSFdITUFfNmYwNzk3ZTQtMWFjNS00YjM3LWEyZTgt
YzY2ZjQ2MTM0YjhlLTIyMzE5ZjcxIiwiY291bnRlciI6MTMsInVpZCI6IjEwNDE3
IiwicnNhX3Bzc19zYWx0bGVuIjozMn0EADASv4N3AgUAv4U9CAIGAWwjMJ1KMEah
CTEHAgUA/wEAAaIDAgEBowQCAggApQUxAwIBBKYFMQMCAQO/gUgFAgMBAAG/hT4D
AgEAv4VBBQIDAV+Qv4VCBQIDAxSxMCkGCSsGAQQBj1seAgEBAAQZMBcCAQCiAwEB
Ab+BSAswCaEHAwUABoAAgDALBgkqhkiG9w0BAQsDggEBAFZRjVpDqujJrwaZqycw
VgrM/J1b2VcVKUPJ39eJXs2S/ur7yUlgSxRcpOufa3IF0XekOUyHTNIroWUz/xLb
X6pv32PCMeavI/6ldl/zEJyy11PKX8ZrVfE05WiWUIJ6BwmDX+RtNjJSJ/xmwfDu
dn0CAx5apWsCMYpcGXQ2g8DRGQpYVdJS/aOTlDHGdSdSesx0TbGL39gjfdDb851L
spVFtcdoxw5nb0obwRItPBF+gHIh3xsYGGDN/EKSNN9YMja4MzgjTeLjNXjs1pXO
f4Fm3OiOfSFnTJuJk/rKQ0TiW+p3EvuZ+tRT+iffJvhdvDAIp7I3pJjaoZw4xwHH
Tr8=
-----END CERTIFICATE-----`
	block, _ := pem.Decode([]byte(certPEM))
	if block == nil {
		panic("failed to parse certificate PEM")
	}
	cert, err := x509.ParseCertificate(block.Bytes)
	if err != nil {
		panic("failed to parse certificate: " + err.Error())
	}
	fmt.Println(cert)
}

What did you expect to see?

I think x509 implement would be the same and golang code could parse certificate correctly.

What did you see instead?

golang output is:
panic: failed to parse certificate: asn1: syntax error: truncated tag or length

@Freeaqingme
Copy link

How was the certificate generated?

@andybons andybons changed the title golang x509 implement is different from java ? crypto/x509: unable to parse certificate parsable by Java Jul 24, 2019
@andybons andybons added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jul 24, 2019
@andybons andybons added this to the Unplanned milestone Jul 24, 2019
@tomjamescn
Copy link
Author

tomjamescn commented Jul 25, 2019

@Freeaqingme This is a certificate generated from third-party sdk and the generate code is closed source.

I think this post will help: https://groups.google.com/forum/#!topic/golang-nuts/SCzlQPNfURk
it pointed out that the certificate is not qualified and java, python and openssl command line can parse it.

Should crypto/x509 be lenient in parsing ?

@lucasmoten
Copy link

Looking at that other thread, in the golang-nuts

I think the extensions in your certificate might be invalid. Namely CRL Distribution Points: 2.5.29.31
Per the RFC https://www.ietf.org/rfc/rfc5280.txt, the cRLDistributionPoints has to respect a certain definition and is sequence of distributionPoint(s). In your case it seems to be empty.

The relevant code here is x509.ParseCertificate
https://github.com/golang/go/blob/master/src/crypto/x509/x509.go#L1497

If e.Value is a single byte, of value 0 as described, then attempt to Unmarshal would fail when it calls into parseTagAndLength
https://github.com/golang/go/blob/master/src/encoding/asn1/asn1.go#L524

It appears that some leniency was added when iterating the array of distributionPoint per line 1504 of x509.go checking name. Perhaps some upfront checks on this field, and possibly others, for "empty value" type conditions could be added for leniency. Alternatively, giving clarity on the "truncated tag or length" for this specific condition in x509.go

@rolandshoemaker
Copy link
Member

I don't think it makes sense to add an exception here, the provided certificate is very badly encoded. I'm actually quite surprised Java will happily parse it.

The cRLDistributionPoints extension value here is just a single byte, 0 (not even ASN.1 NULL), rather than, say, an empty sequence (which would also be invalid given the 5280 definition, but somewhat more understandable to mess up).

@ALTree ALTree added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

6 participants