Use base32 for 2FA scratch token

[wxiaoguang]

Close #6267

• 2FA scratch token includes ambiguous characters #6267

Changes:

1. refactor util.Random* to util.CryptoRandom* to indicate that they are generating crypto/secure random data.
   1. indeed it's different from something like MathRandomInt because it is much slower and more secure.
2. clarify the comment of CryptoRandomBytes
3. bytes is an imported package, so the local variables are renamed to buf from bytes
4. use base32 for 2FA scratch token (only for the scratch token)
   1. the new token is slightly longer, but easier to be written down
   2. the security level doesn't change. 6 * log(256) > 8 * log(62)

Before

After

[silverwind]

I think we should only output non-ambigous readable strings, e.g. exclude 1,l,0,O from the corpus, similar to pwgen -B:

  -B or --ambiguous
	Don't include ambiguous characters in the password

[wxiaoguang]

I think we should only output non-ambigous readable strings, e.g. exclude 1,l,0,O from the corpus, similar to pwgen -B:

  -B or --ambiguous
	Don't include ambiguous characters in the password

Yes, it is, see the code const base32Chars = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789", I choose customized chars.

[silverwind]

Ah, I missed that. Looks fine.

[Gusted]

Code looks good btw. Just a few things that we should address.

[lunny]

I don't think we should change all tokens generation methods. Only those users needs to input but cannot copy/paste should be changed.

[wxiaoguang]

@lunny Sorry I didn't get the point. Do you mean this PR is fine or not fine?

This PR only improves the scratch token generation (GenerateScratchToken), such scratch token might need to be written down or printed, see the original issue #6267, and from the screenshot, we can see the new token is much better and clear than before. I didn't change other tokens.

[lunny]

@lunny Sorry I didn't get the point. Do you mean this PR is fine or not fine?

This PR only improves the scratch token generation (GenerateScratchToken), such scratch token might need to be written down or printed, see the original issue #6267, and from the screenshot, we can see the new token is much better and clear than before. I didn't change other tokens.

It seems you have changed NewToken and UserSalt?

[wxiaoguang]

@lunny Sorry I didn't get the point. Do you mean this PR is fine or not fine?
This PR only improves the scratch token generation (GenerateScratchToken), such scratch token might need to be written down or printed, see the original issue #6267, and from the screenshot, we can see the new token is much better and clear than before. I didn't change other tokens.

It seems you have changed NewToken and UserSalt?

Where? That's just a refactoring of RandomString => CryptoRandomString and RandomBytes => CryptoRandomBytes. Nothing else is changed. It has been explained in the issue comment: refactor util.Random* to util.CryptoRandom* to indicate that they are generating crypto/secure random data.