Bug 2299208

Summary: Can't add or remove a single tag with a slash in it
Product: Red Hat OpenStack Reporter: Pierre Prinetti <pprinett>
Component: openstack-neutronAssignee: Rodolfo Alonso <ralonsoh>
Status: CLOSED MIGRATED QA Contact: Eran Kuris <ekuris>
Severity: low Docs Contact:
Priority: low    
Version: 17.1 (Wallaby)CC: chrisw, jlibosva, ralonsoh, scohen
Target Milestone: ---Keywords: Triaged
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2024-08-02 09:21:24 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Pierre Prinetti 2024-07-22 09:32:53 UTC
Description of problem:

The calls to add and remove an individual Neutron tag accept the tag as part of the URL path. However, a url-encoded slash character (as `%2F`) is interpreted as a literal slash (`/`) BEFORE path splitting:

```
curl -g -i -X PUT \
        'https://neutron.example:13696/v2.0/security-groups/51d6c739-dc9e-454e-bf72-54beb2afc5f8/tags/one%2Ftwo' \
        -H "X-Auth-Token: <token>"
HTTP/1.1 404 Not Found
content-length: 103
content-type: application/json
x-openstack-request-id: req-3d5911e5-10be-41e2-b83f-5b6ea5b0bbdf
date: Mon, 22 Jul 2024 08:57:16 GMT

{"NeutronError": {"type": "HTTPNotFound", "message": "The resource could not be found.", "detail": ""}}
```

Version-Release number of selected component (if applicable):


How reproducible:
100%

Steps to Reproduce:
1. Create a security group
2. Add a tag containing a slash (for example, to add `one/two` send "one%2Ftwo") using the documented call to add a single tag (https://docs.openstack.org/api-ref/network/v2/#add-a-tag) 

Actual results:
HTTP 404

Expected results:
Sending `one%2Ftwo` as the path argument adds the tag `one/two`.

Additional info:
Tested on PSI

Comment 1 Pierre Prinetti 2024-07-22 09:36:07 UTC
The openstackli works around the problem by GETting the resource and issuing a replace-all call (https://docs.openstack.org/api-ref/network/v2/#replace-all-tags) with the union of the existing tags and new tags. However, this solution introduces a data race when tags are edited concurrently by multiple agents.

Comment 2 Rodolfo Alonso 2024-07-22 11:35:59 UTC
Hello Pierre:

With curl "--data-urlencode" it is possible to pass URL encoded values to the request. But in this case the value is in the URL itself and the webob library is parsing the `%2F` character as part of the call path.

It could be possible to introduce a new API method (POST) to create a new tag, passing a value outside the path. However this change implies an API change and should be backported from U/S master to OSP18 and OSP17. Is it possible for you to avoid using tags with `/` character in OSP17? Is it a hard requirement for you?

Regards.

Comment 3 Pierre Prinetti 2024-07-22 12:25:32 UTC
I am a maintainer of Gophercloud, a library that exposes the OpenStack API for the Go programming language. So my case is rather about making sure that the API works in all conditions, rather than bringing a specific use-case that triggers the malfunction.

As they are, every use of the "add" or "delete" endpoints that use user-provided values instead of hardcoded constants is at risk of failure. I do understand that path splitting is performed in a middleware, but perhaps this is the kind of malfunction that warrants a reflection on said middleware? Maybe it can be configured differently, or maybe it can be changed?

Comment 4 Pierre Prinetti 2024-07-22 12:57:46 UTC
To be clear, what I am asking is not an API change.

The Neutron API already accepts url-encoded path arguments: in the RHOSP 17.1 cloud I am currently testing on, you can tag a security group with 'one%percent' by issuing this call:

```
$ curl -X PUT \
        'https://neutron.example:13696/v2.0/security-groups/51d6c739-dc9e-454e-bf72-54beb2afc5f8/tags/one%25percent' \
        -H 'X-Auth-Token: <token>'

$ openstack security group show 51d6c739-dc9e-454e-bf72-54beb2afc5f8 -c tags
+-------+-----------------+
| Field | Value           |
+-------+-----------------+
| tags  | ['one%percent'] |
+-------+-----------------+
```

What I am proposing, is for the URL decoding to happen after path-splitting, rather than before. The outcome would be that I can use "%2F" to mean "/" exactly like I can today use `%25` to mean "%", or "%2D" to mean "=".

Comment 5 Pierre Prinetti 2024-07-22 13:17:01 UTC
There also are cases where as of today URL-encoding is the only way to get what you want.

Example 1: URL-encoding is as of now the only way to atomically add a tag containing a space character (%20).

Example 2: a tag that contains something that collides with a URL-encoded symbol. Take 'no%3Acolumn': the only way you can set that tag is by URL-encoding it:

```
$ curl -X PUT \
        'https://neutron.example:13696/v2.0/security-groups/51d6c739-dc9e-454e-bf72-54beb2afc5f8/tags/no%3Acolumn' \
        -H 'X-Auth-Token: <token>'

$ openstack security group show 51d6c739-dc9e-454e-bf72-54beb2afc5f8 -c tags
+-------+---------------+
| Field | Value         |
+-------+---------------+
| tags  | ['no:column'] |
+-------+---------------+
```

```
$ curl -X PUT \
        'https://neutron.example:13696/v2.0/security-groups/51d6c739-dc9e-454e-bf72-54beb2afc5f8/tags/no%253Acolumn' \
        -H 'X-Auth-Token: <token>'

$ openstack security group show 51d6c739-dc9e-454e-bf72-54beb2afc5f8 -c tags
+-------+-----------------+
| Field | Value           |
+-------+-----------------+
| tags  | ['no%3Acolumn'] |
+-------+-----------------+
```

Comment 7 Pierre Prinetti 2024-08-01 14:08:43 UTC
The ideal resolution as I see it: with path-splitting done right, this bug is resolved in a perfectly backwards-compatible manner. Which means that it could be applied to any version allowed by non-security backport policies in place.

Is path-splitting performed by a downstream-only RHOSP middleware, or is this bug affecting upstream as well? In the latter case, I am happy to open a bug in Launchpad and close this one.

Comment 8 Rodolfo Alonso 2024-08-02 09:14:53 UTC
Hello Pierre:

The issue (if this is actually an issue) in the the webob or urllib3 libraries, I'm not 100% sure of what library is parsing the URL. But to be honest I don't know if that is actually a bug. In a HTTP request we can encode the data, that is not part of the URL itself. E.g.: https://unix.stackexchange.com/questions/86729/any-way-to-encode-the-url-in-curl-command. That means we can send any information outside the URL string and format it freely, including encoded slashes.

But I don't know if that is possible in the URL itself. According to https://datatracker.ietf.org/doc/html/rfc3986, there are a set of characters that are reserved in the URL string:
    reserved    = gen-delims / sub-delims
    gen-delims  = ":" / "/" / "?" / "#" / "[" / "]" / "@"
    sub-delims  = "!" / "$" / "&" / "'" / "(" / ")"
                / "*" / "+" / "," / ";" / "="

What I'm proposing in [1][2] is to have a new API to admit POST commands with parameters:
    # POST /v2.0/{parent_resource}/{parent_resource_id}/tags
    # body: {"tags": ["aaa", "bbb"]}

This implementation allows any character including slashes.

Regards.

[1]https://bugs.launchpad.net/neutron/+bug/2073836
[2]https://review.opendev.org/q/topic:%22bug/2073836%22