code review 4446068: crypto/rand: add utility functions for number generation

crypto/rand: add utility functions for number generation

This code is extracted from crypto/rsa with
a few variables renamed and a comment fixed.

[ality, 2011-04-28 01:36:33]

Hello agl (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc1, 2011-04-28 03:09:38]

after the release

On Wed, Apr 27, 2011 at 21:36,  <ality@pbrane.org> wrote:
> Reviewers: agl,
>
> Message:
> Hello agl (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> crypto/rand: add utility functions for number generation
>
> This code is extracted from crypto/rsa with
> a few variables renamed and a comment fixed.
>
> Please review this at http://codereview.appspot.com/4446068/
>
> Affected files:
>  M src/pkg/crypto/rand/Makefile
>  A src/pkg/crypto/rand/util.go
>  M src/pkg/crypto/rsa/rsa.go
>
>
>

[agl1, 2011-04-28 13:48:45]

As rsc noted, please don't land until after the release. Otherwise, LGTM.

http://codereview.appspot.com/4446068/diff/4004/src/pkg/crypto/rand/util.go
File src/pkg/crypto/rand/util.go (right):

http://codereview.appspot.com/4446068/diff/4004/src/pkg/crypto/rand/util.go#n...
src/pkg/crypto/rand/util.go:1: // Copyright 2010 The Go Authors.  All rights
reserved.
2011

http://codereview.appspot.com/4446068/diff/4004/src/pkg/crypto/rand/util.go#n...
src/pkg/crypto/rand/util.go:20: bytes := make([]byte, (bits+7)/8)
(You don't need to fix this in this CL: for bonus points only.) It used to be
the case that rsa only generated prime numbers that were multiples of 8 bits.
With multi-prime support, that's no longer the case. So this needs the
fraction-of-a-byte trick copied from Number.

http://codereview.appspot.com/4446068/diff/4004/src/pkg/crypto/rand/util.go#n...
src/pkg/crypto/rand/util.go:59: return
return nil, err

[ality, 2011-05-05 20:05:15]

http://codereview.appspot.com/4446068/diff/4004/src/pkg/crypto/rand/util.go
File src/pkg/crypto/rand/util.go (right):

http://codereview.appspot.com/4446068/diff/4004/src/pkg/crypto/rand/util.go#n...
src/pkg/crypto/rand/util.go:1: // Copyright 2010 The Go Authors.  All rights
reserved.
On 2011/04/28 13:48:45, agl1 wrote:
> 2011

Done.

http://codereview.appspot.com/4446068/diff/4004/src/pkg/crypto/rand/util.go#n...
src/pkg/crypto/rand/util.go:20: bytes := make([]byte, (bits+7)/8)
On 2011/04/28 13:48:45, agl1 wrote:
> (You don't need to fix this in this CL: for bonus points only.) It used to be
> the case that rsa only generated prime numbers that were multiples of 8 bits.
> With multi-prime support, that's no longer the case. So this needs the
> fraction-of-a-byte trick copied from Number.

Done.

http://codereview.appspot.com/4446068/diff/4004/src/pkg/crypto/rand/util.go#n...
src/pkg/crypto/rand/util.go:59: return
On 2011/04/28 13:48:45, agl1 wrote:
> return nil, err

Done.

[agl1, 2011-05-05 20:11:19]

LGTM

[ality, 2011-05-13 20:43:19]

Ping

[rsc, 2011-05-17 02:46:28]

http://codereview.appspot.com/4446068/diff/8001/src/pkg/crypto/rand/util.go
File src/pkg/crypto/rand/util.go (right):

http://codereview.appspot.com/4446068/diff/8001/src/pkg/crypto/rand/util.go#n...
src/pkg/crypto/rand/util.go:50: // Number returns a uniform random value in [0,
max).
I think this should be rand.Int, just like in package rand.

http://codereview.appspot.com/4446068/diff/8001/src/pkg/crypto/rsa/rsa.go
File src/pkg/crypto/rsa/rsa.go (right):

http://codereview.appspot.com/4446068/diff/8001/src/pkg/crypto/rsa/rsa.go#new...
src/pkg/crypto/rsa/rsa.go:13: "crypto/rand"
sort (should be up 1 line)

[ality, 2011-05-17 06:24:47]

Hello agl@chromium.org, rsc@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[ality, 2011-05-17 06:26:08]

Should I say something in the function comments
about the type of io.Reader that should generally
be passed in, i.e., a rand.Reader?

Thanks.

http://codereview.appspot.com/4446068/diff/8001/src/pkg/crypto/rand/util.go
File src/pkg/crypto/rand/util.go (right):

http://codereview.appspot.com/4446068/diff/8001/src/pkg/crypto/rand/util.go#n...
src/pkg/crypto/rand/util.go:50: // Number returns a uniform random value in [0,
max).
On 2011/05/17 02:46:28, rsc wrote:
> I think this should be rand.Int, just like in package rand.

Done.

http://codereview.appspot.com/4446068/diff/8001/src/pkg/crypto/rsa/rsa.go
File src/pkg/crypto/rsa/rsa.go (right):

http://codereview.appspot.com/4446068/diff/8001/src/pkg/crypto/rsa/rsa.go#new...
src/pkg/crypto/rsa/rsa.go:13: "crypto/rand"
On 2011/05/17 02:46:28, rsc wrote:
> sort (should be up 1 line)

Done.

[rsc, 2011-05-17 14:31:25]

On Tue, May 17, 2011 at 02:26,  <ality@pbrane.org> wrote:
> Should I say something in the function comments
> about the type of io.Reader that should generally
> be passed in, i.e., a rand.Reader?

Maybe just name the argument rand instead of r.

[ality, 2011-05-17 22:21:48]

PTAL

[agl1, 2011-05-18 22:57:35]

I think rsc is out for a few days, but LGTM. Will land tonight unless anyone
objects.

[agl1, 2011-05-19 01:56:18]

*** Submitted as http://code.google.com/p/go/source/detail?r=b2d0fe8068f0 ***

crypto/rand: add utility functions for number generation

This code is extracted from crypto/rsa with
a few variables renamed and a comment fixed.

R=agl, rsc, agl
CC=golang-dev
http://codereview.appspot.com/4446068

Committer: Adam Langley <agl@golang.org>