Skip to content

Commit 891a67d

Browse files
committed
Strengthen generateRandomString
Previously, generateRandomString used crypto/rand bytes encoded with hex.EncodeToString, so every character in the output was limited to the 16 hex digits. This gave only 4 bits of entropy per character, making the generated passwords significantly weaker than their length would suggest. Replace hex encoding with an alphanumeric charset (a-z, A-Z, 0-9), which provides 62 possible values per character, enforce a minimum length of 16 characters, and guarantee that every generated password contains at least one lowercase letter, one uppercase letter, and one digit. This makes the password more secure against brute-force attacks. Update tests to cover the minimum-length error path and the new character-class requirements.
1 parent c950aa2 commit 891a67d

2 files changed

Lines changed: 70 additions & 44 deletions

File tree

internal/controller/common.go

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ import (
2020
"context"
2121
"crypto/rand"
2222
_ "embed"
23-
"encoding/hex"
2423
"encoding/json"
2524
"errors"
2625
"fmt"
26+
"math/big"
2727
"strings"
2828

2929
common_helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper"
@@ -187,16 +187,46 @@ func getDeployment(ctx context.Context, h *common_helper.Helper, name string, na
187187
return deployment, nil
188188
}
189189

190-
// generateRandomString generates a random hex string of the given length.
190+
// generateRandomString generates a random alphanumeric string of the given length containing at
191+
// least one lowercase letter, one uppercase letter, and one digit. Minimum required secretLength
192+
// is 16.
191193
func generateRandomString(secretLength int) (string, error) {
192-
// Ceiling division: hex.EncodeToString doubles the byte count, so we need ceil(secretLength/2) bytes.
193-
randDataLen := (secretLength + 1) / 2
194+
const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
195+
const charsetLen = len(charset)
196+
const minLength = 16
197+
const maxIterations = 1000
194198

195-
b := make([]byte, randDataLen)
196-
if _, err := rand.Read(b); err != nil {
197-
return "", fmt.Errorf("failed to generate secret: %w", err)
199+
if secretLength < minLength {
200+
return "", fmt.Errorf("secret length must be at least %d", minLength)
198201
}
199202

200-
randString := hex.EncodeToString(b)
201-
return randString[:secretLength], nil
203+
for range maxIterations {
204+
205+
randPassword := make([]byte, secretLength)
206+
var hasLowerCase, hasUpperCase, hasNumber bool
207+
208+
for passwdCharIdx := range secretLength {
209+
idx, err := rand.Int(rand.Reader, big.NewInt(int64(charsetLen)))
210+
if err != nil {
211+
return "", fmt.Errorf("failed to generate secret: %w", err)
212+
}
213+
214+
passwdChar := charset[idx.Int64()]
215+
if passwdChar >= 'a' && passwdChar <= 'z' {
216+
hasLowerCase = true
217+
} else if passwdChar >= 'A' && passwdChar <= 'Z' {
218+
hasUpperCase = true
219+
} else if passwdChar >= '0' && passwdChar <= '9' {
220+
hasNumber = true
221+
}
222+
223+
randPassword[passwdCharIdx] = passwdChar
224+
}
225+
226+
if hasLowerCase && hasUpperCase && hasNumber {
227+
return string(randPassword), nil
228+
}
229+
}
230+
231+
return "", fmt.Errorf("failed to generate secret: exceeded iterations - %d", maxIterations)
202232
}

internal/controller/common_test.go

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -21,42 +21,20 @@ import (
2121
)
2222

2323
func TestGenerateRandomStringLength(t *testing.T) {
24+
t.Run("Below minimum length returns error", func(t *testing.T) {
25+
_, err := generateRandomString(15)
26+
if err == nil {
27+
t.Error("generateRandomString(15) expected error, got nil")
28+
}
29+
})
30+
2431
tests := []struct {
2532
name string
2633
length int
2734
}{
28-
{
29-
name: "Zero length",
30-
length: 0,
31-
},
32-
{
33-
name: "Odd length 1",
34-
length: 1,
35-
},
36-
{
37-
name: "Even length 2",
38-
length: 2,
39-
},
40-
{
41-
name: "Odd length 7",
42-
length: 7,
43-
},
44-
{
45-
name: "Even length 8",
46-
length: 8,
47-
},
48-
{
49-
name: "Odd length 15",
50-
length: 15,
51-
},
52-
{
53-
name: "Even length 16",
54-
length: 16,
55-
},
56-
{
57-
name: "Even length 32",
58-
length: 32,
59-
},
35+
{name: "Min length 16", length: 16},
36+
{name: "Even length 32", length: 32},
37+
{name: "Odd length 33", length: 33},
6038
}
6139

6240
for _, tt := range tests {
@@ -72,16 +50,34 @@ func TestGenerateRandomStringLength(t *testing.T) {
7250
}
7351
}
7452

75-
func TestGenerateRandomStringHexCharacters(t *testing.T) {
53+
func TestGenerateRandomStringCharacters(t *testing.T) {
7654
result, err := generateRandomString(32)
7755
if err != nil {
7856
t.Fatalf("generateRandomString(32) unexpected error: %v", err)
7957
}
58+
59+
var hasLower, hasUpper, hasDigit bool
8060
for i, c := range result {
81-
if (c < '0' || c > '9') && (c < 'a' || c > 'f') {
82-
t.Errorf("generateRandomString(32) character at index %d is %q, not a lowercase hex character", i, c)
61+
switch {
62+
case c >= 'a' && c <= 'z':
63+
hasLower = true
64+
case c >= 'A' && c <= 'Z':
65+
hasUpper = true
66+
case c >= '0' && c <= '9':
67+
hasDigit = true
68+
default:
69+
t.Errorf("generateRandomString(32) character at index %d is %q, not alphanumeric", i, c)
8370
}
8471
}
72+
if !hasLower {
73+
t.Error("generateRandomString(32) result has no lowercase letter")
74+
}
75+
if !hasUpper {
76+
t.Error("generateRandomString(32) result has no uppercase letter")
77+
}
78+
if !hasDigit {
79+
t.Error("generateRandomString(32) result has no digit")
80+
}
8581
}
8682

8783
func TestGenerateRandomStringUniqueness(t *testing.T) {

0 commit comments

Comments
 (0)