-
Notifications
You must be signed in to change notification settings - Fork 521
Add functions to generate Scram Credentials #102
Conversation
| func TestScramSha256SecretsMatch(t *testing.T) { | ||
| assertSecretsMatch(t, sha256.New, "Gy4ZNMr-SYEsEpAEZv", 15000, "ajdf1E1QTsNAQdBEodB4vzQOFuvcw9K6PmouVg==", "/pBk9XBwSm9UyeQmyJ3LfogfHu9Z/XTjGmRhQDHx/4I=", "Avm8mjtMyg659LAyeD4VmuzQb5lxL5iy3dCuzfscfMc=") | ||
| assertSecretsMatch(t, sha256.New, "Y9SPYSJYUJB_", 15000, "Oplsu3uju+lYyX4apKb0K6xfHpmFtH99Oyk4Ow==", "oTJhml8KKZUSt9k4tg+tS6D/ygR+a2Xfo8JKjTpQoAI=", "SUfA2+SKL35u665WY5NnJJmA9L5dHu/TnWXX/0nm42Y=") | ||
| assertSecretsMatch(t, sha256.New, "157VDZr0h-Pz-wj72", 15000, "P/4xs3anygxu3/l2p35CSBe4Z47IV/FtE/e44A==", "jOb27nFF72SQoY7WUqKXOTR4e8jETXxMS67SONrcbjA=", "3FnslkgUweautAfPRCOEjhS+YbUYUNmdDQUGxB+oaFE=") | ||
| assertSecretsMatch(t, sha256.New, "P8z1sDfELCePTNbVqX", 15000, "RPNhenwTHlqW5OE597XpuwvPLaiecPpYFa58Pg==", "sJ8UhQRszLNo15cOe62+HLjt2NxmSkJGjdJpclTIMBs=", "CSg02ODAvh9+swUHoimXcDsT9lLp/A5IhQXavXl7+qA=") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test cases are using the salt, stored key and sever keys of users created through the mongo shell. E.g.
db.createUser({user: "user4",
pwd: "P8z1sDfELCePTNbVqX",
roles: [ { role: "readWrite", db: "admin" } ]})
db.system.users.findOne({user: "user4"})
outputs
example-mongodb:PRIMARY> db.system.users.findOne({user: "user4"})
{
"_id" : "admin.user4",
"userId" : UUID("6b5aec1f-312b-491a-aeaa-b00cc374cf52"),
"user" : "user4",
"db" : "admin",
"credentials" : {
"SCRAM-SHA-1" : {
"iterationCount" : 10000,
"salt" : "Cd9RJHbdRt5b+kPf8YR0WQ==",
"storedKey" : "v3BB0CLs5AwLiAgIfE/yqDujoI4=",
"serverKey" : "yLqlYgg5GW04nAeBmFpgLU/vtto="
},
"SCRAM-SHA-256" : {
"iterationCount" : 15000,
"salt" : "RPNhenwTHlqW5OE597XpuwvPLaiecPpYFa58Pg==",
"storedKey" : "sJ8UhQRszLNo15cOe62+HLjt2NxmSkJGjdJpclTIMBs=",
"serverKey" : "CSg02ODAvh9+swUHoimXcDsT9lLp/A5IhQXavXl7+qA="
}
},
"roles" : [
{
"role" : "readWrite",
"db" : "admin"
}
]
}
rodrigovalin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a small comment to get rid of one function indirection, otherwise LGTM
| } | ||
|
|
||
| func ComputeScramSha256Creds(username, password string, salt []byte) (ScramCreds, error) { | ||
| return computeCreds(username, password, salt, sha256mechanismName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function only dispatches the request to computeCreds and then, there's branching in computeCreds depending on that..
Why not:
func ComputeScramSha256Creds(username, password string, salt []byte) (ScramCreds, error) {
base64EncodedSalt := base64.StdEncoding.EncodeToString(salt)
return computeScramCredentials(sha256.New, scramSha256Iterations, base64EncodedSalt, password)
}
func ComputeScramSha1Creds(username, password string, salt []byte) ScramCreds, error) {
base64EncodedSalt := base64.StdEncoding.EncodeToString(salt)
password = md5Hex(username + ":mongo:" + password)
return computeScramCredentials(sha1.New, scramSha1Iterations, base64EncodedSalt, password)
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is actually much nicer, as the Sha256 function doesn't actually require username 👍
All Submissions:
This PR moves the Credential generating logic from the enterprise code base to community. There are no functional changes. I've added additional unit tests for Sha256 secret generation.