crypto/rand: Read violates condition n == len(b) if and only if err == nil for len(b) == 0

[pdewilde]

Go version

go1.22.3

Output of go env in your module/workspace:

N/A: Issue found in source, have not reproduced.

What did you do?

rand.Read claims:

Read is a helper function that calls Reader.Read using io.ReadFull. On return, n == len(b) if and only if err == nil.

If and only if is saying err != nil -> n != len(b) but this is not necessarily the case.

What did you see happen?

For the case of len(b) == 0, there are codepaths in some implementations of Read where a non-nil err is returned along with an n of 0. This violates the claim in the documentation as err is non-nill and the returned n is the same as len(b)

An example can be found in the rand_unix.go implementation:

If r hasn't been used yet, the call to os.Open() for the urandomDevice can return an error. in this scenario, Read returns 0, err.

Since no short-circuiting is in the implementation, this code path will run even for a input of length 0.

After the first run, this will not happen, as reading is delegated to io.ReadFull which does properly enforce if and only if semantics as it will short circuit on a length of zero, never returning an error.

What did you expect to see?

As I see it there are two possible courses of action:

1. Weaken the docstring's garuntee for rand.Read to claim Read is a helper function that calls Reader.Read using io.ReadFull. On return, n == len(b) if err == nil.

2. Add short-circuiting to every implementation so that a Read to an array of length zero will never return an error.

[pdewilde]

Here is an implementation of fix 1:
https://go-review.git.corp.google.com/c/go/+/584455

[zephyrtronium]

See also #66821, which proposes to make the error always be nil instead.

[pdewilde]

Interesting. Seems that for the most part err should be unreachable except for really old Linux kernels (and technically WASIP1 though its unlikely the underlying platform will actually error).

If #66821 is approved, then this is obsolete. Its a shame the API had to be polluted with an err if that ends up being the solution.

[itstarsun]

io.ReadFull calls io.ReadAtLeast which never calls Reader.Read when len(b) == 0.
See https://go.dev/play/p/LS5maI0i2Dn.

I don't get how this violates the claim or the reason to weaken the docs.

[pdewilde]

@itstarsun

The issue happens before io.ReadFull is called. rand.Read in some platforms (legacy linux) can have an error before io.FullRead is called.

See: https://cs.opensource.google/go/go/+/refs/tags/go1.22.3:src/crypto/rand/rand_unix.go;l=65

The call to os.Open(urandomDevice) happens regardless of len(b) during first time setup.

[itstarsun]

@pdewilde

rand.Read, the function, did call io.ReadFull, so the code path never runs when len(b) == 0 as you see in the playground above.

If you mean rand.Reader.Read, the method, then the same apply to io.Reader.Read which never guarantees that n == len(b) if and only if err == nil.

[pdewilde]

@itstarsun You are totally right, I was conflating rand.Reader.Read and rand.Read.

Thanks for catching that!