peeringdb / peeringdb

Server code for https://www.peeringdb.com/
BSD 2-Clause "Simplified" License
340 stars 111 forks source link

When updating netixlan entry it seems we accept a non-validated status field content #1562

Open netravnen opened 2 months ago

netravnen commented 2 months ago

Describe the bug

A user reported trouble with updating netixlan entries for their network, after having updated the netíxlan entry via API calls.

The netixlan object is not available in the API (reported by the user) but is visible in the Web-UI.

% /usr/bin/curl \
  --config ${HOME}/.config/peeringdb/.curlconf \
  --get --location --silent \
  https://www.peeringdb.com/api/netixlan/62553

{"meta": {"error": "Not found."}}

DeskPro ticket 100003

Affected, networkixlan/12178 networkixlan/62553 networkixlan/70908

(I resolved the issue, see version object version history, asked the user to enhance this ticket with more information on what steps leading to the broken state)

To Reproduce

Unsure how to reproduce the error ATM.

Expected behavior

Status field should not be allowed to be set to anything other than [ok, pending, deleted], IMO.

Who is affected by the problem?

Users who (incorrectly?) define the status field to anything not equal [ok, pending, deleted] for the object they are updating via API calls.

What is the impact?

The user cannot update the object anymore. Object presumed not accessible to "consume" using the public API.

Are there security concerns?

Unknown. Does this fall in the ball-park of data injektion?

Are there privacy concerns?

None

What are the proposed actions?

Only accept status fields set to any value equal [ok, pending, deleted] when creating/patching/updating objects using API calls.

What is the proposed priority?

bugfix

Provide a rationale for any/all of the above

the report hints towards missing input validation for the status field when updating an existing netixlan object via API calls using authenticated API calls.

Additional context

image

image

image

Here I succeeded in updating the status of the netixlan using the PDB API.

% /usr/bin/curl \
  --config ${HOME}/.config/peeringdb/.curlconf \
  -X DELETE --location --silent \
  https://www.peeringdb.com/api/netixlan/62553

Succeeds changing the status to deleted. (Unexpected behavior, the as prior status is "Testing")

% /usr/bin/curl \
  --config ${HOME}/.config/peeringdb/.curlconf \
  --header "Content-Type: application/json" \
  --request POST \
  --location \
  --silent \
  --data "{ \"net_id\": 15731, \"ixlan_id\": 804, \"speed\": 10000, \"asn\": 36062, \"ipaddr4\": \"206.82.105.40\", \"ipaddr6\": \"2001:504:36::8cde:0:2\" }" \
  https://www.peeringdb.com/api/netixlan

And I succeed in changing the status from deleted to OK. (Expected behavior)

image

Not sure how the user was able to set the status to testing, As I cannot use POST or PUT requests to apply changes to the netixlan object.

{"meta": {"error": "Method \"PATCH\" not allowed."}}
{"meta": {"error": "Method \"PUT\" not allowed."}}

EDIT: Forgot to include the netixlan in the above request.

I can set the status field of the netixlan entry to something differently than [ok, pending, deleted] it seems (regardless of the operational field value, as first suspected, it just has to be either 'true' or 'false' (lowercase!)).

%  /usr/bin/curl \
  --config ${HOME}/.config/peeringdb/.curlconf \
  --header "Content-Type: application/json" \
  --request PUT \
  --location \
  --silent \
  --data "{ \"net_id\": 15731, \"ixlan_id\": 12, \"notes\": \"Testing\", \"speed\": 10000, \"asn\": 36062, \"ipaddr4\": \"198.32.118.205\", \"ipaddr6\": \"\", \"status\": \"Testing152\$??$213-.,_\", \"operational\": true }" \
  https://www.peeringdb.com/api/netixlan/12178

{"data": [{"id": 12178, "net_id": 15731, "net": {"id": 157(...)
% /usr/bin/curl \
  --config ${HOME}/.config/peeringdb/.curlconf \
  --header "Content-Type: application/json" \
  --request PUT \
  --location \
  --silent \
  --data "{ \"net_id\": 15731, \"ixlan_id\": 12, \"notes\": \"Testing\", \"speed\": 10000, \"asn\": 36062, \"ipaddr4\": \"198.32.118.205\", \"ipaddr6\": \"\", \"status\": \"Testing\", \"operational\": false }" \
  https://www.peeringdb.com/api/netixlan/12178

{"data": [{"id": 12178, "net_id": 15731, "net": {"id": 15731, "org_id": 18 (...)
image image
ianbarrere commented 2 months ago

If you just omit the operational field from a netixlan when making a put/post request it seems to do the trick:

body = {
    "net_id": 15731,
    "ixlan_id": 12,
    "notes": "Autogenerated entry from peering definition",
    "speed": 10000,
    "asn": 36062,
    "ipaddr4": "198.32.118.205",
    "ipaddr6": "",
    "status": "Testing",
    # "operational": True,
}
response = requests.put(
    url="https://www.peeringdb.com/api/netixlan/12178",
    headers=headers,
    data=body,
)

This call resulted in the entity not being found, but still existing in the backend somehow.

Thanks, Ian

netravnen commented 2 months ago

Side-Q Can this be enabled?

https://github.com/peeringdb/peeringdb/blob/f8b42c0fa09ff1ba7309d3114d0bc1da800fe057/peeringdb_server/models.py#L1741-L1742

netravnen commented 2 months ago

Proposed solution

Define a method for validation the status field regardless of the operational status.

Could be a modified function copied from validate_parent_status()?

https://github.com/peeringdb/peeringdb/blob/f8b42c0fa09ff1ba7309d3114d0bc1da800fe057/peeringdb_server/models.py#L280-L315