diff --git a/api/v1beta1/user_types.go b/api/v1beta1/user_types.go index 16f8f7ca..0a1cf3d1 100644 --- a/api/v1beta1/user_types.go +++ b/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"` } diff --git a/config/crd/bases/rabbitmq.com_users.yaml b/config/crd/bases/rabbitmq.com_users.yaml index b534d567..619c82a9 100644 --- a/config/crd/bases/rabbitmq.com_users.yaml +++ b/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: "" diff --git a/controllers/user_controller.go b/controllers/user_controller.go index 73decc7d..8c7eb340 100644 --- a/controllers/user_controller.go +++ b/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 { diff --git a/docs/api/rabbitmq.com.ref.asciidoc b/docs/api/rabbitmq.com.ref.asciidoc index 50b5c41a..6957e54b 100644 --- a/docs/api/rabbitmq.com.ref.asciidoc +++ b/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. |=== diff --git a/docs/examples/users/README.md b/docs/examples/users/README.md index c454b46e..29f4ecdd 100644 --- a/docs/examples/users/README.md +++ b/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. diff --git a/docs/examples/users/passwordlessUser.yaml b/docs/examples/users/passwordlessUser.yaml new file mode 100644 index 00000000..3b007213 --- /dev/null +++ b/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 diff --git a/docs/examples/users/userWithPasswordHash.yaml b/docs/examples/users/userWithPasswordHash.yaml new file mode 100644 index 00000000..33303242 --- /dev/null +++ b/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 diff --git a/internal/user_settings.go b/internal/user.go similarity index 57% rename from internal/user_settings.go rename to internal/user.go index 67dbcda2..c64cfb32 100644 --- a/internal/user_settings.go +++ b/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 } diff --git a/internal/user_settings_test.go b/internal/user_test.go similarity index 66% rename from internal/user_settings_test.go rename to internal/user_test.go index f454d829..07492c4e 100644 --- a/internal/user_settings_test.go +++ b/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()) + }) }) diff --git a/system_tests/user_system_test.go b/system_tests/user_system_test.go index b4ae859b..ea3de4f2 100644 --- a/system_tests/user_system_test.go +++ b/system_tests/user_system_test.go @@ -194,6 +194,7 @@ var _ = Describe("Users", func() { Expect(generatedSecret.Data).To(HaveKeyWithValue("password", []uint8("-grace.hopper_9453$"))) }) }) + When("providing a pre-defined username but autogenerated password", func() { var credentialSecret corev1.Secret BeforeEach(func() { @@ -246,4 +247,145 @@ var _ = Describe("Users", func() { Expect(generatedSecret.Data).To(HaveKey("password")) }) }) + + When("providing a predefined username & passwordHash", func() { + const ( + username = "`got*special_ch$racter5" + password = "S3cur3/P455" + ) + hash := rabbithole.Base64EncodedSaltedPasswordHashSHA512(password) + + var credentialSecret corev1.Secret + BeforeEach(func() { + credentialSecret = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "credential-list-secret", + Namespace: namespace, + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "username": []byte(username), + "passwordHash": []byte(hash), + "password": []byte("should$be_ignored"), + "some.irrelevant.key": []byte("some-useless-value"), + }, + } + Expect(k8sClient.Create(ctx, &credentialSecret, &client.CreateOptions{})).To(Succeed()) + user = &topology.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user-4", + Namespace: namespace, + }, + Spec: topology.UserSpec{ + RabbitmqClusterReference: topology.RabbitmqClusterReference{ + Name: rmq.Name, + }, + ImportCredentialsSecret: &corev1.LocalObjectReference{ + Name: credentialSecret.Name, + }, + }, + } + }) + AfterEach(func() { + Expect(k8sClient.Delete(context.Background(), &credentialSecret)).ToNot(HaveOccurred()) + Expect(k8sClient.Delete(context.Background(), user)).To(Succeed()) + }) + + It("declares a user successfully", func() { + By("declaring user") + Expect(k8sClient.Create(ctx, user, &client.CreateOptions{})).To(Succeed()) + + By("creating a new Secret with the provided credentials secret") + generatedSecretKey := types.NamespacedName{ + Name: "user-4-user-credentials", + Namespace: namespace, + } + var generatedSecret = &corev1.Secret{} + Eventually(func() error { + return k8sClient.Get(ctx, generatedSecretKey, generatedSecret) + }, 30, 2).Should(Succeed()) + Expect(generatedSecret.Data).To(HaveKeyWithValue("username", []uint8(username))) + Expect(generatedSecret.Data).To(HaveKeyWithValue("passwordHash", []uint8(hash))) + + By("ignoring the redundant password") + Expect(generatedSecret.Data).ToNot(HaveKey("password")) + + By("creating a user that can be authenticated with the original password") + var err error + managementEndpoint, err := managementEndpoint(ctx, clientSet, user.Namespace, user.Spec.RabbitmqClusterReference.Name) + Expect(err).NotTo(HaveOccurred()) + _, err = rabbithole.NewClient(managementEndpoint, username, password) + Expect(err).NotTo(HaveOccurred()) + }) + }) + + When("providing a predefined username & empty passwordHash", func() { + const ( + username = "`got*special_ch$racter5" + hash = "" + ignoredPassword = "should$be_ignored" + ) + + var credentialSecret corev1.Secret + BeforeEach(func() { + credentialSecret = corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "credential-list-secret", + Namespace: namespace, + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "username": []byte(username), + "passwordHash": []byte(hash), + "password": []byte(ignoredPassword), + "some.irrelevant.key": []byte("some-useless-value"), + }, + } + Expect(k8sClient.Create(ctx, &credentialSecret, &client.CreateOptions{})).To(Succeed()) + user = &topology.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user-5", + Namespace: namespace, + }, + Spec: topology.UserSpec{ + RabbitmqClusterReference: topology.RabbitmqClusterReference{ + Name: rmq.Name, + }, + ImportCredentialsSecret: &corev1.LocalObjectReference{ + Name: credentialSecret.Name, + }, + }, + } + }) + AfterEach(func() { + Expect(k8sClient.Delete(context.Background(), &credentialSecret)).ToNot(HaveOccurred()) + Expect(k8sClient.Delete(context.Background(), user)).To(Succeed()) + }) + + It("declares a passwordless user successfully", func() { + By("declaring user") + Expect(k8sClient.Create(ctx, user, &client.CreateOptions{})).To(Succeed()) + + By("creating a new Secret with the provided credentials secret") + generatedSecretKey := types.NamespacedName{ + Name: "user-5-user-credentials", + Namespace: namespace, + } + var generatedSecret = &corev1.Secret{} + Eventually(func() error { + return k8sClient.Get(ctx, generatedSecretKey, generatedSecret) + }, 30, 2).Should(Succeed()) + Expect(generatedSecret.Data).To(HaveKeyWithValue("username", []uint8(username))) + Expect(generatedSecret.Data).To(HaveKeyWithValue("passwordHash", []uint8(hash))) + + By("ignoring the redundant password") + Expect(generatedSecret.Data).ToNot(HaveKey("password")) + + By("creating a user with empty password hash") + var err error + user, err := rabbitClient.GetUser(username) + Expect(err).NotTo(HaveOccurred()) + Expect(user.PasswordHash).To(Equal("")) + }) + }) })