Merge pull request #25 from lpiwowar/lpiwowar/postgresql-compliance

Strengthen generateRandomString

Author: openshift-merge-bot[bot]

internal/controller/assets/postgres.conf

@@ -7,3 +7,5 @@ ssl_key_file = '/etc/certs/tls.key'
 # (sslcert/sslkey) to PostgreSQL, so ssl_ca_file has no effect (even when pg_hba.conf
 # is correctly configured for mTLS)
 # ssl_ca_file = '<none>'
+log_connections = on
+log_disconnections = on

internal/controller/common.go

@@ -20,10 +20,10 @@ import (
 	"context"
 	"crypto/rand"
 	_ "embed"
-	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math/big"
 	"strings"
 
 	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
 	return deployment, nil
 }
 
-// generateRandomString generates a random hex string of the given length.
+// generateRandomString generates a random alphanumeric string of the given length containing at
+// least one lowercase letter, one uppercase letter, and one digit. Minimum required secretLength
+// is 16.
 func generateRandomString(secretLength int) (string, error) {
-	// Ceiling division: hex.EncodeToString doubles the byte count, so we need ceil(secretLength/2) bytes.
-	randDataLen := (secretLength + 1) / 2
+	const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
+	const charsetLen = len(charset)
+	const minLength = 16
+	const maxIterations = 1000
 
-	b := make([]byte, randDataLen)
-	if _, err := rand.Read(b); err != nil {
-		return "", fmt.Errorf("failed to generate secret: %w", err)
+	if secretLength < minLength {
+		return "", fmt.Errorf("secret length must be at least %d", minLength)
 	}
 
-	randString := hex.EncodeToString(b)
-	return randString[:secretLength], nil
+	for range maxIterations {
+
+		randPassword := make([]byte, secretLength)
+		var hasLowerCase, hasUpperCase, hasNumber bool
+
+		for passwdCharIdx := range secretLength {
+			idx, err := rand.Int(rand.Reader, big.NewInt(int64(charsetLen)))
+			if err != nil {
+				return "", fmt.Errorf("failed to generate secret: %w", err)
+			}
+
+			passwdChar := charset[idx.Int64()]
+			if passwdChar >= 'a' && passwdChar <= 'z' {
+				hasLowerCase = true
+			} else if passwdChar >= 'A' && passwdChar <= 'Z' {
+				hasUpperCase = true
+			} else if passwdChar >= '0' && passwdChar <= '9' {
+				hasNumber = true
+			}
+
+			randPassword[passwdCharIdx] = passwdChar
+		}
+
+		if hasLowerCase && hasUpperCase && hasNumber {
+			return string(randPassword), nil
+		}
+	}
+
+	return "", fmt.Errorf("failed to generate secret: exceeded iterations - %d", maxIterations)
 }

internal/controller/common_test.go

@@ -21,42 +21,20 @@ import (
 )
 
 func TestGenerateRandomStringLength(t *testing.T) {
+	t.Run("Below minimum length returns error", func(t *testing.T) {
+		_, err := generateRandomString(15)
+		if err == nil {
+			t.Error("generateRandomString(15) expected error, got nil")
+		}
+	})
+
 	tests := []struct {
 		name   string
 		length int
 	}{
-		{
-			name:   "Zero length",
-			length: 0,
-		},
-		{
-			name:   "Odd length 1",
-			length: 1,
-		},
-		{
-			name:   "Even length 2",
-			length: 2,
-		},
-		{
-			name:   "Odd length 7",
-			length: 7,
-		},
-		{
-			name:   "Even length 8",
-			length: 8,
-		},
-		{
-			name:   "Odd length 15",
-			length: 15,
-		},
-		{
-			name:   "Even length 16",
-			length: 16,
-		},
-		{
-			name:   "Even length 32",
-			length: 32,
-		},
+		{name: "Min length 16", length: 16},
+		{name: "Even length 32", length: 32},
+		{name: "Odd length 33", length: 33},
 	}
 
 	for _, tt := range tests {
@@ -72,16 +50,34 @@ func TestGenerateRandomStringLength(t *testing.T) {
 	}
 }
 
-func TestGenerateRandomStringHexCharacters(t *testing.T) {
+func TestGenerateRandomStringCharacters(t *testing.T) {
 	result, err := generateRandomString(32)
 	if err != nil {
 		t.Fatalf("generateRandomString(32) unexpected error: %v", err)
 	}
+
+	var hasLower, hasUpper, hasDigit bool
 	for i, c := range result {
-		if (c < '0' || c > '9') && (c < 'a' || c > 'f') {
-			t.Errorf("generateRandomString(32) character at index %d is %q, not a lowercase hex character", i, c)
+		switch {
+		case c >= 'a' && c <= 'z':
+			hasLower = true
+		case c >= 'A' && c <= 'Z':
+			hasUpper = true
+		case c >= '0' && c <= '9':
+			hasDigit = true
+		default:
+			t.Errorf("generateRandomString(32) character at index %d is %q, not alphanumeric", i, c)
 		}
 	}
+	if !hasLower {
+		t.Error("generateRandomString(32) result has no lowercase letter")
+	}
+	if !hasUpper {
+		t.Error("generateRandomString(32) result has no uppercase letter")
+	}
+	if !hasDigit {
+		t.Error("generateRandomString(32) result has no digit")
+	}
 }
 
 func TestGenerateRandomStringUniqueness(t *testing.T) {