Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix password changing for SHA-1 mechanism #649 #650

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

makhovaa
Copy link

Pull Request (PR) description

Added the mechanism parameter for the password_hash command

 def password_hash=(_value)
    if db_ismaster
      command = {
        updateUser: @resource[:username],
        pwd: @resource[:password_hash],
        digestPassword: false
      }
      command[:mechanisms] = @resource[:auth_mechanism] == :scram_sha_1 ? ['SCRAM-SHA-1'] : ['SCRAM-SHA-256']

      mongo_eval("db.runCommand(#{command.to_json})", @resource[:database])
    else
      Puppet.warning 'User password operations are available only from master host'
    end
  end

Changed unit test for mongodb_user due to new expected line generated for password_hash command.

This Pull Request (PR) fixes the following issues

Fixes #649

Aleksandr Makhov added 4 commits October 27, 2022 13:22
According the ussie voxpupuli#649, the module ignore changes in passwrd hashes.
Add the mechanism parameter to the password_hash command
Changed unit test to mongodb_user due to new expected line generated for
password_hash command.
Copy link
Contributor

@poloz-lab poloz-lab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @makhovaa ,

I'm not a maintainer, just a user, but I have a little suggestion as I've worked on #643.

Thank's for contributing to this module.

@@ -103,6 +103,7 @@ def password_hash=(_value)
pwd: @resource[:password_hash],
digestPassword: false
}
command[:mechanisms] = @resource[:auth_mechanism] == :scram_sha_1 ? ['SCRAM-SHA-1'] : ['SCRAM-SHA-256']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'SCRAM-SHA-256' should not be possible.

This mechanism is not compatible with digestPassword: false.

@see https://www.mongodb.com/docs/v5.0/reference/command/updateUser/

The update of password_hash is not compatible with 'SCRAM-SHA-256'.

Copy link
Author

@makhovaa makhovaa Jan 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @poloz-lab ,
I agree with you, it should not be possible, but the module tries to use 256. I've just tested it with 4.2.0.

  1. I've changed password in hiera
  2. Run puppet agent and see the nest lines in debug output:
Debug: Executing: '/usr/bin/mongo unixtest_db --quiet --host 127.0.0.1:27017 --eval load('/root/.mongorc.js'); db.runCommand({"updateUser":"unixtest","pwd":"4c12585bf6d58e07be667e0b13bbf2eb","digestPassword":fals
e})'                                                                                                                                                                                                                
Notice: /Stage[main]/Tele2_mongodb/Mongodb::Db[unixtest_db]/Mongodb_user[User unixtest on db unixtest_db]/password_hash: defined 'password_hash' as '4c12585bf6d58e07be667e0b13bbf2eb' (corrective)
  1. Try new password:
# mongo -u test -p new_password TEST_DB
MongoDB shell version v5.0.9
connecting to: mongodb://127.0.0.1:27017/UNIXTEST_DB?compressors=disabled&gssapiServiceName=mongodb
Error: Authentication failed. :
connect@src/mongo/shell/mongo.js:372:17
@(connect):2:6
exception: connect failed
exiting with code 1
  1. The old password work.
  2. From the puppet run debug messages I catched the command which it uses to change password and format it regular bash:
# /usr/bin/mongo unixtest_db  --host 127.0.0.1:27017 --eval "load('/root/.mongorc.js'); db.runCommand({'updateUser':'unixtest','pwd':'4c12585bf6d58e07be667e0b13bbf2eb','digestPassword':false})"
MongoDB shell version v5.0.9
connecting to: mongodb://127.0.0.1:27017/unixtest_db?compressors=disabled&gssapiServiceName=mongodb
Implicit session: session { "id" : UUID("8b669dcf-57ec-4e21-ae5d-2304e484622c") }
MongoDB server version: 5.0.9
{
        "ok" : 0,
        "errmsg" : "Use of SCRAM-SHA-256 requires undigested passwords",
        "code" : 2,
        "codeName" : "BadValue",
        "$clusterTime" : {
                "clusterTime" : Timestamp(1673440333, 1),
                "signature" : {
                        "hash" : BinData(0,"5lUVoFtqmTq5zA6ZSHcZh71lBuk="),
                        "keyId" : NumberLong("7124297124161781766")
                }
        },
        "operationTime" : Timestamp(1673440333, 1)
}

But it works if I specify the mechanism:

# /usr/bin/mongo unixtest_db  --host 127.0.0.1:27017 --eval "load('/root/.mongorc.js'); db.runCommand({'updateUser':'unixtest','pwd':'4c12585bf6d58e07be667e0b13bbf2eb','digestPassword':false,'mechanisms':['SCRAM-SHA-1']})"
MongoDB shell version v5.0.9
connecting to: mongodb://127.0.0.1:27017/unixtest_db?compressors=disabled&gssapiServiceName=mongodb
Implicit session: session { "id" : UUID("2922dcab-a1e1-455a-84fb-51e50e17115f") }
MongoDB server version: 5.0.9
{
        "ok" : 1,
        "$clusterTime" : {
                "clusterTime" : Timestamp(1673440425, 1),
                "signature" : {
                        "hash" : BinData(0,"NlQ+MOKXhwZDtlTV7S/m8bYvA1g="),
                        "keyId" : NumberLong("7124297124161781766")
                }
        },
        "operationTime" : Timestamp(1673440425, 1)
}

So my changes just fix this issue by adding 'mechanisms':['SCRAM-SHA-1'] to the password changing command, but probably it can be fixed somewhere else.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @makhovaa ,

Yes you need to change it here.

But I was suggesting to not add the possibility to set 'SCRAM-SHA-256' in your ternary because as you see, we can not change the password when it's SCRAM-SHA-256.

Maybe it would be better to raise an error when we arrive there and we have the SCRAM-SHA-256 mechanism.

What do you think ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but the problem is I don't try to add SHA-256, The module just try to work with user credentials as if they are SHA-256. After looking through the module code I got the impression SHA-1 is a default mechanism, which must be used if user don't specify the alternative. But on practice, I have different and it tries to apply SHA-256:
"errmsg" : "Use of SCRAM-SHA-256 requires undigested passwords"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot change user's password
2 participants