)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"8e64a1b5930d9409cf5e2379d0ecf8bd94093b1d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fde13932_8f2084ca","updated":"2025-04-18 16:25:23.000000000","message":"I don\u0027t know if this bug being open for ~8 years means it\u0027s not a very common issue or that it\u0027s somehow resolved itself?  Were you able to functionally reproduce?\n\nI think @tburke@nvidia.com is asking the right question:\n\nhttps://bugs.launchpad.net/swift/+bug/1657390/comments/5\n\n\u003e we could also get away with having swift\u0027s keystoneauth middleware drop any x-service-catalog headers it finds? I\u0027m not sure who uses it or where they would be in the pipeline, though.\n\nwhy does the proxy controller have to do this for the keystone mw?  Can we wrap that mw to not inject the massive header, or move it to the req.envion so it doesn\u0027t get forwarded to backend servers?\n\nIf we\u0027re going to go in the direction of the proxy app having to sanitize req headers that may have been received on the client request (or in this case added by mw) - it\u0027d be better to make an exhaustive white-list instead of ad-hoc blacklisting of headers from out-of-tree mw as we discover them causing problems.\n\nIf this is a big problem and this change is the best way to address it it would look nicer with a behavioral assertion on the removal of the header, presumably in test.unit.proxy.controllers.test_base","commit_id":"d841f9fa83f67584774120db5ee2803c9f1126bc"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"8f5561daf8019e67aab4c574fdff698d16df3d10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e6c79801_5ca612bd","updated":"2025-04-18 18:45:04.000000000","message":"Thanks for this review guys!\n\nIn the mean time i will create a change suggestion for the keystonemiddleware project.","commit_id":"d841f9fa83f67584774120db5ee2803c9f1126bc"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"e5bb4b4d9203d565fc65a0d2c6e3104c46d82dc7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8a3ed08a_b9bf06b1","updated":"2025-04-18 13:58:45.000000000","message":"The backends do not need the service catalog, so in requests toward a/c/o servers we should remove the service catalog from the headers.\n\nThe default is to not include the service catalog anyway, but for application credential acl\u0027s they are required.\n\nThis service catalog is then stored in the request headers, and copied to backend requests. The backend will fail with http 400 header line too long, unless you reconfigure all services.\n\nOther services in the proxy pipeline can still access and use the service catalog (if needed) as this change only affects backend requests.","commit_id":"d841f9fa83f67584774120db5ee2803c9f1126bc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"810aed545a2f8035a2e91933e3b02cc51026d5a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f35a67ac_c8bfb13a","in_reply_to":"8a3ed08a_b9bf06b1","updated":"2025-04-18 16:15:26.000000000","message":"\u003e Other services in the proxy pipeline can still access and use the service catalog (if needed)\n\nI\u0027m curious about this part -- do you know of other middlewares that need the service catalog and how they\u0027re typically configured? I hadn\u0027t thought of that immediately, so my gut reaction was to recommend popping it off in [`KeystoneAuth._keystone_identity`](https://github.com/openstack/swift/blob/2.35.0/swift/common/middleware/keystoneauth.py#L240-L241) which seems much more in-the-know regarding what headers keystonemiddleware might be adding.\n\nSide note: it weirds me out that Keystone uses headers for intra-pipeline communication at all -- [we\u0027ve seen issues with that before](https://bugs.launchpad.net/swift3/+bug/1561199). It seems like [`keystone.token_info`](https://opendev.org/openstack/keystonemiddleware/src/tag/10.9.0/keystonemiddleware/auth_token/__init__.py#L177-L180) might contain this, too -- I wonder if there\u0027s a deprecation plan...","commit_id":"d841f9fa83f67584774120db5ee2803c9f1126bc"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"8f5561daf8019e67aab4c574fdff698d16df3d10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"37f3475f_1778fd21","in_reply_to":"f35a67ac_c8bfb13a","updated":"2025-04-18 18:45:04.000000000","message":"\u003e do you know of other middlewares that need the service catalog and how they\u0027re typically configured? \n\nI do not, in our deployment the `include_service_catalog` option had been set to False (as per recommendation in the docs), but for the application credential ACL we now needed it to be included for the ACL to work in the [keystonemiddleware](https://github.com/openstack/keystonemiddleware/blob/master/keystonemiddleware/auth_token/__init__.py#L567)\n\nI merely suggested this, as there might be homebrew middleware out there that might rely on this being there...\n\n\u003e my gut reaction was to recommend popping it off in [`KeystoneAuth._keystone_identity`](https://github.com/openstack/swift/blob/2.35.0/swift/common/middleware/keystoneauth.py#L240-L241) which seems much more in-the-know regarding what headers keystonemiddleware might be adding.\n\nI had not thought of this yet, but that kind of makes more sense\n\n\u003e Side note: it weirds me out that Keystonemiddleware uses headers for intra-pipeline communication at all\n\nAs i mention in my other reply to Clay, i think this is some legacy backward compatible stuff, as it might have made sense 13 years ago (when there was no oslo.context lib yet) to store/cache this in the request header. So yeah, some deprecation plan would be nice 😊\n\nI seem to remember that a lot of services were spun up with those swift-like pipelines and `use:egg` definitions to enable or disable features in the api. Nowadays a lot of those projects (for example nova) have changed this in something entirely different..","commit_id":"d841f9fa83f67584774120db5ee2803c9f1126bc"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"8f5561daf8019e67aab4c574fdff698d16df3d10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0fb8afd6_9fdc5835","in_reply_to":"fde13932_8f2084ca","updated":"2025-04-18 18:45:04.000000000","message":"Hi, yes we were able to reproduce the issue, as it is eventlet.wsgi complaining about the header being too long. Initially we thought it was eventlet.wsgi on the proxy itself, but it turned out that it was the container or account server that complained about it.\n\nAnd i think the application credential ACL is a commonly used feature to be honest. So this bug being open for 8 years means probably it is not a common bug, as you need your catalog to be bigger than 8kb to start with. \n\nWith the introduction of application credentials (and the ACL you can set here, for example, to only allow a subset of containers for a specific application credential in the case of swift) the need of including the service catalog has only now become a [reality](https://github.com/openstack/keystonemiddleware/blob/master/keystonemiddleware/auth_token/__init__.py#L567) as part of the validation is to see if the service_type is included in the catalog.\n\nI do agree that setting the catalog in the headers seems kind of pointless, but (assuming here), when it was [introduced like 13 years ago](https://github.com/openstack/keystonemiddleware/commit/8f7d90fc1ab1ad4a5ffd03a23deae0ff8dd314f1) it was the option that made the most sense at the time i guess, as this information was then available in some context. As we always try to keep as much backward compatible as possible, this has undoubtedly happened here as well, giving it some unexpected results in this scenario.\n\n\nAs for filtering headers on proxy level; i already found code that we did this already, with the use of [`pass_through_headers`](https://github.com/openstack/swift/blob/master/swift/proxy/controllers/container.py#L40) in [`transfer_headers`](https://github.com/openstack/swift/blob/master/swift/proxy/controllers/container.py#L40), but with another review this time, i notice this is only used for other swift services (as transfer is not True for proxies, but rather the entire request headers are passed along as they are provided by the additional param)\n\n\nAs you mention, there are also other workarounds or potential fixes possible:\n- One could configure the max_header_size in the swift.conf file (though this needs to be configured on all a/c/o servers (so is quite the intrusive option), which also introduces some security concerns, as incoming headers are then also allowed to be bigger (unless you do not configure it on proxies...)\n\n- We could address this issue in keystonemiddleware and have it fixed there in some way (i like this option too though)\n\n\nLooking from another point of view, one could also argue that since the swift backend services do not use those headers, we could also filter [them](https://github.com/openstack/keystonemiddleware/blob/master/keystonemiddleware/auth_token/_request.py#L79) and the others out...\n\nI will create a change request in the keystonemiddleware, that does not do the catalog check, if the `include_service_catalog` conf option is set to False. As far as i know, this is a nice to have, but other than that not really required for application credential acl to work. (with the added benefit that this catalog is then also not generated anymore on the keystone side, saving generation and transfer time on those token validation requests)","commit_id":"d841f9fa83f67584774120db5ee2803c9f1126bc"}]}