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: Document how to make custom cert root list #6267

Closed
dsymonds opened this issue Aug 28, 2013 · 13 comments
Closed

crypto/x509: Document how to make custom cert root list #6267

dsymonds opened this issue Aug 28, 2013 · 13 comments

Comments

@dsymonds
Copy link
Member

@dsymonds dsymonds commented Aug 28, 2013

crypto/x509 has a hardcoded list of places to look for SSL system roots. If you're
running in an environment that doesn't have them there, you're out of luck. You either
need to modify Go, or use chroot or similar. It'd be much nicer if there was a hook in
that package such that a program could point at a different place for the root
certificates.
@dsymonds
Copy link
Member Author

@dsymonds dsymonds commented Nov 8, 2013

Comment 1:

Labels changed: added priority-soon, go1.3maybe, removed priority-triage.

@dsymonds
Copy link
Member Author

@dsymonds dsymonds commented Nov 11, 2013

Comment 2:

Owner changed to @agl.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Nov 11, 2013

Comment 3:

Out of luck?
In https://github.com/bradfitz/camlistore/blob/master/pkg/client/android/android.go I
use x509.NewCertPool() to make my own *tls.Config.
@dsymonds
Copy link
Member Author

@dsymonds dsymonds commented Nov 11, 2013

Comment 4:

Okay, maybe not out of luck. But it'd be nice to make it easier for cert verification to
work without having to do manual wiring.
@rsc
Copy link
Contributor

@rsc rsc commented Nov 11, 2013

Comment 5:

Unclear we want to give more control over this than what's already there.
@dsymonds
Copy link
Member Author

@dsymonds dsymonds commented Nov 11, 2013

Comment 6:

It's not really about more control, but just making it easier to use the control that is
already present. e.g. if I'm on a system that puts the SSL system roots in some weird
place, what do I do? When *I* read through the package I didn't find the
x509.NewCertPool approach that Brad used, so at the least I think the docs could make it
clearer (maybe an example?).
@rsc
Copy link
Contributor

@rsc rsc commented Nov 12, 2013

Comment 7:

The question is whether one package you import should be able to override
the system x509 location for others. Normally we trust packages to behave
but this is a fairly serious thing to be able to change.
In package time we use $ZONEINFO. Perhaps a similar environment variable
makes more sense than having each program change it, but even then I am a
little worried.
Certainly we should have a good example.
@dsymonds
Copy link
Member Author

@dsymonds dsymonds commented Nov 12, 2013

Comment 8:

Maybe from a safety perspective the API exposed can be limited to only *appending* to
the system root list, so it'll only be effective if the system can't find the roots in
the usual places. That reduces the attack vector from being able to subvert any cert
validation to only being able to permit cert validation when it otherwise would be
unable to be validated.
But you're right it needs some careful thought. Are we considering one package being
able to attack another (e.g. a reflection attack)? I guess that's only relevant if
places don't take a *tls.Config but only ever use the default, since you're still able
to load arbitrary roots into a custom tls.Config.
@rsc
Copy link
Contributor

@rsc rsc commented Nov 12, 2013

Comment 9:

I'm a little worried about attacks but more worried about inadvertent
breakage, where a package decides to install its own root list as the
"system" list because that's all *it* needs and doing so is easier than
preparing a tls.Config, and then other packages that need the standard list
break.
I also don't think it's so terrible to just edit the source code if you're
compiling the standard library for a system with a non-standard place.
@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 10:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 11:

Labels changed: added repo-main.

@dsymonds
Copy link
Member Author

@dsymonds dsymonds commented Dec 4, 2013

Comment 12:

Let's make this a documentation bug and get it fixed for 1.3.

Labels changed: added documentation, release-go1.3, removed packagechange, release-none.

@agl
Copy link
Contributor

@agl agl commented Feb 19, 2014

Comment 13:

This issue was closed by revision d4d7705.

Status changed to Fixed.

@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
FiloSottile pushed a commit to FiloSottile/go that referenced this issue Oct 12, 2018
Fixes golang#6267.

LGTM=r, josharian
R=golang-codereviews, josharian, r
CC=golang-codereviews
https://golang.org/cl/61020043
This issue was closed.
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
5 participants
You can’t perform that action at this time.