Merge pull request #886 from NikSays/password-hash

Support PasswordHash in User type

api/v1beta1/user_types.go

@@ -27,12 +27,15 @@ type UserSpec struct {
 	// exist for the User object to be created.
 	// +kubebuilder:validation:Required
 	RabbitmqClusterReference RabbitmqClusterReference `json:"rabbitmqClusterReference"`
-	// Defines a Secret used to pre-define the username and password set for this User. User objects created
-	// with this field set will not have randomly-generated credentials, and will instead import
-	// the username/password values from this Secret.
-	// The Secret must contain the keys `username` and `password` in its Data field, or the import will fail.
-	// Note that this import only occurs at creation time, and is ignored once a password has been set
-	// on a User.
+	// Defines a Secret containing the credentials for the User. If this field is omitted, random a username and
+	// password will be generated. The Secret must have the following keys in its Data field:
+	//
+	//  * `username` – Must be present or the import will fail.
+	//  * `passwordHash` – The SHA-512 hash of the password. If the hash is an empty string, a passwordless user
+	//    will be created. For more information, see https://www.rabbitmq.com/docs/passwords.
+	//  * `password` – Plain-text password. Will be used only if the `passwordHash` key is missing.
+	//
+	// Note that this import only occurs at creation time, and is ignored once a password has been set on a User.
 	ImportCredentialsSecret *corev1.LocalObjectReference `json:"importCredentialsSecret,omitempty"`
 }
 

config/crd/bases/rabbitmq.com_users.yaml

@@ -43,12 +43,17 @@ spec:
             properties:
               importCredentialsSecret:
                 description: |-
-                  Defines a Secret used to pre-define the username and password set for this User. User objects created
-                  with this field set will not have randomly-generated credentials, and will instead import
-                  the username/password values from this Secret.
-                  The Secret must contain the keys `username` and `password` in its Data field, or the import will fail.
-                  Note that this import only occurs at creation time, and is ignored once a password has been set
-                  on a User.
+                  Defines a Secret containing the credentials for the User. If this field is omitted, random a username and
+                  password will be generated. The Secret must have the following keys in its Data field:
+
+
+                   * `username` – Must be present or the import will fail.
+                   * `passwordHash` – The SHA-512 hash of the password. If the hash is an empty string, a passwordless user
+                     will be created. For more information, see https://www.rabbitmq.com/docs/passwords.
+                   * `password` – Plain-text password. Will be used only if the `passwordHash` key is missing.
+
+
+                  Note that this import only occurs at creation time, and is ignored once a password has been set on a User.
                 properties:
                   name:
                     default: ""

controllers/user_controller.go

@@ -48,21 +48,31 @@ type UserReconciler struct {
 func (r *UserReconciler) declareCredentials(ctx context.Context, user *topology.User) (string, error) {
 	logger := ctrl.LoggerFrom(ctx)
 
-	username, password, err := r.generateCredentials(ctx, user)
+	credentials, err := r.generateCredentials(ctx, user)
 	if err != nil {
 		logger.Error(err, "failed to generate credentials")
 		return "", err
 	}
-	// Password wasn't in the provided input secret we need to generate a random one
-	if password == "" {
-		password, err = internal.RandomEncodedString(24)
+	// Neither PasswordHash nor Password wasn't in the provided input secret we need to generate a random password
+	if credentials.PasswordHash == nil && credentials.Password == "" {
+		credentials.Password, err = internal.RandomEncodedString(24)
 		if err != nil {
 			return "", fmt.Errorf("failed to generate random password: %w", err)
 		}
-
 	}
 
-	logger.Info("Credentials generated for User", "user", user.Name, "generatedUsername", username)
+	logger.Info("Credentials generated for User", "user", user.Name, "generatedUsername", credentials.Username)
+
+	credentialSecretData := map[string][]byte{
+		"username": []byte(credentials.Username),
+	}
+	if credentials.PasswordHash != nil {
+		// Create `passwordHash` field only if necessary, to distinguish between an unset hash and an empty one
+		credentialSecretData["passwordHash"] = []byte(*credentials.PasswordHash)
+	} else {
+		// Store password in the credential secret only if it will be used
+		credentialSecretData["password"] = []byte(credentials.Password)
+	}
 
 	credentialSecret := corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -72,10 +82,7 @@ func (r *UserReconciler) declareCredentials(ctx context.Context, user *topology.
 		Type: corev1.SecretTypeOpaque,
 		// The format of the generated Secret conforms to the Provisioned Service
 		// type Spec. For more information, see https://k8s-service-bindings.github.io/spec/#provisioned-service.
-		Data: map[string][]byte{
-			"username": []byte(username),
-			"password": []byte(password),
-		},
+		Data: credentialSecretData,
 	}
 
 	var operationResult controllerutil.OperationResult
@@ -102,10 +109,10 @@ func (r *UserReconciler) declareCredentials(ctx context.Context, user *topology.
 	}
 
 	logger.Info("Successfully declared credentials secret", "secret", credentialSecret.Name, "namespace", credentialSecret.Namespace)
-	return username, nil
+	return credentials.Username, nil
 }
 
-func (r *UserReconciler) generateCredentials(ctx context.Context, user *topology.User) (string, string, error) {
+func (r *UserReconciler) generateCredentials(ctx context.Context, user *topology.User) (internal.UserCredentials, error) {
 	logger := ctrl.LoggerFrom(ctx)
 
 	var err error
@@ -117,37 +124,48 @@ func (r *UserReconciler) generateCredentials(ctx context.Context, user *topology
 		return r.importCredentials(ctx, user.Spec.ImportCredentialsSecret.Name, user.Namespace)
 	}
 
-	username, err := internal.RandomEncodedString(24)
+	credentials := internal.UserCredentials{}
+
+	credentials.Username, err = internal.RandomEncodedString(24)
 	if err != nil {
-		return "", "", fmt.Errorf("failed to generate random username: %w", err)
+		return credentials, fmt.Errorf("failed to generate random username: %w", err)
 	}
-	password, err := internal.RandomEncodedString(24)
+	credentials.Password, err = internal.RandomEncodedString(24)
 	if err != nil {
-		return "", "", fmt.Errorf("failed to generate random password: %w", err)
+		return credentials, fmt.Errorf("failed to generate random password: %w", err)
 	}
-	return username, password, nil
+	return credentials, nil
 }
 
-func (r *UserReconciler) importCredentials(ctx context.Context, secretName, secretNamespace string) (string, string, error) {
+func (r *UserReconciler) importCredentials(ctx context.Context, secretName, secretNamespace string) (internal.UserCredentials, error) {
 	logger := ctrl.LoggerFrom(ctx)
 	logger.Info("Importing user credentials from provided Secret", "secretName", secretName, "secretNamespace", secretNamespace)
 
+	var credentials internal.UserCredentials
 	var credentialsSecret corev1.Secret
+
 	err := r.Client.Get(ctx, types.NamespacedName{Name: secretName, Namespace: secretNamespace}, &credentialsSecret)
 	if err != nil {
-		return "", "", fmt.Errorf("could not find password secret %s in namespace %s; Err: %w", secretName, secretNamespace, err)
+		return credentials, fmt.Errorf("could not find password secret %s in namespace %s; Err: %w", secretName, secretNamespace, err)
 	}
+
 	username, ok := credentialsSecret.Data["username"]
-	if !ok {
-		return "", "", fmt.Errorf("could not find username key in credentials secret: %s", credentialsSecret.Name)
+	if !ok || len(username) == 0 {
+		return credentials, fmt.Errorf("could not find username key in credentials secret: %s", credentialsSecret.Name)
 	}
-	password, ok := credentialsSecret.Data["password"]
-	if !ok {
-		return string(username), "", nil
+	credentials.Username = string(username)
+
+	password := credentialsSecret.Data["password"]
+	credentials.Password = string(password)
+
+	passwordHash, ok := credentialsSecret.Data["passwordHash"]
+	if ok {
+		credentials.PasswordHash = new(string)
+		*credentials.PasswordHash = string(passwordHash)
 	}
 
 	logger.Info("Retrieved credentials from Secret", "secretName", secretName, "retrievedUsername", string(username))
-	return string(username), string(password), nil
+	return credentials, nil
 }
 
 func (r *UserReconciler) setUserStatus(ctx context.Context, user *topology.User, username string) error {

docs/api/rabbitmq.com.ref.asciidoc

@@ -1411,12 +1411,17 @@ but cannot perform any management actions.
 For more information, see https://www.rabbitmq.com/management.html#permissions.
 | *`rabbitmqClusterReference`* __xref:{anchor_prefix}-github-com-rabbitmq-messaging-topology-operator-api-v1beta1-rabbitmqclusterreference[$$RabbitmqClusterReference$$]__ | Reference to the RabbitmqCluster that the user will be created for. This cluster must
 exist for the User object to be created.
-| *`importCredentialsSecret`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.22/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | Defines a Secret used to pre-define the username and password set for this User. User objects created
-with this field set will not have randomly-generated credentials, and will instead import
-the username/password values from this Secret.
-The Secret must contain the keys `username` and `password` in its Data field, or the import will fail.
-Note that this import only occurs at creation time, and is ignored once a password has been set
-on a User.
+| *`importCredentialsSecret`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.22/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | Defines a Secret containing the credentials for the User. If this field is omitted, random a username and
+password will be generated. The Secret must have the following keys in its Data field:
+
+
+ * `username` – Must be present or the import will fail.
+ * `passwordHash` – The SHA-512 hash of the password. If the hash is an empty string, a passwordless user
+   will be created. For more information, see https://www.rabbitmq.com/docs/passwords.
+ * `password` – Plain-text password. Will be used only if the `passwordHash` key is missing.
+
+
+Note that this import only occurs at creation time, and is ignored once a password has been set on a User.
 |===
 
 

docs/examples/users/README.md

@@ -1,8 +1,15 @@
 # User examples
 
-This section contains 3 examples for creating RabbitMQ users.
-Messaging Topology Operator creates users with generated credentials by default. To create RabbitMQ users with provided credentials, you can reference a kubernetes secret object contains keys `username` and `password` in its Data field.
-See [userPreDefinedCreds.yaml](./userPreDefinedCreds.yaml) and [publish-consume-user.yaml](./publish-consume-user.yaml) as examples.
+This section contains the examples for creating RabbitMQ users.
+
+Messaging Topology Operator creates users with generated credentials by default. To create RabbitMQ users with provided credentials, you can reference a kubernetes secret object with the following keys in its Data field:
+
+* `username` – Must be present or the import will fail.
+* `passwordHash` – The SHA-512 hash of the password, as described in [RabbitMQ Docs](https://www.rabbitmq.com/docs/passwords). If the hash is an empty string, a passwordless user will be created.
+* `password` – Plain-text password. Will be used only if the `passwordHash` key is missing.  
+
+See [userPreDefinedCreds.yaml](./userPreDefinedCreds.yaml), [userWithPasswordHash.yaml](userWithPasswordHash.yaml), [passwordlessUser.yaml](passwordlessUser.yaml) and [publish-consume-user.yaml](./publish-consume-user.yaml) as examples.
+
 From [Messaging Topology Operator v1.10.0](https://github.com/rabbitmq/messaging-topology-operator/releases/tag/v1.10.1), you can provide a username and reply on the Operator to generate its password for you.
 See [setUsernamewithGenPass.yaml](./setUsernamewithGenPass.yaml) as an example.
 

docs/examples/users/passwordlessUser.yaml

@@ -0,0 +1,22 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: credentials-secret
+type: Opaque
+stringData:
+  username: import-user-sample
+  passwordHash: "" # The user will not have a valid password. Login attempts with any password will be rejected
+  password: anythingreally # This value will be ignored, because `passwordHash` takes precedence
+---
+apiVersion: rabbitmq.com/v1beta1
+kind: User
+metadata:
+  name: import-user-sample
+spec:
+  tags:
+  - management # available tags are 'management', 'policymaker', 'monitoring' and 'administrator'
+  - policymaker
+  rabbitmqClusterReference:
+    name: test # rabbitmqCluster must exist in the same namespace as this resource
+  importCredentialsSecret:
+    name: credentials-secret

docs/examples/users/userWithPasswordHash.yaml

@@ -0,0 +1,21 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: credentials-secret
+type: Opaque
+stringData:
+  username: import-user-sample
+  passwordHash: SjWbNXaNEwcoOOZWxG6J1HCF5P83lUavsCto+wh1s9zdOfoZ/CPv6l/SSdK3RC2+1QWmJGdYt5740j3ZLf/0RbpusNc= # SHA-512 hash of "some-password"
+---
+apiVersion: rabbitmq.com/v1beta1
+kind: User
+metadata:
+  name: import-user-sample
+spec:
+  tags:
+  - management # available tags are 'management', 'policymaker', 'monitoring' and 'administrator'
+  - policymaker
+  rabbitmqClusterReference:
+    name: test # rabbitmqCluster must exist in the same namespace as this resource
+  importCredentialsSecret:
+    name: credentials-secret

internal/user_settings.go → internal/user.go

@@ -17,14 +17,37 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
+// UserCredentials describes the credentials that can be provided in ImportCredentialsSecret for a User.
+// If the secret is not provided, a random username and password will be generated.
+type UserCredentials struct {
+	// Must be present if ImportCredentialsSecret is provided.
+	Username string
+	// If PasswordHash is an empty string, a passwordless user is created.
+	// If PasswordHash is nil, Password is used instead.
+	PasswordHash *string
+	// If Password is empty and PasswordHash is nil, a random password is generated.
+	Password string
+}
+
 func GenerateUserSettings(credentials *corev1.Secret, tags []topology.UserTag) (rabbithole.UserSettings, error) {
 	username, ok := credentials.Data["username"]
 	if !ok {
 		return rabbithole.UserSettings{}, fmt.Errorf("could not find username in credentials secret %s", credentials.Name)
 	}
-	password, ok := credentials.Data["password"]
+
+	passwordHash, ok := credentials.Data["passwordHash"]
 	if !ok {
-		return rabbithole.UserSettings{}, fmt.Errorf("could not find password in credentials secret %s", credentials.Name)
+		// Use password as a fallback
+		password, ok := credentials.Data["password"]
+		if !ok {
+			return rabbithole.UserSettings{}, fmt.Errorf("could not find passwordHash or password in credentials secret %s", credentials.Name)
+		}
+		// To avoid sending raw passwords over the wire, compute a password hash using a random salt
+		// and use this in the UserSettings instead.
+		// For more information on this hashing algorithm, see
+		// https://www.rabbitmq.com/passwords.html#computing-password-hash.
+		passwordHashStr := rabbithole.Base64EncodedSaltedPasswordHashSHA512(string(password))
+		passwordHash = []byte(passwordHashStr)
 	}
 
 	var userTagStrings []string
@@ -33,13 +56,9 @@ func GenerateUserSettings(credentials *corev1.Secret, tags []topology.UserTag) (
 	}
 
 	return rabbithole.UserSettings{
-		Name: string(username),
-		Tags: userTagStrings,
-		// To avoid sending raw passwords over the wire, compute a password hash using a random salt
-		// and use this in the UserSettings instead.
-		// For more information on this hashing algorithm, see
-		// https://www.rabbitmq.com/passwords.html#computing-password-hash.
-		PasswordHash:     rabbithole.Base64EncodedSaltedPasswordHashSHA512(string(password)),
+		Name:             string(username),
+		Tags:             userTagStrings,
+		PasswordHash:     string(passwordHash),
 		HashingAlgorithm: rabbithole.HashingAlgorithmSHA512,
 	}, nil
 }

internal/user_settings_test.go → internal/user_test.go

@@ -27,13 +27,16 @@ var _ = Describe("GenerateUserSettings", func() {
 		userTags = []topology.UserTag{"administrator", "monitoring"}
 	})
 
-	It("generates the expected rabbithole.UserSettings", func() {
+	It("uses the password to generate the expected rabbithole.UserSettings", func() {
 		settings, err := internal.GenerateUserSettings(&credentialSecret, userTags)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(settings.Name).To(Equal("my-rabbit-user"))
 		Expect(settings.Tags).To(ConsistOf("administrator", "monitoring"))
 		Expect(settings.HashingAlgorithm.String()).To(Equal(rabbithole.HashingAlgorithmSHA512.String()))
 
+		// Password should not be sent, even if provided
+		Expect(settings.Password).To(BeEmpty())
+
 		// The first 4 bytes of the PasswordHash will be the salt used in the hashing algorithm.
 		// See https://www.rabbitmq.com/passwords.html#computing-password-hash.
 		// We can take this salt and calculate what the correct hashed salted value would
@@ -45,4 +48,19 @@ var _ = Describe("GenerateUserSettings", func() {
 		saltedHash := sha512.Sum512([]byte(string(salt) + "a-secure-password"))
 		Expect(base64.StdEncoding.EncodeToString([]byte(string(salt) + string(saltedHash[:])))).To(Equal(settings.PasswordHash))
 	})
+
+	It("uses the passwordHash to generate the expected rabbithole.UserSettings", func() {
+		hash, _ := rabbithole.SaltedPasswordHashSHA256("a-different-password")
+		credentialSecret.Data["passwordHash"] = []byte(hash)
+
+		settings, err := internal.GenerateUserSettings(&credentialSecret, userTags)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(settings.Name).To(Equal("my-rabbit-user"))
+		Expect(settings.Tags).To(ConsistOf("administrator", "monitoring"))
+		Expect(settings.HashingAlgorithm.String()).To(Equal(rabbithole.HashingAlgorithmSHA512.String()))
+		Expect(settings.PasswordHash).To(Equal(hash))
+
+		// Password should not be sent, even if provided
+		Expect(settings.Password).To(BeEmpty())
+	})
 })