Review random string/password generation

- factor password generation into lib/random.Password
- call from appropriate places
- choose appropriate use of random.String vs random.Password

backend/premiumizeme/premiumizeme.go

@@ -19,8 +19,6 @@ canStream = false
 
 import (
 	"context"
-	"crypto/rand"
-	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -44,6 +42,7 @@ import (
 	"github.com/rclone/rclone/lib/dircache"
 	"github.com/rclone/rclone/lib/oauthutil"
 	"github.com/rclone/rclone/lib/pacer"
+	"github.com/rclone/rclone/lib/random"
 	"github.com/rclone/rclone/lib/rest"
 	"golang.org/x/oauth2"
 )
@@ -664,7 +663,7 @@ func (f *Fs) move(ctx context.Context, isFile bool, id, oldLeaf, newLeaf, oldDir
 	// another directory to make sure we don't overwrite something
 	// in the destination directory by accident
 	if doRenameLeaf && doMove {
-		tmpLeaf := newLeaf + "." + randomString(8)
+		tmpLeaf := newLeaf + "." + random.String(8)
 		err = f.renameLeaf(ctx, isFile, id, tmpLeaf)
 		if err != nil {
 			return errors.Wrap(err, "Move rename leaf")
@@ -1010,13 +1009,6 @@ func (o *Object) metaHash() string {
 	return fmt.Sprintf("remote=%q, size=%d, modTime=%v, id=%q, mimeType=%q", o.remote, o.size, o.modTime, o.id, o.mimeType)
 }
 
-// randomString returns a string with 8*bytes bits of entropy
-func randomString(bytes int) string {
-	var pw = make([]byte, bytes)
-	_, _ = rand.Read(pw)
-	return base64.RawURLEncoding.EncodeToString(pw)
-}
-
 // Update the object with the contents of the io.Reader, modTime and size
 //
 // If existing is set then it updates the object rather than creating a new one
@@ -1073,7 +1065,7 @@ func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, op
 	uploaded := false
 	var oldID = o.id
 	if o.hasMetaData {
-		newLeaf := leaf + "." + randomString(8)
+		newLeaf := leaf + "." + random.String(8)
 		fs.Debugf(o, "Moving old file out the way to %q", newLeaf)
 		err = o.fs.renameLeaf(ctx, true, oldID, newLeaf)
 		if err != nil {

cmd/rcd/rcd.go

@@ -64,7 +64,10 @@ See the [rc documentation](/rc/) for more info on the rc flags.
 				fs.Infof("Using default username: %s \n", rcflags.Opt.HTTPOptions.BasicUser)
 			}
 			if rcflags.Opt.HTTPOptions.BasicPass == "" {
-				randomPass := random.String(16)
+				randomPass, err := random.Password(128)
+				if err != nil {
+					log.Fatalf("Failed to make password: %v", err)
+				}
 				rcflags.Opt.HTTPOptions.BasicPass = randomPass
 				fs.Infof("No password specified. Using random password: %s \n", randomPass)
 			}

fs/config/config.go

@@ -35,6 +35,7 @@ import (
 	"github.com/rclone/rclone/fs/fshttp"
 	"github.com/rclone/rclone/fs/fspath"
 	"github.com/rclone/rclone/fs/rc"
+	"github.com/rclone/rclone/lib/random"
 	"golang.org/x/crypto/nacl/secretbox"
 	"golang.org/x/text/unicode/norm"
 )
@@ -860,16 +861,10 @@ func ChooseOption(o *fs.Option, name string) string {
 			for {
 				fmt.Printf("Password strength in bits.\n64 is just about memorable\n128 is secure\n1024 is the maximum\n")
 				bits := ChooseNumber("Bits", 64, 1024)
-				bytes := bits / 8
+				password, err := random.Password(bits)
-				if bits%8 != 0 {
+				if err != nil {
-					bytes++
+					log.Fatalf("Failed to make password: %v", err)
 				}
-				var pw = make([]byte, bytes)
-				n, _ := rand.Read(pw)
-				if n != bytes {
-					log.Fatalf("password short read: %d", n)
-				}
-				password = base64.RawURLEncoding.EncodeToString(pw)
 				fmt.Printf("Your password is: %s\n", password)
 				fmt.Printf("Use this password? Please note that an obscured version of this \npassword (and not the " +
 					"password itself) will be stored under your \nconfiguration file, so keep this generated password " +

lib/random/random.go

@@ -1,9 +1,16 @@
 // Package random holds a few functions for working with random numbers
 package random
 
-import "math/rand"
+import (
+	"encoding/base64"
+	"math/rand"
 
-// String create a random string for test purposes
+	"github.com/pkg/errors"
+)
+
+// String create a random string for test purposes.
+//
+// Do not use these for passwords.
 func String(n int) string {
 	const (
 		vowel     = "aeiou"
@@ -20,3 +27,27 @@ func String(n int) string {
 	}
 	return string(out)
 }
+
+// Password creates a crypto strong password which is just about
+// memorable.  The password is composed of printable ASCII characters
+// from the base64 alphabet.
+//
+// Requres password strength in bits.
+// 64 is just about memorable
+// 128 is secure
+func Password(bits int) (password string, err error) {
+	bytes := bits / 8
+	if bits%8 != 0 {
+		bytes++
+	}
+	var pw = make([]byte, bytes)
+	n, err := rand.Read(pw)
+	if err != nil {
+		return "", errors.Wrap(err, "password read failed")
+	}
+	if n != bytes {
+		return "", errors.Errorf("password short read: %d", n)
+	}
+	password = base64.RawURLEncoding.EncodeToString(pw)
+	return password, nil
+}

lib/random/random_test.go

@@ -4,10 +4,47 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func TestString(t *testing.T) {
+func TestStringLength(t *testing.T) {
 	for i := 0; i < 100; i++ {
-		assert.Equal(t, i, len(String(i)))
+		s := String(i)
+		assert.Equal(t, i, len(s))
+	}
+}
+
+func TestStringDuplicates(t *testing.T) {
+	seen := map[string]bool{}
+	for i := 0; i < 100; i++ {
+		s := String(8)
+		assert.False(t, seen[s])
+		assert.Equal(t, 8, len(s))
+		seen[s] = true
+	}
+}
+
+func TestPasswordLength(t *testing.T) {
+	for i := 0; i <= 128; i++ {
+		s, err := Password(i)
+		require.NoError(t, err)
+		// expected length is number of bytes rounded up
+		expected := i / 8
+		if i%8 != 0 {
+			expected++
+		}
+		// then converted to base 64
+		expected = (expected*8 + 5) / 6
+		assert.Equal(t, expected, len(s), i)
+	}
+}
+
+func TestPasswordDuplicates(t *testing.T) {
+	seen := map[string]bool{}
+	for i := 0; i < 100; i++ {
+		s, err := Password(64)
+		require.NoError(t, err)
+		assert.False(t, seen[s])
+		seen[s] = true
 	}
 }