Fix potential vulnerability in use of the rand.Seed

[PiotrCzapla]

According to webauthn spec. the challange needs to be hard to guess
otherwise the protocol is vulnerable to reply attack see:
https://www.w3.org/TR/webauthn-2/#sctn-cryptographic-challenges

In order to prevent replay attacks, the challenges MUST contain
enough entropy to make guessing them infeasible. Challenges
SHOULD therefore be at least 16 bytes long.

Initating with the random seed with time makes it easy to guess after
a restart. The space of seeds can be quickly enumerated, because time
has lager resultion than nanosecods. See this more details:
https://github.com/Quiq/webauthn_proxy

[pdavies011010]

Thanks, we will review this for inclusion

[Oneiroi]

Thanks @PiotrCzapla,

Thank you for the links provided; I concur that using time as an entropy source is at best sub-optimal as time is predictable and thus leaves the resulting encryption weaker as a result.

Taking from the spec linked:

As a cryptographic protocol, Web Authentication is dependent upon randomised challenges to avoid replay attacks. Therefore, the values of both [PublicKeyCredentialCreationOptions](https://www.w3.org/TR/webauthn-2/#dictdef-publickeycredentialcreationoptions).[challenge](https://www.w3.org/TR/webauthn-2/#dom-publickeycredentialcreationoptions-challenge) and [PublicKeyCredentialRequestOptions](https://www.w3.org/TR/webauthn-2/#dictdef-publickeycredentialrequestoptions).[challenge](https://www.w3.org/TR/webauthn-2/#dom-publickeycredentialrequestoptions-challenge) MUST be randomly generated by [Relying Parties](https://www.w3.org/TR/webauthn-2/#relying-party) in an environment they trust (e.g., on the server-side), and the returned [challenge](https://www.w3.org/TR/webauthn-2/#dom-collectedclientdata-challenge) value in the client’s response MUST match what was generated. This SHOULD be done in a fashion that does not rely upon a client’s behavior, e.g., the Relying Party SHOULD store the challenge temporarily until the operation is complete. Tolerating a mismatch will compromise the security of the protocol.

In order to prevent replay attacks, the challenges MUST contain enough entropy to make guessing them infeasible. Challenges SHOULD therefore be at least 16 bytes long.

1. PubKey must be randomly generated by a trusted entropy source
2. the reutned challenge vault in the clients response MUST math what is generated.
3. Should not tolerate a miss-match in the challenge response.
4. the challenges MUST contain enough entropy to make guessing them infeasible.

The proposed code moves from using time.Now().UnixNano() to Seed the RNG (which is a predictable entropy source), to using a new method RandInit() which leverages crypto/rand then seeding the RNG.

This should result in a greater entropy for the RNG, as the next improvement for this (should time allow) it would be advisable to use purely the crypto/rand as an entropy source and looking at other examples of webauthn a simpler challenge could be: https://github.com/duo-labs/webauthn/blob/1daaee874e43b3bc324ade89467c603b5016d4b9/protocol/challenge.go (as an example), this would however need testing

[Oneiroi]

I further wrote a brief test (please forgive my terrible golang):

package main

import (
	crypto_rand "crypto/rand"
    "time"
	"encoding/binary"
	"fmt"
)

func main() {
	//old method
	oldSeed := time.Now().UnixNano()
	//proposed method
	var b [8]byte
	_, err := crypto_rand.Read(b[:])
	if err != nil {
		panic("cannot seed math/rand package with cryptographically secure random number generator")
	}
	newSeed := int64(binary.LittleEndian.Uint64(b[:]))
	fmt.Println("old:",oldSeed)
	fmt.Println("new:",newSeed)
}

oldSeed pulls the value used in the previous code of time.Now().UnixNano()
newSeed pulls the value used in the proposed code of int64(binary.LittleEndian.Uint64(b[:])) where b is 8 bytes

To produce sample I ran the following

for i in {1..100000}; do ./test >> ./distribution.log; done

Then ran in another shell:

tail -f distribution.log it can be observed that the see produced under the old method is indeed predictable; taking the following sample as an example:

1656415085070941000

First 10 bytes of this seed are the current time to the second: 1656415085 which is enumerable sequentially, the remaining bytes of the seed are more difficult to enumerate, but possible programmatically.

Extracting the values to individual files.

grep new distribution.log | awk '{print $2}' > new.log
grep old distribution.log | awk '{print $2}' > old.log

"new" method Estimated Entropy

"old" method estimated Entropy

Entropy conclusion

There is an improvement over the original time based seed method; future iterations should seek to leverage crypto/rand directly and simplify and perhaps align with examples given time for extending the efficacy of the simplification effort .

[pdavies011010]

@PiotrCzapla One question, can you explain where you're using b[:], I would assume that's the same as just b, but wondering if there's something I'm missing?

[Oneiroi]

For completeness I implemented https://github.com/duo-labs/webauthn/blob/1daaee874e43b3bc324ade89467c603b5016d4b9/protocol/challenge.go for the "proof"; the results of which are as follows:

test.go

package main

import (
	crypto_rand "crypto/rand"
	"encoding/base64"
	"encoding/binary"
	"fmt"
	"time"
)

func main() {
	//old method
	oldSeed := time.Now().UnixNano()
	//new method
	var b [8]byte
	_, err := crypto_rand.Read(b[:])
	if err != nil {
		panic("cannot seed math/rand package with cryptographically secure random number generator")
	}
	newSeed := int64(binary.LittleEndian.Uint64(b[:]))

	const ChallengeLength = 32
	challenge := make([]byte, ChallengeLength)
	_, errC := crypto_rand.Read(challenge)

	if errC != nil {
		panic("cannot seed crypto/rand package with cryptographically secure random number generator")
	}
	fmt.Println("old:", oldSeed)
	fmt.Println("new:", newSeed)
	fmt.Println("new32:", base64.RawURLEncoding.EncodeToString(challenge))
}

produce sample set

for i in {1..100000}; do ./test >> ./distribution.log; done

extract sample set

grep old distribution.log | awk '{print $2}' > old.log
grep new distribution.log | grep -v new32 | awk '{print $2}' > new.log
grep new32 distribution.log | awk '{print $2}' > new32.log

evaluate sample set

old cyberchef entropy

new cyberchef entropy

new32 cyberchef entropy

Conclusion

Using a purely crypto/rand method increasing the keylength to 32bytes bumps the entropy estimation shannon scale from 3.199 to ~6.02.

this PR will improve the Entropy shannon scale score from 3.199 to 3.524; I'll take a look at submitting a PR to get the index to the ~6.02 value shortly.

[pdavies011010]

Looks good. @Oneiroi has found a way to increase the entropy significantly so we'll merge this in and then he'll make his change as well. Thanks a lot @PiotrCzapla !

[Oneiroi]

thanks @PiotrCzapla !

[PiotrCzapla]

Guys,
I've looked up cracking of pseudo-random number generators. It seems it might be possible to predict the next random number without knowing the seed if you know a large enough sequence. Look at this issue: rust-random/rand#905 . (it is rust issue but It refers to all algorithms)

The go docs confirm this:

This package's outputs might be easily predictable regardless of how it's seeded. For random numbers suitable for security-sensitive work, see the crypto/rand package.

So we should use crypto/rand to generate the challanges. It uses /dev/urandom, which cannot be exhausted and is implementing CSPRNG (Cryptographically secure pseudo-random number) kernelside (https://stackoverflow.com/questions/13017023/how-can-i-exhaust-dev-urandom-for-testing)
But it may return EAGIN, I'm not sure why it would do that, assuming that the above SO answer is correct. But It seems it is the right way to generate hard-to-guess challenges.

An alternative that won't block is to get some user space implementation of CSPRNG. But I can't find any reliable library for go :(

• Cloudflare opted to implement simple CSPRNG by themselves: https://github.com/cloudflare/gokey/blob/master/csprng.go
• And the only other "csprng go" I've found is https://github.com/lukechampine/frand which has 70 stars :(.

@Oneiroi do you have better idea how to generate the challanges?

[PiotrCzapla]

@PiotrCzapla One question, can you explain where you're using b[:], I would assume that's the same as just b, but wondering if there's something I'm missing?

I was wondering that my self, I've made the PR on iPad so I've copied the code verbatim from stack :).
It seems that the go threat slices and arrays differently and b[:] convert an array to slice. See: https://stackoverflow.com/questions/47722542/what-does-the-symbol-mean-in-go

[PiotrCzapla]

If anyone is interested this is a really good explanation of /dev/urandom and /dev/random. So we should be safe using crypto/rand:

https://www.redhat.com/en/blog/understanding-random-number-generators-and-their-limitations-linux

[Oneiroi]

@PiotrCzapla

@Oneiroi do you have better idea how to generate the challenges?

I created #3 this implements the challenges in a more secure fashion, admittedly there's issues with these functioning right now I suspect this is to do with a need to update the JS to accept the change from integer based challenges to the base64 encoded version in #3 this is using crypto/rand which is noted to be crytographically secure.

Now Just need to fix the JS (is my gut feeling) to make this improvement viable 😅