)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"29026cf50495f54959348f26b58f32448ee75596","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Matthew Oliver \u003cmatt@oliver.net.au\u003e"},{"line_number":5,"context_line":"CommitDate: 2021-11-11 16:47:48 +1100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix CVE-2017-8761"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I8665e416e419a31f096268413bbd71a01ac8e70a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"6b078692_6d17a4d1","line":7,"updated":"2021-11-11 19:38:08.000000000","message":"Let\u0027s crib some more info from the bug report, and add a Closes-Bug tag.","commit_id":"09314240b5626ea9b80008ee9021081af460d5f7"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6c922c406b1a5abc564800dfe1472ee71cca919e","unresolved":true,"context_lines":[{"line_number":18,"context_line":"the logs:"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"- if eventlet_debug is enabled in the proxy"},{"line_number":21,"context_line":"- if a tempurl sig/expires combination is invalid"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Co-Authored-By: Tim Burke \u003ctim.burke@gmail.com\u003e"},{"line_number":24,"context_line":"Closes-Bug: #1685798"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"0d082481_f1a522fe","line":21,"updated":"2021-11-11 23:04:48.000000000","message":"Feels like a weird (maybe dangerous) carve-out -- what if someone sends an invalid request (say, the expires param got truncated or something), then fix it up and re-send? The first line reveals the whole signature, then the 200 reveals that the first N signature chars match.","commit_id":"5e7fd8efd7076ef9198a04bb972cb4c9c286d126"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7ea2f9ff1b8c41569230d5d6bd76e5cc147d6236","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b2f89cae_e3b9d6c8","updated":"2021-11-11 05:51:40.000000000","message":"Pushing up a new version that takes the hash algorithm into account and checks un-urlencoded in the test.","commit_id":"09314240b5626ea9b80008ee9021081af460d5f7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1e20434f991823f713df753cdc81a231a7cbb6b7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4a267e8c_761ecb4d","updated":"2021-11-11 14:36:02.000000000","message":"recheck\n\n\u003e ERROR: Cannot install glance\u003d\u003d23.1.0.dev17 because these package versions have conflicting dependencies.\n","commit_id":"09314240b5626ea9b80008ee9021081af460d5f7"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"026012ef807a0211bc9cae2fcd208acacd09e2f8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"884b6df9_34a931bd","updated":"2021-12-22 05:30:55.000000000","message":"Here is an attempt at moving the obscuring over to the proxy_logging middleware: https://review.opendev.org/c/openstack/swift/+/822585","commit_id":"5e7fd8efd7076ef9198a04bb972cb4c9c286d126"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6c922c406b1a5abc564800dfe1472ee71cca919e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"5517d2dd_70d82a4b","updated":"2021-11-11 23:04:48.000000000","message":"I\u0027m starting to wonder if the fix really ought to be in proxy-logging -- it seems like our best bet to make sure we\u0027ve covered all possible cases.","commit_id":"5e7fd8efd7076ef9198a04bb972cb4c9c286d126"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"403b078752a0d8f9d0f80d0c216bc55249ca08c6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"4d7e9756_7b5d3541","updated":"2021-12-17 08:59:19.000000000","message":"Just having a quick play to see how something in proxy logging shakes out. Almost there, will have something up on my Monday.","commit_id":"5e7fd8efd7076ef9198a04bb972cb4c9c286d126"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fe04887ab1664ad38d6166f25b66fb95ed23cc08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a9408843_b4f336bb","updated":"2021-11-11 23:19:44.000000000","message":"Loooking good on first parse. Will pull down the code and run some tests.","commit_id":"5e7fd8efd7076ef9198a04bb972cb4c9c286d126"}],"swift/common/middleware/tempurl.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7ea2f9ff1b8c41569230d5d6bd76e5cc147d6236","unresolved":true,"context_lines":[{"line_number":595,"context_line":"        env[\u0027swift.authorize_override\u0027] \u003d True"},{"line_number":596,"context_line":"        env[\u0027REMOTE_USER\u0027] \u003d \u0027.wsgi.tempurl\u0027"},{"line_number":597,"context_line":"        qs \u003d {\u0027temp_url_sig\u0027:"},{"line_number":598,"context_line":"              temp_url_sig[:self.reveal_sensitive_prefix] + \u0027...\u0027,"},{"line_number":599,"context_line":"              \u0027temp_url_expires\u0027: temp_url_expires}"},{"line_number":600,"context_line":"        if temp_url_prefix is not None:"},{"line_number":601,"context_line":"            qs[\u0027temp_url_prefix\u0027] \u003d temp_url_prefix"}],"source_content_type":"text/x-python","patch_set":1,"id":"0050b13d_e2862a9f","line":598,"updated":"2021-11-11 05:51:40.000000000","message":"We need to take into account hash algorithms being in the temp_url, which is important info we\u0027d still like to see.","commit_id":"3655af59a15424407cc0aafeeb8ffd504bbb933a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"29026cf50495f54959348f26b58f32448ee75596","unresolved":true,"context_lines":[{"line_number":489,"context_line":""},{"line_number":490,"context_line":"        # Trim the signature to avoid revealing it in the logfiles"},{"line_number":491,"context_line":"        self.reveal_sensitive_prefix \u003d int("},{"line_number":492,"context_line":"            conf.get(\u0027reveal_sensitive_prefix\u0027, 8))"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    def __call__(self, env, start_response):"},{"line_number":495,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"c4bf8774_21cb946c","line":492,"updated":"2021-11-11 19:38:08.000000000","message":"We should update the sample config. In the description, we should also give some guidance on appropriate values, taking into account the different possible hash algorithms and encodings: 8 seems like a great default, anything under 32 offers at least *some* protection for all possible signatures, but if you want to reveal all signatures in full, you\u0027ll want something like 100.","commit_id":"09314240b5626ea9b80008ee9021081af460d5f7"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"5af2ceb8514101dc5f7874b00a72c9cd4d497097","unresolved":true,"context_lines":[{"line_number":504,"context_line":"        info \u003d self._get_temp_url_info(env)"},{"line_number":505,"context_line":"        temp_url_sig, temp_url_expires, temp_url_prefix, filename,\\"},{"line_number":506,"context_line":"            inline_disposition, temp_url_ip_range \u003d info"},{"line_number":507,"context_line":"        orig_temp_url_sig \u003d temp_url_sig"},{"line_number":508,"context_line":"        if temp_url_sig is None and temp_url_expires is None:"},{"line_number":509,"context_line":"            return self.app(env, start_response)"},{"line_number":510,"context_line":"        if not temp_url_sig or not temp_url_expires:"}],"source_content_type":"text/x-python","patch_set":2,"id":"43ed36eb_a582ff08","line":507,"updated":"2021-11-11 05:53:41.000000000","message":"We want to shrink the actual value sent in, whether it has a algoritm prefix or not. So taking a copy to sanitise and put back.","commit_id":"09314240b5626ea9b80008ee9021081af460d5f7"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"29026cf50495f54959348f26b58f32448ee75596","unresolved":true,"context_lines":[{"line_number":596,"context_line":"        env[\u0027swift.authorize_override\u0027] \u003d True"},{"line_number":597,"context_line":"        env[\u0027REMOTE_USER\u0027] \u003d \u0027.wsgi.tempurl\u0027"},{"line_number":598,"context_line":"        qs \u003d {\u0027temp_url_sig\u0027:"},{"line_number":599,"context_line":"              orig_temp_url_sig[:self.reveal_sensitive_prefix] + \u0027...\u0027,"},{"line_number":600,"context_line":"              \u0027temp_url_expires\u0027: temp_url_expires}"},{"line_number":601,"context_line":"        if temp_url_prefix is not None:"},{"line_number":602,"context_line":"            qs[\u0027temp_url_prefix\u0027] \u003d temp_url_prefix"}],"source_content_type":"text/x-python","patch_set":2,"id":"ccc8da74_8d1908f4","line":599,"updated":"2021-11-11 19:38:08.000000000","message":"We should probably only add the \u0027...\u0027 if we actually truncated.","commit_id":"09314240b5626ea9b80008ee9021081af460d5f7"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6c922c406b1a5abc564800dfe1472ee71cca919e","unresolved":true,"context_lines":[{"line_number":497,"context_line":""},{"line_number":498,"context_line":"        # Trim the signature to avoid revealing it in the logfiles"},{"line_number":499,"context_line":"        self.reveal_sensitive_prefix \u003d int("},{"line_number":500,"context_line":"            conf.get(\u0027reveal_sensitive_prefix\u0027, 8))"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def __call__(self, env, start_response):"},{"line_number":503,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a63dbf1_c2d5be4f","line":500,"updated":"2021-11-11 23:04:48.000000000","message":"Good -- matches the option name in proxy-logging. Part of me wonders whether the fix ought to be *there* -- I\u0027d expect its default of 16 should still be sufficient protection for tempurl sigs, plus it\u0027d be fairly easy to extend the fix to s3api\u0027s Signature/X-Amz-Signature query params.\n\nWhich reminds me, tempurl and pre-signed queries seem like they *really* don\u0027t interact very well with log_msg_template values that include\n\n {path.anonymized}","commit_id":"5e7fd8efd7076ef9198a04bb972cb4c9c286d126"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fcf1d877934fd2fa290d94dd076c190d5758484d","unresolved":true,"context_lines":[{"line_number":497,"context_line":""},{"line_number":498,"context_line":"        # Trim the signature to avoid revealing it in the logfiles"},{"line_number":499,"context_line":"        self.reveal_sensitive_prefix \u003d int("},{"line_number":500,"context_line":"            conf.get(\u0027reveal_sensitive_prefix\u0027, 8))"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def __call__(self, env, start_response):"},{"line_number":503,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"429b929d_f576c2d5","line":500,"in_reply_to":"7a63dbf1_c2d5be4f","updated":"2021-11-11 23:32:14.000000000","message":"How much middleware knowledge bleeding into proxy-logging would this cause.\nAnd it\u0027s in the querystring so it\u0027ll need to learn to look through it.\n\nIt does make sesne though. I wonder if it could grow an obscure list in the ENV that other middleware can pop what they want to be obscured maybe something like:\n\n   env[\u0027obscure_map\u0027] \u003d {\u0027params\u0027: [], \u0027headers\u0027: []}\n\nAnd whatever else in a requiest we may want to obscure in a req. Then the tempurl middleware can just drop a:\n\n  env.setdefault(\u0027obsucre_map\u0027, {}).setdefault(\u0027params\u0027 []).append(\u0027temp_url_sig\u0027)\n\n( or rather turn that ^ into a helper)\nAnd then the proxy_logging middleware can happily scrub all the things.","commit_id":"5e7fd8efd7076ef9198a04bb972cb4c9c286d126"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6c922c406b1a5abc564800dfe1472ee71cca919e","unresolved":true,"context_lines":[{"line_number":515,"context_line":"        info \u003d self._get_temp_url_info(env)"},{"line_number":516,"context_line":"        temp_url_sig, temp_url_expires, temp_url_prefix, filename,\\"},{"line_number":517,"context_line":"            inline_disposition, temp_url_ip_range \u003d info"},{"line_number":518,"context_line":"        self._scrub_signature(env)"},{"line_number":519,"context_line":"        orig_temp_url_sig \u003d temp_url_sig"},{"line_number":520,"context_line":"        if temp_url_sig is None and temp_url_expires is None:"},{"line_number":521,"context_line":"            return self.app(env, start_response)"}],"source_content_type":"text/x-python","patch_set":5,"id":"d3a7f15f_c0ee4dd4","line":518,"updated":"2021-11-11 23:04:48.000000000","message":"Might be worth calling out that this is for things like invalid requests, not happy path.","commit_id":"5e7fd8efd7076ef9198a04bb972cb4c9c286d126"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"fe04887ab1664ad38d6166f25b66fb95ed23cc08","unresolved":true,"context_lines":[{"line_number":607,"context_line":"                                                              container)"},{"line_number":608,"context_line":"        env[\u0027swift.authorize_override\u0027] \u003d True"},{"line_number":609,"context_line":"        env[\u0027REMOTE_USER\u0027] \u003d \u0027.wsgi.tempurl\u0027"},{"line_number":610,"context_line":"        qs \u003d {\u0027temp_url_sig\u0027: orig_temp_url_sig"},{"line_number":611,"context_line":"              if len(orig_temp_url_sig) \u003c\u003d self.reveal_sensitive_prefix else"},{"line_number":612,"context_line":"              orig_temp_url_sig[:self.reveal_sensitive_prefix] + \u0027...\u0027,"},{"line_number":613,"context_line":"              \u0027temp_url_expires\u0027: temp_url_expires}"},{"line_number":614,"context_line":"        if temp_url_prefix is not None:"},{"line_number":615,"context_line":"            qs[\u0027temp_url_prefix\u0027] \u003d temp_url_prefix"},{"line_number":616,"context_line":"        if filename:"},{"line_number":617,"context_line":"            qs[\u0027filename\u0027] \u003d filename"},{"line_number":618,"context_line":"        env[\u0027QUERY_STRING\u0027] \u003d urlencode(qs)"},{"line_number":619,"context_line":""},{"line_number":620,"context_line":"        def _start_response(status, headers, exc_info\u003dNone):"},{"line_number":621,"context_line":"            headers \u003d self._clean_outgoing_headers(headers)"}],"source_content_type":"text/x-python","patch_set":5,"id":"731ee1d7_72960f27","line":618,"range":{"start_line":610,"start_character":8,"end_line":618,"end_character":43},"updated":"2021-11-11 23:19:44.000000000","message":"So the only reason we rewrite the QUERY_STRING here I guess is to strip out unexpected extra params that could cause security issues. Like putting a symlink or getting a SLO manifest etc?\n\nBecause otherwise _scrub_signature is enough to, well scrub the signature.","commit_id":"5e7fd8efd7076ef9198a04bb972cb4c9c286d126"}],"test/unit/common/middleware/test_tempurl.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7ea2f9ff1b8c41569230d5d6bd76e5cc147d6236","unresolved":true,"context_lines":[{"line_number":150,"context_line":"        self.assertEqual(req.environ[\u0027swift.authorize_override\u0027], True)"},{"line_number":151,"context_line":"        self.assertEqual(req.environ[\u0027REMOTE_USER\u0027], \u0027.wsgi.tempurl\u0027)"},{"line_number":152,"context_line":"        trimmed_sig_qs \u003d \u0027temp_url_sig\u003d%s...\u0027 % sig[:8]"},{"line_number":153,"context_line":"        self.assertIn(trimmed_sig_qs, req.environ.get(\u0027QUERY_STRING\u0027))"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def test_get_valid(self):"},{"line_number":156,"context_line":"        method \u003d \u0027GET\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"61bf2d22_e7ed3ef9","line":153,"updated":"2021-11-11 05:51:40.000000000","message":"The querystring is urlencoded, so this doesn\u0027t match, this needs to change to:\n\n  trimmed_sig_qs \u003d \u0027%s...\u0027 % sig[:8]\n  self.assertEqual(req.param[\u0027temp_url_sig\u0027], trimmed_sig_qs)","commit_id":"3655af59a15424407cc0aafeeb8ffd504bbb933a"}]}