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/tls: X509KeyPair skips over invalid pem blocks but doesn't error #7042

Closed
jmhodges opened this issue Jan 1, 2014 · 6 comments
Closed

crypto/tls: X509KeyPair skips over invalid pem blocks but doesn't error #7042

jmhodges opened this issue Jan 1, 2014 · 6 comments
Labels
FrozenDueToAge Thinking
Milestone
Unplanned

Comments

@jmhodges
Copy link
Contributor

jmhodges commented Jan 1, 2014 

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
1. Put three pem certs together in a file, but put the second one's BEGIN line on the
same line as the first one's end
2. Load with LoadX509KeyPair or X509KeyPair.
3. See 2 certificates loaded (the first and the last) but the second one is silently
skipped

What is the expected output?
Error when the file doesn't parse.

What do you see instead?
Loads the first and third certs with no acknowledgment of the second one.

Which operating system are you using?
This occurs on OS X and Linux


Which version are you using?  (run 'go version')
go version go1.2 darwin/amd64

`openssl x509` manages to error out both in this case and when the certs are broken up
incorrectly and the trailing dashes on the first is less by one than expected and the
leading dashes on the second is greater by one than expected.
@jmhodges
Copy link
Contributor Author

jmhodges commented Jan 1, 2014

Comment 1:

This is really a bug with encoding/pem, but the pem.Decode signature doesn't afford us
much leeway to fix it.

@rsc
Copy link
Contributor

rsc commented Mar 3, 2014

Comment 2:

pem is supposed to be extracting pem blocks from arbitrary text (like emails) that might
have other things. syntax errors are very difficult to distinguish from the arbitrary
text.

Labels changed: added release-none.

Status changed to Thinking.

@agl
Copy link
Contributor

agl commented Mar 4, 2014

Comment 3:

Perhaps the answer is to require that the end line contain nothing but optional
whitespace and EOF or newline after it. I think that would solve your issue, do you
agree?

@jmhodges
Copy link
Contributor Author

jmhodges commented Mar 4, 2014

Comment 4:

That would handle what I found, yep.

@griesemer
Copy link
Contributor

Comment 5:

Labels changed: added repo-main.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@taylorhughes taylorhughes mentioned this issue Jun 5, 2015
Closed
@odeke-em odeke-em mentioned this issue Feb 19, 2017
Closed
@odeke-em
Copy link
Member

The CL https://golang.org/cl/37147 for issue #19147 looks like it will fix this issue as a side-effect implementing exactly what @agl had mentioned the fix would be #7042 (comment).

@golang golang locked and limited conversation to collaborators Mar 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Thinking
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@jmhodges @agl @rsc @odeke-em @griesemer @gopherbot