switch from u-root/pkg/rand to crypto/rand

[stapelberg]

As per its documentation, u-root’s rand package implements cancelable reads, which is not something that this DHCP client uses.

The crypto/rand package has the advantage of using getrandom(2) on platforms where that is possible, and otherwise behaves exactly like u-root/pkg/rand.

Notably, this commit fixes the build when running with GO111MODULES=on, so it’s related to issue #123.

[codecov[bot]]

Codecov Report

Merging #279 into master will decrease coverage by 1.31%. The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #279      +/-   ##
=========================================
- Coverage   75.02%   73.7%   -1.32%     
=========================================
  Files          70      75       +5     
  Lines        3155    3666     +511     
=========================================
+ Hits         2367    2702     +335     
- Misses        681     836     +155     
- Partials      107     128      +21

Impacted Files	Coverage Δ
dhcpv6/dhcpv6message.go	58.16% <ø> (ø)	:arrow_up:
dhcpv4/dhcpv4.go	79.66% <ø> (ø)	:arrow_up:
dhcpv4/nclient4/client.go	68.7% <0%> (ø)
dhcpv4/nclient4/conn_linux.go	48.48% <0%> (ø)
dhcpv6/nclient6/client.go	58.06% <0%> (ø)
dhcpv4/nclient4/ipv4.go	80.9% <0%> (ø)
dhcpv4/server4/server.go	69.69% <0%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 03e9ac9...5244c0d. Read the comment docs.

[hugelgupf]

u-root's rand uses getrandom.

The package was requested by @insomniacslk - is your usage of it internal-only at this point?

I do plan to make use of the cancelable functionality. We often end up on systems that do not have a random number source initially available and this package provides nicer diagnostics than the print-and-hang-forever that crypto/rand will do.

[stapelberg]

u-root's rand uses getrandom.

Ah, I missed this because godoc referred me to random_urandom.go. Nevermind :)

I do plan to make use of the cancelable functionality. We often end up on systems that do not have a random number source initially available and this package provides nicer diagnostics than the print-and-hang-forever that crypto/rand will do.

That’s fair. Might be good to suggest said nicer diagnostics upstream for crypto/rand, too, though :)

[insomniacslk]

Might be good to suggest said nicer diagnostics upstream for crypto/rand, too, though :)

Go 1.12 introduced a 60-seconds timeout before printing a warning, which may be good enough if we only wanted to know what's going on. I still think it's valuable to cancel the random number generation, and I like your idea of submitting the proposition upstream. What do you think @hugelgupf ?

(Also CC @mdlayher )

[mdlayher]

+1 for at least filing an issue with Go upstream to explain the use case.

[stapelberg]

still think it's valuable to cancel the random number generation, and I like your idea of submitting the proposition upstream

To be clear, I only proposed submitting better diagnostics upstream. I’m not sure yet whether canceling random number reading is really a good idea. If random numbers are optional, why read them in the first place? If they’re not, why cancel?

[insomniacslk]

To be clear, I only proposed submitting better diagnostics upstream.

I know, sorry for the confusion. In my (mis-worded) reply I meant that some form of diagnostic is already available starting with Go 1.12, and that I'd like the idea of submitting upstream a feature I find useful (cancellation).

I’m not sure yet whether canceling random number reading is really a good idea. If random numbers are optional, why read them in the first place? If they’re not, why cancel?

A scenario where this may be useful is when you need to do something with those numbers within a certain amount of time, or fail and do something else. This is exactly my use case: in the early seconds of booting, a host may or may not have enough entropy (that I use for DHCP transactions and other cryptographic operations), and I need to:

• detect whether we are stuck on random number generation (Go 1.12 now handles this, but it wasn't available before)
• react and report the error in some way.

However I agree that the feature may be considered not useful enough to be part of the standard library, as there are not many scenarios with these needs.