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.

Sign up for GitHub

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/rand: use BCryptGenRandom instead of CryptGenRandom on Windows #38938

Closed
wants to merge 1 commit into from
Closed

crypto/rand: use BCryptGenRandom instead of CryptGenRandom on Windows #38938

wants to merge 1 commit into from

Conversation

neolit123
Copy link
Contributor

The existing function that is used is CryptGenRandom. This function
and the whole underling API is deprecated.

Use the function BCryptGenRandom from the new recommended
API called "Cryptography API: Next Generation (CNG)".

Fixes #33542

@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 May 7, 2020
@gopherbot
Copy link
Contributor

This PR (HEAD: 7c840d4) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/232860 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

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

@gopherbot
Copy link
Contributor

Message from Lubomir I. Ivanov:

Patch Set 1:

hi, this is the opposite proposal to https://go-review.googlesource.com/c/go/+/210057
where a the undocumented function RtlGenRandom is used:
https://docs.microsoft.com/en-us/windows/win32/api/ntsecapi/nf-ntsecapi-rtlgenrandom

xref https://docs.microsoft.com/en-us/windows/win32/api/ntsecapi/nf-ntsecapi-rtlgenrandom

with that particular backend, we do not know:

  • what is the underlying implementation and standard
  • when and if the exported function SystemFunction036 will be removed.

this change on the other hand uses BCryptGenRandom which is the new recommended API by Microsoft and as the issue #33542 mentions is compliant with NIST SP800-90:

https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom#remarks

The default random number provider implements an algorithm for generating random numbers that complies with the NIST SP800-90 standard, specifically the CTR_DRBG portion of that standard.

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

@neolit123 neolit123 force-pushed the 1.15-switch-win32-crypto-backend branch from 7c840d4 to 88f6563 Compare May 8, 2020 04:31
@gopherbot
Copy link
Contributor

This PR (HEAD: 88f6563) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/232860 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@neolit123 neolit123 force-pushed the 1.15-switch-win32-crypto-backend branch from 88f6563 to ddc7dea Compare May 8, 2020 06:53
@gopherbot
Copy link
Contributor

This PR (HEAD: ddc7dea) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/232860 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Alex Brainman:

Patch Set 3:

(4 comments)

Thank you for doing this, but I am not convinced it is worth changing.

But let's decide what to do first

#33542 (comment)

before continuing with this CL.

Alex

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

@gopherbot
Copy link
Contributor

Message from Lubomir I. Ivanov:

Patch Set 3:

(3 comments)

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

@neolit123 neolit123 force-pushed the 1.15-switch-win32-crypto-backend branch from ddc7dea to 2919c3f Compare May 9, 2020 23:10
@gopherbot
Copy link
Contributor

This PR (HEAD: 2919c3f) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/232860 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Lubomir I. Ivanov:

Patch Set 4:

(4 comments)

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

@gopherbot
Copy link
Contributor

Message from Lubomir I. Ivanov:

Patch Set 4:

(2 comments)

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

@gopherbot
Copy link
Contributor

Message from Alex Brainman:

Patch Set 4:

(8 comments)

Lets wait for decision here

#33542 (comment)

before we proceed with this CL.

Alex

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

@gopherbot
Copy link
Contributor

Message from Lubomir I. Ivanov:

Patch Set 4:

(5 comments)

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

@neolit123 neolit123 force-pushed the 1.15-switch-win32-crypto-backend branch from 2919c3f to e70e214 Compare May 10, 2020 20:41
@gopherbot
Copy link
Contributor

This PR (HEAD: e70e214) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/232860 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Lubomir I. Ivanov:

Patch Set 5:

(3 comments)

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

@gopherbot
Copy link
Contributor

Message from Alex Brainman:

Patch Set 5:

(1 comment)

I will wait for decision on the issue, before proceeding here.

Thank you.

Alex

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

@neolit123
crypto/rand: use BCryptGenRandom instead of CryptGenRandom on Windows
c7fb621 
The existing function that is used is CryptGenRandom. This function
and the whole underling API is deprecated.

Use the function BCryptGenRandom from the new recommended
API called "Cryptography API: Next Generation (CNG)".

Preload and use the BCRYPT_RNG_ALGORITHM provider.
It follows the standards: FIPS 186-2, FIPS 140-2, NIST SP 800-90

Fixes golang#33542
@neolit123 neolit123 force-pushed the 1.15-switch-win32-crypto-backend branch from e70e214 to c7fb621 Compare September 6, 2020 21:05
@gopherbot
Copy link
Contributor

This PR (HEAD: c7fb621) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/232860 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Alex Brainman:

Patch Set 6:

(1 comment)

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

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

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

@neolit123 neolit123 closed this Oct 24, 2020
@neolit123
Copy link
Contributor Author

#33542 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

crypto/rand: Currently using deprecated API for random number generation on Windows
3 participants
@neolit123 @gopherbot @googlebot