PMM-9320 MongoDB password escape special characters

[YaroslavPodorvanov]

PMM-9320

Build: SUBMODULES-2494

• PMM-9320 Unable to add MongoDB service due to + in the password #1003

Research: https://gitlab.com/go-yp/mongodb-escape-password-research/

[artemgavrilov]

Tests are failing

[artemgavrilov]

Why just not to use url.UserPassword(username, url.QueryEscape(password)) ?

[YaroslavPodorvanov]

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)
}

[artemgavrilov]

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?

[YaroslavPodorvanov]

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:"

[ritbl]

@YaroslavPodorvanov @artemgavrilov please take a look

[codecov]

Codecov Report

Merging #1098 (4d1e74a) into main (1dded39) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           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           

Flag	Coverage Δ
all	48.38% <100.00%> (+<0.01%)	⬆️
cover	48.77% <100.00%> (+<0.01%)	⬆️
crosscover	48.73% <100.00%> (+<0.01%)	⬆️
update	12.57% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
models/agent_model.go	87.88% <100.00%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1dded39...4d1e74a. Read the comment docs.