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: 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

Comments

@jmhodges
Copy link
Contributor

@jmhodges 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

This comment has been minimized.

Copy link
Contributor Author

@jmhodges 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

This comment has been minimized.

Copy link
Contributor

@rsc 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

This comment has been minimized.

Copy link
Contributor

@agl 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

This comment has been minimized.

Copy link
Contributor Author

@jmhodges jmhodges commented Mar 4, 2014

Comment 4:

That would handle what I found, yep.
@griesemer

This comment has been minimized.

Copy link
Contributor

@griesemer griesemer commented Oct 1, 2014

Comment 5:

Labels changed: added repo-main.

@odeke-em

This comment has been minimized.

Copy link
Member

@odeke-em odeke-em commented Feb 19, 2017

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).

@gopherbot gopherbot closed this in d271576 Mar 1, 2017
@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.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.