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/tls: Client Side TLS Authentication fails for certs with long fields #36467

Open
antevens opened this issue Jan 8, 2020 · 5 comments
Open
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@antevens
Copy link

antevens commented Jan 8, 2020

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

go version go1.13.4 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
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user/Revisions/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.13"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.13/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/user/go/src/helm.sh/helm/go.mod"
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-build386574731=/tmp/go-build -gno-record-gcc-switches"

What did you do?

package main

import (
"crypto/tls"
"fmt"
)

func main() {
cert, err := tls.LoadX509KeyPair("./helm-test-crt", "./helm-test.key")
fmt.Println(cert, err)
}

Test certs:
https://gist.github.com/antevens/a05409165d33bd771e39bc219fe2c6bb/archive/e56cf114dee64a6b367abf08ab1c34a405a62f88.zip

helm/helm#7343

What did you expect to see?

Client side TLS authentication succeed.

What did you see instead?

asn1: structure error: base 128 integer too large

@mikedanese
Copy link
Contributor

mikedanese commented Jan 9, 2020

See also #19933

Where did the OIDs in this cert come from? FWIW, derdump fails on it too:

$ derdump -i helm-test-crt 
C-0x0D  (45)
   C-0x0D  (45)
derdump: error -8183: SEC_ERROR_BAD_DER: security library: improperly formatted DER-encoded message.
derdump: errno=2: No such file or directory

@antevens
Copy link
Author

antevens commented Jan 9, 2020

Hi Mike, thanks for responding.

The certs and by extension the OIDs are created by a Foreman (RedHat Satellite) server, these are used to distribute things like RPMs/Debs/Containers and are at the core of things like rhn.redhat.com.

As far as I understand the reason the OIDs are so long is because they include authorization information in addition to the authentication info (gives the ability to grant access to specific repositories/life-cycles).

Perhaps someone more knowledgeable like @ohadlevy or @tbrisker from the @theforeman project could elaborate if needed.

https://github.com/theforeman/foreman

@awood
Copy link

awood commented Jan 9, 2020

The files provided in the bug report are not DER encoded. They are PEM encoded.

If you run openssl asn1parse -inform PEM -in helm-test-cert, you get normal output. If you run openssl asn1parse -inform DER -in helm-test-cert you get a message about an encoding error.

To convert them to DER, you'll need to strip the -----BEGIN... and -----END... markers and then base64 decode the rest. Hope that helps!

@toothrot toothrot changed the title Client Side TLS Authentication fails for certs with long fields crypto/tls: Client Side TLS Authentication fails for certs with long fields Jan 9, 2020
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 9, 2020
@toothrot toothrot added this to the Backlog milestone Jan 9, 2020
@toothrot
Copy link
Contributor

toothrot commented Jan 9, 2020

/cc @agl @FiloSottile

@gopherbot
Copy link

gopherbot commented Aug 12, 2020

Change https://golang.org/cl/248259 mentions this issue: encoding/asn1: add dynamic length integer support for base 128 integer parsing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants