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

encoding/pem: pem.Decode should take care of PEM file's encapsulation boundaries #16335

Closed
simon-xia opened this issue Jul 12, 2016 · 6 comments
Milestone

Comments

@simon-xia
Copy link

simon-xia commented Jul 12, 2016

Please answer these questions before submitting your issue. Thanks!

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

    go version go1.6.2 darwin/amd64

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

    GOARCH="amd64"
    GOBIN="/Users/simon/go/bin"
    GOEXE=""
    GOHOSTARCH="amd64"
    GOHOSTOS="darwin"
    GOOS="darwin"
    GOPATH="/Users/simon/Myfruit/Go/imports:/Users/simon/Mygo"
    GORACE=""
    GOROOT="/Users/simon/go"
    GOTOOLDIR="/Users/simon/go/pkg/tool/darwin_amd64"
    GO15VENDOREXPERIMENT="1"
    CC="clang"
    GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fno-common"
    CXX="clang++"
    CGO_ENABLED="1"
    
  3. What did you do?
    If possible, provide a recipe for reproducing the error.
    A complete runnable program is good.
    A link on play.golang.org is best.

code: https://play.golang.org/p/kjL-dBzhI5

according to rfc1421 4.4 Encapsulation Mechanism, the key is in invaild format

when I run the command openssl rsa -noout -text -in test.key, the following error occurred:

unable to load Private Key
10374:error:0906D066:PEM routines:PEM_read_bio:bad end line:/SourceCache/OpenSSL098/OpenSSL098-50/src/crypto/pem/pem_lib.c:747:

similar issue goes here:
http://stackoverflow.com/questions/18460035/unable-to-load-private-key-pem-routinespem-read-biono-start-linepem-lib-c6

  1. What did you expect to see?

output message: fail to decode the key

  1. What did you see instead?

output message: success to decode the key

maybe the following patch help

diff --git a/src/encoding/pem/pem.go b/src/encoding/pem/pem.go
index 506196b..b00d504 100644
--- a/src/encoding/pem/pem.go
+++ b/src/encoding/pem/pem.go
@@ -132,6 +132,11 @@ func Decode(data []byte) (p *Block, rest []byte) {
                return decodeError(data, rest)
        }

+       endLine, next := getline(rest[endIndex:])
+       if !bytes.HasSuffix(endLine, pemEndOfLine) {
+               return decodeError(data, next)
+       }
+
        base64Data := removeWhitespace(rest[:endIndex])
        p.Bytes = make([]byte, base64.StdEncoding.DecodedLen(len(base64Data)))
        n, err := base64.StdEncoding.Decode(p.Bytes, base64Data)
@quentinmit
Copy link
Contributor

/cc @agl

@quentinmit quentinmit added this to the Go1.8 milestone Jul 20, 2016
@agl agl self-assigned this Aug 7, 2016
@agl
Copy link
Contributor

agl commented Aug 17, 2016

I don't immediately see where the RFC requires that the PEM block end with a newline. I'm usually happy to make things strict but this case isn't clear to me.

@simon-xia
Copy link
Author

it is not about newline, it's about the footer of the PEM file. there should be five - at end of the file, less or more is incorrect.

@agl
Copy link
Contributor

agl commented Aug 18, 2016

Ha, right. Sorry I didn't see that. Patch coming.

@gopherbot
Copy link

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

@tarndt
Copy link

tarndt commented Jun 21, 2017

@agl I don't think this PR was rolled out as thoughtfully as it should have been; this made the same production Go code (and the same config files) when redeployed with an updated version of Go break.... all because some config files were missing a dash. 😿

Since not being strict here wasn't really a security issue I would suggest that in the future fixes for issues of trivial severity, but potentially high impact, like this warn/log for a Go version before they start enforcing. Thanks!

@golang golang locked and limited conversation to collaborators Jun 21, 2018
@rsc rsc unassigned agl Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants