-
Notifications
You must be signed in to change notification settings - Fork 39
PMM-9320 MongoDB password escape special characters #1098
PMM-9320 MongoDB password escape special characters #1098
Conversation
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.
Tests are failing
@@ -442,13 +442,14 @@ func (s *Agent) DSN(service *Service, dialTimeout time.Duration, database string | |||
} | |||
switch { | |||
case password != "": | |||
u.User = url.UserPassword(username, password) | |||
u.User = url.UserPassword(username, "__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.
Why just not to use url.UserPassword(username, url.QueryEscape(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.
Why just not to use
url.UserPassword(username, url.QueryEscape(password))
I'm research that solution too in https://gitlab.com/go-yp/mongodb-escape-password-research/:
SOLUTION | SUCCESS | TOTAL | COMMENT |
---|---|---|---|
0 | 36 | 38 | url.UserPassword(username, password) |
1 | 10 | 38 | url.UserPassword(username, url.QueryEscape(password)) |
2 | 35 | 38 | "__plus__" |
3 | 36 | 38 | "__password__" |
4 | 37 | 38 | ":__password__@" |
5 | 1 | 38 |
Generate 38 password with specials symbols:
var symbols = "" +
";,/?:@&=+$" +
"-_.!~*'()" +
"#" +
" " +
"%" +
"[]" +
"абвгґдеє" +
"\""
// one quote symbol ' failed always
for i, symbol := range symbols {
password := fmt.Sprintf("example%spassword", string(symbol))
userPasswords = append(userPasswords, [3]string{
"tester" + strconv.Itoa(i),
password,
"/tester_db",
})
}
test solutions:
func ping(username, password, db string, solution int) error {
u := &url.URL{
Scheme: "mongodb",
Host: "localhost:27017",
Path: db,
RawQuery: "connectTimeoutMS=2000&serverSelectionTimeoutMS=2000",
}
var (
dsn string
setAuth = false
)
switch solution {
// SOLUTION 0
// SOLUTION NOW EXISTS IN pmm-manager
// 36 / 38
case solutionExists:
u.User = url.UserPassword(username, password)
dsn = u.String()
// SOLUTION 1
// Artem's propose solution https://github.com/percona/pmm-managed/pull/1098#discussion_r861838935
// 10 / 38
case solutionArtem:
u.User = url.UserPassword(username, url.QueryEscape(password))
dsn = u.String()
// SOLUTION 2
// Another solution https://github.com/percona/pmm-managed/pull/1003/files
// 35 / 38
case solutionPR:
const plusPlaceholder = "__plus__" // There is a risk of error, when the password contains this string. Can change it to a more unique string.
password = strings.ReplaceAll(password, "+", plusPlaceholder)
u.User = url.UserPassword(username, password)
dsn = u.String()
dsn = strings.ReplaceAll(dsn, url.QueryEscape(plusPlaceholder), "%2B")
// SOLUTION 3
// 36 / 38
case solutionMy1:
u.User = url.UserPassword(username, "__password__")
dsn = u.String()
dsn = strings.Replace(dsn, "__password__", url.QueryEscape(password), 1)
// SOLUTION 4
// 37 / 38
case solutionMy2:
u.User = url.UserPassword(username, "__password__")
dsn = u.String()
dsn = strings.Replace(dsn, ":__password__@", ":"+url.QueryEscape(password)+"@", 1)
// SOLUTION 5
// 1 / 38
case solutionMy3:
dsn = u.String()
setAuth = true
}
ctx := context.Background()
opts := options.Client().ApplyURI(dsn)
if setAuth {
opts = opts.SetAuth(options.Credential{
Username: username,
Password: password,
})
}
client, err := mongo.Connect(ctx, opts)
if err != nil {
return err
}
defer client.Disconnect(ctx) //nolint:errcheck
if err = client.Ping(ctx, nil); err != nil {
return err
}
return nil
}
So main problem that in one sender side standard Go package "url" use one logic for stringify password to valid URL, in receiver side go.mongodb.org/mongo-driver/mongo use another logic for escape this password from URL.
In standard Go we have:
// String returns the encoded userinfo information in the standard form
// of "username[:password]".
func (u *Userinfo) String() string {
if u == nil {
return ""
}
s := escape(u.username, encodeUserPassword)
if u.passwordSet {
s += ":" + escape(u.password, encodeUserPassword)
}
return s
}
// Package url parses URLs and implements query escaping.
package url
escape(u.password, encodeUserPassword)
In go.mongodb.org/mongo-driver/mongo :
p.Password, err = url.QueryUnescape(password)
// Package url parses URLs and implements query escaping.
package url
func QueryUnescape(s string) (string, error) {
return unescape(s, encodeQueryComponent)
}
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.
Damn. Also it seems that we are not alone with this problem: https://kecci.medium.com/handle-mongo-connection-with-percent-encoding-in-go-c8b52b58094f
But what is more important that net/url
is not suitable for MongoDB URIs. Maybe we can use something that mongo driver uses instead of net/url
here?
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.
MongoDB driver use "go.mongodb.org/mongo-driver/x/mongo/driver/connstring", but connstring work only with parsing https://pkg.go.dev/go.mongodb.org/mongo-driver/x/mongo/driver/connstring
ConnString.String() not stringify URL
type ConnString struct {
Original string
// ...
}
func (p *parser) parse(original string) error {
p.Original = original
// ...
return nil
}
func (u *ConnString) String() string {
return u.Original
}
Maybe we can use something that mongo driver uses instead of
net/url
here?
No, ConnString cannot do it, and i did search in MongoDB driver by *.go "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.
@YaroslavPodorvanov @artemgavrilov please take a look
Codecov Report
@@ Coverage Diff @@
## main #1098 +/- ##
=======================================
Coverage 48.73% 48.73%
=======================================
Files 180 180
Lines 21134 21135 +1
=======================================
+ Hits 10299 10300 +1
Misses 9697 9697
Partials 1138 1138
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
PMM-9320
Build: SUBMODULES-2494
Research: https://gitlab.com/go-yp/mongodb-escape-password-research/