crypto/rsa: add rand initialization for rsa.SignPSS

[shibumi]

If nil as random source is being passed to rsa.SignPSS
this is going to lead to a nil pointer dereference and invalid memory
access. This commit intents to this fix via initializing
a secure random source with crypto/rand.Reader

[shibumi]

Hi,
This is my first PR for Go, so please look twice over this PR.

[gopherbot]

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

Please visit https://go-review.googlesource.com/c/go/+/240008 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]

Message from Gobot Gobot:

Patch Set 1:

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

Next steps:
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 or adds a tag like "wait-release", 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/240008.
After addressing review feedback, remember to publish your drafts!

[shibumi]

The nil pointer dereference happens here:

go/src/crypto/rsa/pss.go

Line 283 in c240625

if _, err := io.ReadFull(rand, salt); err != nil {

A possible stacktrace for it looks like this:

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x49e5e8]

goroutine 19 [running]:
testing.tRunner.func1.1(0x5f4660, 0x7be6c0)
	/usr/lib/go/src/testing/testing.go:940 +0x2f5
testing.tRunner.func1(0xc00014e240)
	/usr/lib/go/src/testing/testing.go:943 +0x3f9
panic(0x5f4660, 0x7be6c0)
	/usr/lib/go/src/runtime/panic.go:969 +0x166
io.ReadAtLeast(0x0, 0x0, 0xc00014c640, 0x20, 0x20, 0x20, 0x736c616972657461, 0x616e222c7d7b3a22, 0x6f6f66223a22656d)
	/usr/lib/go/src/io/io.go:310 +0x58
io.ReadFull(...)
	/usr/lib/go/src/io/io.go:329
crypto/rsa.SignPSS(0x0, 0x0, 0xc000132fc0, 0x5, 0xc00014c620, 0x20, 0x20, 0xc00017dde0, 0x44fd58, 0xc000182228, ...)
	/usr/lib/go/src/crypto/rsa/pss.go:263 +0x105

[gopherbot]

Message from Christian Rebischke:

Patch Set 1:

Hi,
This is my first PR for Go, so please look twice over this PR.

The nil pointer dereference happens here:

go/src/crypto/rsa/pss.go

Line 283 in c240625

if _, err := io.ReadFull(rand, salt); err != nil {

A possible stacktrace for it looks like this:

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x49e5e8]

goroutine 19 [running]:
testing.tRunner.func1.1(0x5f4660, 0x7be6c0)
	/usr/lib/go/src/testing/testing.go:940 +0x2f5
testing.tRunner.func1(0xc00014e240)
	/usr/lib/go/src/testing/testing.go:943 +0x3f9
panic(0x5f4660, 0x7be6c0)
	/usr/lib/go/src/runtime/panic.go:969 +0x166
io.ReadAtLeast(0x0, 0x0, 0xc00014c640, 0x20, 0x20, 0x20, 0x736c616972657461, 0x616e222c7d7b3a22, 0x6f6f66223a22656d)
	/usr/lib/go/src/io/io.go:310 +0x58
io.ReadFull(...)
	/usr/lib/go/src/io/io.go:329
crypto/rsa.SignPSS(0x0, 0x0, 0xc000132fc0, 0x5, 0xc00014c620, 0x20, 0x20, 0xc00017dde0, 0x44fd58, 0xc000182228, ...)
	/usr/lib/go/src/crypto/rsa/pss.go:263 +0x105

---

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

[gopherbot]

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

Please visit https://go-review.googlesource.com/c/go/+/240008 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]

Message from Christian Rebischke:

Patch Set 2:

Initialising rand with a secure random source should also increase protection against side-channel-attacks, because of using blinding for the decryption. I am aware that these are just a few rare cases, but it is definitely better than allowing a user using nil.

---

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

[gopherbot]

Message from Christian Rebischke:

Patch Set 2:

What do you think about adding a secure random source for other functions in crypto/rsa, too?
There are similar problems for other functions. For example: https://golang.org/src/crypto/rsa/rsa.go?s=7589:7677#L212 (creds to siXy in #golang-nuts for spotting)

Shall I initialize with rand.Reader for the other functions, as well?

---

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

[gopherbot]

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

Please visit https://go-review.googlesource.com/c/go/+/240008 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]

Message from Go Bot:

Patch Set 1:

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

Next steps:
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 or adds a tag like "wait-release", 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/240008.
After addressing review feedback, remember to publish your drafts!

[shibumi]

@heschi can you provide some information why this has been closed? I still think that throwing panics is bad practice and initializing the random as fallback is a good way.

[heschi]

I closed old PRs to reduce load on the Gerrit importer (#50197), sorry for the trouble. I'll reopen the CL and PR.

[shibumi]

@heschi no worries :) @FiloSottile I don't know if discussions around this should happen in gerrit or github, but can I get an answer on my latest comment or a final decision? I am not sad, if you reject this commit. I would just like to know if I should invest more time into this.