You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2021/05/14 19:04:14 UTC
[GitHub] [trafficcontrol] ocket8888 opened a new pull request #5851: Delivery Service Supported TLS Versions Blueprint
ocket8888 opened a new pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851
## What does this PR (Pull Request) do?
- [x] This PR is not related to any Issue
This PR adds a blueprint for setting the supported TLS versions for content requests on a per-Delivery Service basis.
## Which Traffic Control components are affected by this PR?
None
## What is the best way to verify this PR?
Read the blueprint.
## The following criteria are ALL met by this PR
- [x] Tests are unnecessary
- [x] Documentation is unnecessary
- [x] An update to CHANGELOG.md is not necessary
- [x] This PR includes any and all required license headers
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] srijeet0406 commented on a change in pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
srijeet0406 commented on a change in pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851#discussion_r640124196
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,358 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
Review comment:
Got you, makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851#discussion_r635604452
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,351 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
+is an invalid configuration because then HTTPS content cannot be served - and
+if that's the intention, the Delivery Service's Protocol should be modified
+such that it doesn't serve HTTPs content.
+
+Speaking of which, the only valid value for the `tlsVersions` of a Delivery
+Service with a Protocol set to `0` (which is called "HTTP" in Traffic Portal)
+is `null`.
+
+## Component Impact
+This change primarily concerns Traffic Ops, Traffic Portal, and
+ORT/T3C/atstccfg.
+
+Traffic Router should not concern itself with validating the TLS versions used
+by routing requests for HTTP-routed and STEERING/CLIENT STEERING Delivery
+Services. Doing so wouldn't significantly increase content security since that
+would all be enforced by the caching layers anyway (even in the case of an
+`HTTP_NO_CACHE`-Type Delivery Service) and would reqire invasive, complex
+changes as well as a headache-inducing web of edge cases to handle for STEERING
+and CLIENT STEERING routing.
+
+### Traffic Portal Impact
+Traffic Portal's Delivery Services forms (all types) will need to add two
+controls. First, a checkbox that controls whether TLS Version settings will
+exist on the Delivery Service. Secondly, a list of TLS Versions to be permitted
+for the Delivery Service - shown if and only if the aforementioned checkbox is
+checked. Below is a sample of what this would look like while the box is
+unchecked:
+
+
+
+... and while the box *is* checked:
+
+
+
+If desired, the markup used for that sample may be found in
+[Appendix A](#appendix-a).
+
+Traffic Portal should disallow the addition of a TLS version that is not a
+"whole match" of the regular expression `\d+\.\d+`, and should issue a warning
+if and when a TLS version is entered that is not known to exist (at the time of
+this writing, that would be any version besides 1.0, 1.1, 1.2, and 1.3). In
+keeping with the API restrictions, it MUST NOT disallow the addition of unknown
+protocol versions, to facilitate the ability of system administrators to
+quickly upgrade to a new protocol as it's released.
+
+A warning should also be issued if any Delivery Service is found to be a target
+of the Delivery Service being edited that does not support all of the TLS
+versions that the Delivery Service being edited itself does support, or if the
+Delivery Service being edited does not support all of the TLS versions supported
+by a Delivery Service of which it is a target, or if the Delivery Service being
+edited is the target of a Delivery Service which has other targets that support
+TLS versions not supported by the Delivery Service being edited.
+
+Traffic Poral should also display some kind of warning - possibly in a dialog
+box - whenever the "Restrict TLS Versions" box is checked, advising that
+clients using TLS versions not listed will become unable to retrieve the
+Delivery Service's content.
+
+Finally, Traffic Portal should display a warning whenever there are TLS versions
+enabled that exclude newer versions but specify older versions (where unknown
+versions are ignored). For example, at the time of this writing, the following
+TLS version sets should produce this warning:
+
+- 1.0
+- 1.0 and 1.1
+- 1.1
+- 1.0, 1.1, and 1.2
+- 1.1 and 1.2
+- 1.0 and 1.2
+
+### Traffic Ops Impact
+Traffic Ops will require changes to its API endpoints that return or manipulate
+Delivery Services and/or representations thereof. There are also a few database
+changes that are required, but client code should be generally unaffected,
+since it is using the the library structures that will already be changed as a
+result of changes to Traffic Ops itself.
+
+#### API Impact
+The endpoints that will need to be updated to reflect the new structure of a
+Delivery service are:
+
+- `/deliveryservices`
+- `/deliveryservices/{{ID}}`
+- `/deliveryservices/{{ID}}/safe`
+- `/deliveryservice_requests`
+- `/deliveryservice_requests/{{ID}}/assign`
+- `/deliveryservice_requests/{{ID}}/status`
+- `/servers/{{ID}}/deliveryservices`
+
+Endpoints relating to CDN Snapshots and/or monitoring configuration do not need
+changes, since those already do not represent Delivery Services in the same way
+as the rest of the API specifically because they omit data unnecessary for
+Traffic Monitor and Traffic Router, and the supported TLS versions of a
+Delivery Service do not affect monitoring or routing.
+
+As a Delivery Service's Supported TLS Versions are not considered a "safe"
+field, the only endpoints that will need changes beyond just the response
+format are:
+
+- `/deliveryservices`
+- `/deliveryservices/{{ID}}`
+- `/deliveryservice_requests`
+
+When a Delivery Service or Delivery Service Request is created or modified
+using one of these endpoints, the API MUST verify that the `tlsVersions`
+property is either `null` or an array of strings that match the regular
+expression `\d+\.\d+` (an empty array may be treated implicitly as `null` in
+request bodies, but MUST ALWAYS be emitted as `null` in responses). In the
Review comment:
Oh ok. That sounds fine
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851#discussion_r635600364
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,351 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
+is an invalid configuration because then HTTPS content cannot be served - and
+if that's the intention, the Delivery Service's Protocol should be modified
+such that it doesn't serve HTTPs content.
+
+Speaking of which, the only valid value for the `tlsVersions` of a Delivery
+Service with a Protocol set to `0` (which is called "HTTP" in Traffic Portal)
+is `null`.
+
+## Component Impact
+This change primarily concerns Traffic Ops, Traffic Portal, and
+ORT/T3C/atstccfg.
+
+Traffic Router should not concern itself with validating the TLS versions used
+by routing requests for HTTP-routed and STEERING/CLIENT STEERING Delivery
+Services. Doing so wouldn't significantly increase content security since that
+would all be enforced by the caching layers anyway (even in the case of an
+`HTTP_NO_CACHE`-Type Delivery Service) and would reqire invasive, complex
+changes as well as a headache-inducing web of edge cases to handle for STEERING
+and CLIENT STEERING routing.
+
+### Traffic Portal Impact
+Traffic Portal's Delivery Services forms (all types) will need to add two
+controls. First, a checkbox that controls whether TLS Version settings will
+exist on the Delivery Service. Secondly, a list of TLS Versions to be permitted
+for the Delivery Service - shown if and only if the aforementioned checkbox is
+checked. Below is a sample of what this would look like while the box is
+unchecked:
+
+
+
+... and while the box *is* checked:
+
+
+
+If desired, the markup used for that sample may be found in
+[Appendix A](#appendix-a).
+
+Traffic Portal should disallow the addition of a TLS version that is not a
+"whole match" of the regular expression `\d+\.\d+`, and should issue a warning
+if and when a TLS version is entered that is not known to exist (at the time of
+this writing, that would be any version besides 1.0, 1.1, 1.2, and 1.3). In
+keeping with the API restrictions, it MUST NOT disallow the addition of unknown
+protocol versions, to facilitate the ability of system administrators to
+quickly upgrade to a new protocol as it's released.
+
+A warning should also be issued if any Delivery Service is found to be a target
+of the Delivery Service being edited that does not support all of the TLS
+versions that the Delivery Service being edited itself does support, or if the
+Delivery Service being edited does not support all of the TLS versions supported
+by a Delivery Service of which it is a target, or if the Delivery Service being
+edited is the target of a Delivery Service which has other targets that support
+TLS versions not supported by the Delivery Service being edited.
Review comment:
I'm thinking the API should disallow it because it won't actually affect the client steering DS or its targets.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c commented on a change in pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851#discussion_r640141317
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,358 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
Review comment:
I'm not strictly a -1, but UI hacks like this are poor design. It shouldn't be possible for users to assign TLS versions to an HTTP service. If the system can't handle that, we should fix the system.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c commented on a change in pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851#discussion_r640142153
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,358 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
Review comment:
I realize that's not trivial, which is why I'm not -1'ing this. But I do wish we would prioritize fixing those footguns, instead of adding more hacks around them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851#discussion_r640123244
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,358 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
Review comment:
We can hide the controls, but because all Delivery Service "Types" are still the same "Type" of object we should not overwrite existing tlsVersions. For example, Traffic Router has issues with Delivery Services that don't have SSL keys but do support HTTPS, but you can't generate keys for a DS that doesn't exist. So, you could reasonably create a DS as HTTP-only with everything configured the way you want it, then generate the keys and switch it to use HTTPS
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp merged pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
rawlinp merged pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851#discussion_r635599120
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,351 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
+is an invalid configuration because then HTTPS content cannot be served - and
+if that's the intention, the Delivery Service's Protocol should be modified
+such that it doesn't serve HTTPs content.
+
+Speaking of which, the only valid value for the `tlsVersions` of a Delivery
+Service with a Protocol set to `0` (which is called "HTTP" in Traffic Portal)
+is `null`.
+
+## Component Impact
+This change primarily concerns Traffic Ops, Traffic Portal, and
+ORT/T3C/atstccfg.
+
+Traffic Router should not concern itself with validating the TLS versions used
+by routing requests for HTTP-routed and STEERING/CLIENT STEERING Delivery
+Services. Doing so wouldn't significantly increase content security since that
+would all be enforced by the caching layers anyway (even in the case of an
+`HTTP_NO_CACHE`-Type Delivery Service) and would reqire invasive, complex
+changes as well as a headache-inducing web of edge cases to handle for STEERING
+and CLIENT STEERING routing.
+
+### Traffic Portal Impact
+Traffic Portal's Delivery Services forms (all types) will need to add two
+controls. First, a checkbox that controls whether TLS Version settings will
+exist on the Delivery Service. Secondly, a list of TLS Versions to be permitted
+for the Delivery Service - shown if and only if the aforementioned checkbox is
+checked. Below is a sample of what this would look like while the box is
+unchecked:
+
+
+
+... and while the box *is* checked:
+
+
+
+If desired, the markup used for that sample may be found in
+[Appendix A](#appendix-a).
+
+Traffic Portal should disallow the addition of a TLS version that is not a
+"whole match" of the regular expression `\d+\.\d+`, and should issue a warning
+if and when a TLS version is entered that is not known to exist (at the time of
+this writing, that would be any version besides 1.0, 1.1, 1.2, and 1.3). In
+keeping with the API restrictions, it MUST NOT disallow the addition of unknown
+protocol versions, to facilitate the ability of system administrators to
+quickly upgrade to a new protocol as it's released.
+
+A warning should also be issued if any Delivery Service is found to be a target
+of the Delivery Service being edited that does not support all of the TLS
+versions that the Delivery Service being edited itself does support, or if the
+Delivery Service being edited does not support all of the TLS versions supported
+by a Delivery Service of which it is a target, or if the Delivery Service being
+edited is the target of a Delivery Service which has other targets that support
+TLS versions not supported by the Delivery Service being edited.
+
+Traffic Poral should also display some kind of warning - possibly in a dialog
+box - whenever the "Restrict TLS Versions" box is checked, advising that
+clients using TLS versions not listed will become unable to retrieve the
+Delivery Service's content.
+
+Finally, Traffic Portal should display a warning whenever there are TLS versions
+enabled that exclude newer versions but specify older versions (where unknown
+versions are ignored). For example, at the time of this writing, the following
+TLS version sets should produce this warning:
+
+- 1.0
+- 1.0 and 1.1
+- 1.1
+- 1.0, 1.1, and 1.2
+- 1.1 and 1.2
+- 1.0 and 1.2
+
+### Traffic Ops Impact
+Traffic Ops will require changes to its API endpoints that return or manipulate
+Delivery Services and/or representations thereof. There are also a few database
+changes that are required, but client code should be generally unaffected,
+since it is using the the library structures that will already be changed as a
+result of changes to Traffic Ops itself.
+
+#### API Impact
+The endpoints that will need to be updated to reflect the new structure of a
+Delivery service are:
+
+- `/deliveryservices`
+- `/deliveryservices/{{ID}}`
+- `/deliveryservices/{{ID}}/safe`
+- `/deliveryservice_requests`
+- `/deliveryservice_requests/{{ID}}/assign`
+- `/deliveryservice_requests/{{ID}}/status`
+- `/servers/{{ID}}/deliveryservices`
+
+Endpoints relating to CDN Snapshots and/or monitoring configuration do not need
+changes, since those already do not represent Delivery Services in the same way
+as the rest of the API specifically because they omit data unnecessary for
+Traffic Monitor and Traffic Router, and the supported TLS versions of a
+Delivery Service do not affect monitoring or routing.
+
+As a Delivery Service's Supported TLS Versions are not considered a "safe"
+field, the only endpoints that will need changes beyond just the response
+format are:
+
+- `/deliveryservices`
+- `/deliveryservices/{{ID}}`
+- `/deliveryservice_requests`
+
+When a Delivery Service or Delivery Service Request is created or modified
+using one of these endpoints, the API MUST verify that the `tlsVersions`
+property is either `null` or an array of strings that match the regular
+expression `\d+\.\d+` (an empty array may be treated implicitly as `null` in
+request bodies, but MUST ALWAYS be emitted as `null` in responses). In the
Review comment:
In requests they do. It's just more obvious in responses to use `null` IMO, because then you don't ever have to think about what it means for the collection of allowed TLS versions to be empty. `null` seems like a less "special" case than `[]`, if that makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851#discussion_r635598069
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,351 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
+is an invalid configuration because then HTTPS content cannot be served - and
+if that's the intention, the Delivery Service's Protocol should be modified
+such that it doesn't serve HTTPs content.
+
+Speaking of which, the only valid value for the `tlsVersions` of a Delivery
+Service with a Protocol set to `0` (which is called "HTTP" in Traffic Portal)
+is `null`.
+
+## Component Impact
+This change primarily concerns Traffic Ops, Traffic Portal, and
+ORT/T3C/atstccfg.
+
+Traffic Router should not concern itself with validating the TLS versions used
+by routing requests for HTTP-routed and STEERING/CLIENT STEERING Delivery
+Services. Doing so wouldn't significantly increase content security since that
+would all be enforced by the caching layers anyway (even in the case of an
+`HTTP_NO_CACHE`-Type Delivery Service) and would reqire invasive, complex
+changes as well as a headache-inducing web of edge cases to handle for STEERING
+and CLIENT STEERING routing.
+
+### Traffic Portal Impact
+Traffic Portal's Delivery Services forms (all types) will need to add two
+controls. First, a checkbox that controls whether TLS Version settings will
+exist on the Delivery Service. Secondly, a list of TLS Versions to be permitted
+for the Delivery Service - shown if and only if the aforementioned checkbox is
+checked. Below is a sample of what this would look like while the box is
+unchecked:
+
+
+
+... and while the box *is* checked:
+
+
+
+If desired, the markup used for that sample may be found in
+[Appendix A](#appendix-a).
+
+Traffic Portal should disallow the addition of a TLS version that is not a
+"whole match" of the regular expression `\d+\.\d+`, and should issue a warning
+if and when a TLS version is entered that is not known to exist (at the time of
+this writing, that would be any version besides 1.0, 1.1, 1.2, and 1.3). In
+keeping with the API restrictions, it MUST NOT disallow the addition of unknown
+protocol versions, to facilitate the ability of system administrators to
+quickly upgrade to a new protocol as it's released.
+
+A warning should also be issued if any Delivery Service is found to be a target
+of the Delivery Service being edited that does not support all of the TLS
+versions that the Delivery Service being edited itself does support, or if the
+Delivery Service being edited does not support all of the TLS versions supported
+by a Delivery Service of which it is a target, or if the Delivery Service being
+edited is the target of a Delivery Service which has other targets that support
+TLS versions not supported by the Delivery Service being edited.
Review comment:
So should it be possible to set `tlsVersions` to not `null` on STEERING/CLIENT_STEERING, or should the API just disallow it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851#discussion_r635573676
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,351 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
+is an invalid configuration because then HTTPS content cannot be served - and
+if that's the intention, the Delivery Service's Protocol should be modified
+such that it doesn't serve HTTPs content.
+
+Speaking of which, the only valid value for the `tlsVersions` of a Delivery
+Service with a Protocol set to `0` (which is called "HTTP" in Traffic Portal)
+is `null`.
+
+## Component Impact
+This change primarily concerns Traffic Ops, Traffic Portal, and
+ORT/T3C/atstccfg.
+
+Traffic Router should not concern itself with validating the TLS versions used
+by routing requests for HTTP-routed and STEERING/CLIENT STEERING Delivery
+Services. Doing so wouldn't significantly increase content security since that
+would all be enforced by the caching layers anyway (even in the case of an
+`HTTP_NO_CACHE`-Type Delivery Service) and would reqire invasive, complex
+changes as well as a headache-inducing web of edge cases to handle for STEERING
+and CLIENT STEERING routing.
+
+### Traffic Portal Impact
+Traffic Portal's Delivery Services forms (all types) will need to add two
+controls. First, a checkbox that controls whether TLS Version settings will
+exist on the Delivery Service. Secondly, a list of TLS Versions to be permitted
+for the Delivery Service - shown if and only if the aforementioned checkbox is
+checked. Below is a sample of what this would look like while the box is
+unchecked:
+
+
+
+... and while the box *is* checked:
+
+
+
+If desired, the markup used for that sample may be found in
+[Appendix A](#appendix-a).
+
+Traffic Portal should disallow the addition of a TLS version that is not a
+"whole match" of the regular expression `\d+\.\d+`, and should issue a warning
+if and when a TLS version is entered that is not known to exist (at the time of
+this writing, that would be any version besides 1.0, 1.1, 1.2, and 1.3). In
+keeping with the API restrictions, it MUST NOT disallow the addition of unknown
+protocol versions, to facilitate the ability of system administrators to
+quickly upgrade to a new protocol as it's released.
+
+A warning should also be issued if any Delivery Service is found to be a target
+of the Delivery Service being edited that does not support all of the TLS
+versions that the Delivery Service being edited itself does support, or if the
+Delivery Service being edited does not support all of the TLS versions supported
+by a Delivery Service of which it is a target, or if the Delivery Service being
+edited is the target of a Delivery Service which has other targets that support
+TLS versions not supported by the Delivery Service being edited.
+
+Traffic Poral should also display some kind of warning - possibly in a dialog
+box - whenever the "Restrict TLS Versions" box is checked, advising that
+clients using TLS versions not listed will become unable to retrieve the
+Delivery Service's content.
+
+Finally, Traffic Portal should display a warning whenever there are TLS versions
+enabled that exclude newer versions but specify older versions (where unknown
+versions are ignored). For example, at the time of this writing, the following
+TLS version sets should produce this warning:
+
+- 1.0
+- 1.0 and 1.1
+- 1.1
+- 1.0, 1.1, and 1.2
+- 1.1 and 1.2
+- 1.0 and 1.2
+
+### Traffic Ops Impact
+Traffic Ops will require changes to its API endpoints that return or manipulate
+Delivery Services and/or representations thereof. There are also a few database
+changes that are required, but client code should be generally unaffected,
+since it is using the the library structures that will already be changed as a
+result of changes to Traffic Ops itself.
+
+#### API Impact
+The endpoints that will need to be updated to reflect the new structure of a
+Delivery service are:
+
+- `/deliveryservices`
+- `/deliveryservices/{{ID}}`
+- `/deliveryservices/{{ID}}/safe`
+- `/deliveryservice_requests`
+- `/deliveryservice_requests/{{ID}}/assign`
+- `/deliveryservice_requests/{{ID}}/status`
+- `/servers/{{ID}}/deliveryservices`
+
+Endpoints relating to CDN Snapshots and/or monitoring configuration do not need
+changes, since those already do not represent Delivery Services in the same way
+as the rest of the API specifically because they omit data unnecessary for
+Traffic Monitor and Traffic Router, and the supported TLS versions of a
+Delivery Service do not affect monitoring or routing.
+
+As a Delivery Service's Supported TLS Versions are not considered a "safe"
+field, the only endpoints that will need changes beyond just the response
+format are:
+
+- `/deliveryservices`
+- `/deliveryservices/{{ID}}`
+- `/deliveryservice_requests`
+
+When a Delivery Service or Delivery Service Request is created or modified
+using one of these endpoints, the API MUST verify that the `tlsVersions`
+property is either `null` or an array of strings that match the regular
+expression `\d+\.\d+` (an empty array may be treated implicitly as `null` in
+request bodies, but MUST ALWAYS be emitted as `null` in responses). In the
+event that one or more entries in the array does not match the required
+pattern, the response should be in the form of a 400 Bad Request with an
+error-level Alert indicating the problematic entry(ies).
+
+Also, similar to Traffic Portal, Traffic Ops should emit warning-level alerts
+whenever a Delivery Service is created with or modified to have a `tlsVersions`
+property that is not `null`, indicating that this could cause problems for
+certain clients.
+
+It must also issue a warning-level Alert whenever the `tlsVersions` property is
+set such that it includes TLS versions not known to exist - for example at the
+time of this writing that would be in the case that it includes any element
+that is not exactly one of:
+
+- `"1.0"`
+- `"1.1"`
+- `"1.2"`
+- `"1.3"`
+
+Finally, a warning-level Alert is to be returned when the TLS versions exclude
+newer versions but include older versions. For example the values that would
+cause that warning to be issued today are:
+
+- `["1.0"]`
+- `["1.0", "1.1"]`
+- `["1.1"]`
+- `["1.0", "1.1", "1.2"]`
+- `["1.1", "1.2"]`
+- `["1.0", "1.2"]`
+
+(Note that ordering is not important.)
+
+#### Database Impact
+To implement the storage of a Delivery Service's supported TLS versions, a new
+table should be created like so:
+
+```text
+ Table "public.deliveryservice_supported_tls_version"
+ Column | Type | Collation | Nullable | Default
+-----------------+--------+-----------+----------+---------
+ deliveryservice | bigint | | not null |
+ tls_version | text | | not null |
+Check constraints:
+ "valid_tls_version" CHECK (tls_version ~ similar_to_escape('[0-9]+.[0-9]+'::text))
+Foreign-key constraints:
+ "deliveryservice_supported_tls_version_deliveryservice_fkey" FOREIGN KEY (deliveryservice) REFERENCES deliveryservice(id) ON UPDATE CASCADE ON DELETE CASCADE
+```
+
+Additionally, a migration will be needed to set the value of `tlsVersions` on
+existing Delivery Service Requests to `null`.
+
+## Documentation Impact
+The new field will need to be added to both the Delivery Services overview
+section as well as all of the API documentation for endpoints in which it will
+appear. Note also that the current documentation for the `PUT` method of
+`/deliveryservices/{{ID}}` incorrectly states that the response is empty,
+whereas it actually returns an array of Delivery Service representationns. The
Review comment:
sp: representationns
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,351 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
+is an invalid configuration because then HTTPS content cannot be served - and
+if that's the intention, the Delivery Service's Protocol should be modified
+such that it doesn't serve HTTPs content.
+
+Speaking of which, the only valid value for the `tlsVersions` of a Delivery
+Service with a Protocol set to `0` (which is called "HTTP" in Traffic Portal)
+is `null`.
+
+## Component Impact
+This change primarily concerns Traffic Ops, Traffic Portal, and
+ORT/T3C/atstccfg.
+
+Traffic Router should not concern itself with validating the TLS versions used
+by routing requests for HTTP-routed and STEERING/CLIENT STEERING Delivery
+Services. Doing so wouldn't significantly increase content security since that
+would all be enforced by the caching layers anyway (even in the case of an
+`HTTP_NO_CACHE`-Type Delivery Service) and would reqire invasive, complex
+changes as well as a headache-inducing web of edge cases to handle for STEERING
+and CLIENT STEERING routing.
+
+### Traffic Portal Impact
+Traffic Portal's Delivery Services forms (all types) will need to add two
+controls. First, a checkbox that controls whether TLS Version settings will
+exist on the Delivery Service. Secondly, a list of TLS Versions to be permitted
+for the Delivery Service - shown if and only if the aforementioned checkbox is
+checked. Below is a sample of what this would look like while the box is
+unchecked:
+
+
+
+... and while the box *is* checked:
+
+
+
+If desired, the markup used for that sample may be found in
+[Appendix A](#appendix-a).
+
+Traffic Portal should disallow the addition of a TLS version that is not a
+"whole match" of the regular expression `\d+\.\d+`, and should issue a warning
+if and when a TLS version is entered that is not known to exist (at the time of
+this writing, that would be any version besides 1.0, 1.1, 1.2, and 1.3). In
+keeping with the API restrictions, it MUST NOT disallow the addition of unknown
+protocol versions, to facilitate the ability of system administrators to
+quickly upgrade to a new protocol as it's released.
+
+A warning should also be issued if any Delivery Service is found to be a target
+of the Delivery Service being edited that does not support all of the TLS
+versions that the Delivery Service being edited itself does support, or if the
+Delivery Service being edited does not support all of the TLS versions supported
+by a Delivery Service of which it is a target, or if the Delivery Service being
+edited is the target of a Delivery Service which has other targets that support
+TLS versions not supported by the Delivery Service being edited.
Review comment:
TLS restrictions shouldn't apply to `CLIENT_STEERING/STEERING` since those won't affect TR anyways. So really it's just a matter of issuing a warning if target A and target B of the same client_steering/steering DS have different TLS restrictions.
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,351 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
+is an invalid configuration because then HTTPS content cannot be served - and
+if that's the intention, the Delivery Service's Protocol should be modified
+such that it doesn't serve HTTPs content.
+
+Speaking of which, the only valid value for the `tlsVersions` of a Delivery
+Service with a Protocol set to `0` (which is called "HTTP" in Traffic Portal)
+is `null`.
+
+## Component Impact
+This change primarily concerns Traffic Ops, Traffic Portal, and
+ORT/T3C/atstccfg.
+
+Traffic Router should not concern itself with validating the TLS versions used
+by routing requests for HTTP-routed and STEERING/CLIENT STEERING Delivery
+Services. Doing so wouldn't significantly increase content security since that
+would all be enforced by the caching layers anyway (even in the case of an
+`HTTP_NO_CACHE`-Type Delivery Service) and would reqire invasive, complex
+changes as well as a headache-inducing web of edge cases to handle for STEERING
+and CLIENT STEERING routing.
+
+### Traffic Portal Impact
+Traffic Portal's Delivery Services forms (all types) will need to add two
+controls. First, a checkbox that controls whether TLS Version settings will
+exist on the Delivery Service. Secondly, a list of TLS Versions to be permitted
+for the Delivery Service - shown if and only if the aforementioned checkbox is
+checked. Below is a sample of what this would look like while the box is
+unchecked:
+
+
+
+... and while the box *is* checked:
+
+
+
+If desired, the markup used for that sample may be found in
+[Appendix A](#appendix-a).
+
+Traffic Portal should disallow the addition of a TLS version that is not a
+"whole match" of the regular expression `\d+\.\d+`, and should issue a warning
+if and when a TLS version is entered that is not known to exist (at the time of
+this writing, that would be any version besides 1.0, 1.1, 1.2, and 1.3). In
+keeping with the API restrictions, it MUST NOT disallow the addition of unknown
+protocol versions, to facilitate the ability of system administrators to
+quickly upgrade to a new protocol as it's released.
+
+A warning should also be issued if any Delivery Service is found to be a target
+of the Delivery Service being edited that does not support all of the TLS
+versions that the Delivery Service being edited itself does support, or if the
+Delivery Service being edited does not support all of the TLS versions supported
+by a Delivery Service of which it is a target, or if the Delivery Service being
+edited is the target of a Delivery Service which has other targets that support
+TLS versions not supported by the Delivery Service being edited.
+
+Traffic Poral should also display some kind of warning - possibly in a dialog
+box - whenever the "Restrict TLS Versions" box is checked, advising that
+clients using TLS versions not listed will become unable to retrieve the
+Delivery Service's content.
+
+Finally, Traffic Portal should display a warning whenever there are TLS versions
+enabled that exclude newer versions but specify older versions (where unknown
+versions are ignored). For example, at the time of this writing, the following
+TLS version sets should produce this warning:
+
+- 1.0
+- 1.0 and 1.1
+- 1.1
+- 1.0, 1.1, and 1.2
+- 1.1 and 1.2
+- 1.0 and 1.2
+
+### Traffic Ops Impact
+Traffic Ops will require changes to its API endpoints that return or manipulate
+Delivery Services and/or representations thereof. There are also a few database
+changes that are required, but client code should be generally unaffected,
+since it is using the the library structures that will already be changed as a
+result of changes to Traffic Ops itself.
+
+#### API Impact
+The endpoints that will need to be updated to reflect the new structure of a
+Delivery service are:
+
+- `/deliveryservices`
+- `/deliveryservices/{{ID}}`
+- `/deliveryservices/{{ID}}/safe`
+- `/deliveryservice_requests`
+- `/deliveryservice_requests/{{ID}}/assign`
+- `/deliveryservice_requests/{{ID}}/status`
+- `/servers/{{ID}}/deliveryservices`
+
+Endpoints relating to CDN Snapshots and/or monitoring configuration do not need
+changes, since those already do not represent Delivery Services in the same way
+as the rest of the API specifically because they omit data unnecessary for
+Traffic Monitor and Traffic Router, and the supported TLS versions of a
+Delivery Service do not affect monitoring or routing.
+
+As a Delivery Service's Supported TLS Versions are not considered a "safe"
+field, the only endpoints that will need changes beyond just the response
+format are:
+
+- `/deliveryservices`
+- `/deliveryservices/{{ID}}`
+- `/deliveryservice_requests`
+
+When a Delivery Service or Delivery Service Request is created or modified
+using one of these endpoints, the API MUST verify that the `tlsVersions`
+property is either `null` or an array of strings that match the regular
+expression `\d+\.\d+` (an empty array may be treated implicitly as `null` in
+request bodies, but MUST ALWAYS be emitted as `null` in responses). In the
Review comment:
since it would never be valid to disable all TLS versions, why not just make `null` and `[]` both mean _no restrictions_?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851#discussion_r640162243
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,358 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
Review comment:
Breaking DSes out into separate types per Type (or at least routing Type) has been on my to-do list for well over a year now. It's a big ask.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] srijeet0406 commented on a change in pull request #5851: Delivery Service Supported TLS Versions Blueprint
Posted by GitBox <gi...@apache.org>.
srijeet0406 commented on a change in pull request #5851:
URL: https://github.com/apache/trafficcontrol/pull/5851#discussion_r640111178
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,358 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
+Delivery Service can have clients requesting its content over old, insecure
+versions of TLS, and Tenants controlling that Delivery Service have no way to
+force clients to use more recent versions.
+
+## Proposed Changes
+Changes to ATS in version 8.1 allow cache server configuration generation to
+produce configuration files that can effectively control the TLS versions
+allowed by a Delivery Service, causing clients using other versions to result
+in request failure. Currently, (in unreleased behavior on the master branch),
+this is implemented using a Delivery Service Profile Parameter. However,
+Parameters are notorious for being unvalidated, unstructured data that can be
+just obscure enough to escape notice, even when properly documented. For the
+best possible user, operator, and developer experiences, the data should be
+formally added to Delivery Service objects.
+
+## Data Model Impact
+Delivery Service objects will be augmented to include a set of allowed TLS
+versions for the Delivery Service's content. Specifically, They will be
+expanded to implement this interface:
+
+```typescript
+interface TLSVersionSpecifier {
+ tlsVersions: [string, ...Array<string>] | null;
+}
+```
+
+For those not super familiar with Typescript, this interface specifies a
+`tlsVersions` property that can be either null, or an array of N strings,
+where N is *at least one*. In other words, this can be null, but cannot be an
+empty array.
+
+The reason for this is best understood by first understanding what `null` means
+for this field. If `tlsVersions` is `null`, it means that no TLS version
+restrictions in place; the behavior is the same as today without the field, all
+TLS versions are implicitly supported.
+
+An empty array would seem to signify that no TLS versions are supported, which
Review comment:
Could we build this behavior into TP by default? Meaning the `TLSVersions` field is set to an empty array by default for `HTTP` delivery services, and is unmodifiable.
##########
File path: blueprints/delivery.service.tls.versions.md
##########
@@ -0,0 +1,358 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+# Delivery Service TLS Versions
+
+## Problem Description
+Currently, supprted TLS versions cannot be configured. This means that a
Review comment:
`supported`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org