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: make SystemCertPool work on Windows #26770

Closed
wants to merge 6 commits into from

Conversation

jeet-parekh
Copy link
Contributor

Populate SystemCertPool on Windows using certutil.exe

Fixes #16736

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Aug 2, 2018
@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 1:

(1 comment)

Will review more when Go 1.12 opens (after Go 1.11 is out).


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 2: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 3: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 4: New patch set was added with same tree, parent, and commit message as Patch Set 3.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 4:

I edited the commit message in PS2. I don't know about PS3 and PS4. I'll try again.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 5: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 6: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 7: New patch set was added with same tree, parent, and commit message as Patch Set 6.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 7:

Why does the bot keep reverting the commit message change? I'll give it a final try. Then I'll close this CL for a new one.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 8: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 9: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 10: New patch set was added with same tree, parent, and commit message as Patch Set 9.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 10:

The bot keeps reverting the commit message change. Closing this CL for a new one.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@jeet-parekh jeet-parekh closed this Aug 3, 2018
@bradfitz bradfitz reopened this Aug 3, 2018
@bradfitz bradfitz changed the title crypto/x509: make SystemCertPool work on Windows crypto/x509: make SystemCertPool work on Windows Aug 3, 2018
@bradfitz bradfitz changed the title crypto/x509: make SystemCertPool work on Windows crypto/x509: make SystemCertPool work on Windows Aug 3, 2018
@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 10:

Please don't open a new one. We prefer to keep history & discussion in one place.

See the last section of https://github.com/golang/go/wiki/CommitMessage#github-pull-requests

If you started via a GitHub PR, you have to keep using GitHub PRs. See those docs on how it maps to a Gerrit commit message.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@bradfitz
Copy link
Contributor

bradfitz commented Aug 3, 2018

I trimmed the leading and trailing spaces from this PR title.

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 11: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 12: New patch set was added with same tree, parent, and commit message as Patch Set 11.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 12:

I thought I could edit the commit message directly from the gerrit web UI. Guess I was wrong. Thanks a lot for taking care of it.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5070:

Patch Set 12:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 12:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5070:

Patch Set 12:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 12:

I think what you are proposing is to resurrect CL 30578, but then add all missing root certificates using certutil.exe (but remove any additional roots, if they are disallowed by local administrator). It might work. I do not know.

Almost similar to that, yes. I'll send in a new patch soon.

I do not like adding extra complexity to standard library. But do not listen to me - I do not use any of that code myself. So I am not the expert.

I'll open a proposal for discussion after we complete this CL. We could use more opinions on this.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 14: New patch set was added with same tree, parent, and commit message as Patch Set 13.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5070:

Patch Set 14:

Sorry, but I am not an expert to review this.
You have to find another reviewer.

Alex


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 26193:

Patch Set 14:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 14:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 14:

Just giving this a nudge. I don't know who could review this CL.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 12446:

Uploaded patch set 16: New patch set was added with same tree, parent, and commit message as Patch Set 15.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 16:

Giving this a nudge again.


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5065:

Patch Set 16:

Alex, Filippo?


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 26193:

Patch Set 16:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 24716:

Patch Set 16:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gerrit User 5070:

Patch Set 16:

Patch Set 16:

Alex, Filippo?

This implementation looks complicated and full of hacks to me - not something I like to see in security sensitive code.

But I am not a security expert. I said as much at patch set 14. And not much changed since patch set 14.

Sorry.

Alex


Please don’t reply on this GitHub thread. Visit golang.org/cl/127577.
After addressing review feedback, remember to publish your drafts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crypto/x509: make SystemCertPool work on Windows?
4 participants