Browse Source

Support RFC1738 quoted chars in passwords

In the bug, a user tried setting a devstack password with a "@" in it.

As it turns out, sqlalchmey turns the connection-string into a
sqlalchemy.engine.url.URL object [1] which returns a RFC1738 quoted
string.

However, alembic's set_main_option [2] uses python
string-interpolation which interprets '%' characters.  This means you
end up with an interpolation traceback when using any quoted character
(':@/') in a user/password (more likely password).

Avoid this by ensuring the URL is safe for python interpolation in
set_main_option by replacing '%' -> '%%'.

I convinced myself this is safe because sqlalchemy correctly parses
the quoted and unquoted versions just the same

---
 >>> str(sqlalchemy.engine.url.make_url('mysql+pymysql://foo:crazy:@/pw@/moo'))
 'mysql+pymysql://foo:crazy%3A%40%2Fpw@/moo'
 >>> str(sqlalchemy.engine.url.make_url('mysql+pymysql://foo:crazy%3A%40%2Fpw@/moo'))
 'mysql+pymysql://foo:crazy%3A%40%2Fpw@/moo'
---

A test is added

[1] https://github.com/zzzeek/sqlalchemy/blob/master/lib/sqlalchemy/engine/url.py
[2] http://alembic.zzzcomputing.com/en/latest/api/config.html#alembic.config.Config.set_main_option

Change-Id: I3ef7e3e539e35ce040573f2044ab6eb3c990200a
Closes-Bug: #1695299
Ian Wienand 1 year ago
parent
commit
f601cfccf1

+ 9
- 1
glance/db/sqlalchemy/alembic_migrations/__init__.py View File

@@ -35,7 +35,15 @@ def get_alembic_config(engine=None):
35 35
     config = alembic_config.Config(os.path.abspath(ini_path))
36 36
     if engine is None:
37 37
         engine = db_api.get_engine()
38
-    config.set_main_option('sqlalchemy.url', str(engine.url))
38
+    # str(sqlalchemy.engine.url.URL) returns a RFC-1738 quoted URL.
39
+    # This means that a password like "foo@" will be turned into
40
+    # "foo%40".  This causes a problem for set_main_option() here
41
+    # because that uses ConfigParser.set, which (by design) uses
42
+    # *python* interpolation to write the string out ... where "%" is
43
+    # the special python interpolation character!  Avoid this
44
+    # mis-match by quoting all %'s for the set below.
45
+    quoted_engine_url = str(engine.url).replace('%', '%%')
46
+    config.set_main_option('sqlalchemy.url', quoted_engine_url)
39 47
     return config
40 48
 
41 49
 

+ 13
- 0
glance/tests/unit/test_manage.py View File

@@ -21,9 +21,11 @@ from six.moves import StringIO
21 21
 
22 22
 from glance.cmd import manage
23 23
 from glance.common import exception
24
+from glance.db.sqlalchemy import alembic_migrations
24 25
 from glance.db.sqlalchemy import api as db_api
25 26
 from glance.db.sqlalchemy import metadata as db_metadata
26 27
 from glance.tests import utils as test_utils
28
+from sqlalchemy.engine.url import make_url as sqlalchemy_make_url
27 29
 
28 30
 
29 31
 class TestManageBase(test_utils.BaseTestCase):
@@ -166,6 +168,17 @@ class TestManage(TestManageBase):
166 168
         self.output = StringIO()
167 169
         self.useFixture(fixtures.MonkeyPatch('sys.stdout', self.output))
168 170
 
171
+    def test_db_complex_password(self):
172
+        engine = mock.Mock()
173
+        # See comments in get_alembic_config; make an engine url with
174
+        # password characters that will be escaped, to ensure the
175
+        # resulting value makes it into alembic unaltered.
176
+        engine.url = sqlalchemy_make_url(
177
+            'mysql+pymysql://username:pw@%/!#$()@host:1234/dbname')
178
+        alembic_config = alembic_migrations.get_alembic_config(engine)
179
+        self.assertEqual(str(engine.url),
180
+                         alembic_config.get_main_option('sqlalchemy.url'))
181
+
169 182
     @mock.patch('glance.db.sqlalchemy.api.get_engine')
170 183
     @mock.patch(
171 184
         'glance.db.sqlalchemy.alembic_migrations.data_migrations.'

Loading…
Cancel
Save