apache / trafficcontrol

Apache Traffic Control is an Open Source implementation of a Content Delivery Network
https://trafficcontrol.apache.org/
Apache License 2.0
1.06k stars 340 forks source link

Add Traffic Ops Delivery Service Name FQDN Validation #5658

Open rob05c opened 3 years ago

rob05c commented 3 years ago

I'm submitting a ...

Traffic Control components affected ...

Current behavior:

Traffic Ops validates that Delivery Service names (xmlId) don't contain spaces or periods:

https://github.com/apache/trafficcontrol/blob/6959ec/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go#L1328

But it doesn't verify it's a valid FQDN component.

Traffic Portal does, but Traffic Ops doesn't.

There is specifically an interest in prohibiting underscores, which we use in Header Rewrite files, e.g. hdr_rw_mid_foo.config. As-is, if an untrusted person were allowed to make Delivery Services, they could create a Delivery Service named "mid_foo" in order to attack and manipulate a different Delivery Service. This isn't a huge issue today, without Self Service, but it will be if and when we ever get there. Adding the validation now prevents the future security issue.

New behavior:

Traffic Ops is changed to validate Delivery Service names to only contain lowercase alphanumeric and hyphen characters.

Minimal reproduction of the problem with instructions:

POST a new Delivery Service to Traffic Ops with underscores in the xmlID

Anything else:

rawlinp commented 3 years ago

Since the xmlID is only used to create the HOST_REGEXP w/ order = 0, which actually determines the delivery FQDN, we should also validate the HOST_REGEXP similarly. But since that is a regex, I don't know if we can really limit it to alphanumeric and hyphen characters.

ocket8888 commented 3 years ago

Is this a duplicate of #3312? I know the title there is specifically about underscores, but it was the issue attached to the PR that added the validation to TP.

rob05c commented 3 years ago

It looks like #3312 is a subset of this. I'd vote we close that in favor of this.

ocket8888 commented 3 years ago

That's fine too