Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow untrusted root option on CreateCustomTLSCertificateInput #596

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jdw6359
Copy link

@jdw6359 jdw6359 commented Feb 6, 2025

Details

The fastly api for creating / uploading custom TLS certificates allows for passing in the allow_untrusted_root flag, indicating that a self signed CA (untrusted) is permitted to sign the server auth certificates that get uploaded.

The go-fastly client does not support this flag, presumably because exposing this option could potentially lead to negative consequences (someone uploading a self signed cert that browsers don't trust for a domain that is already seeing production traffic).

Without this change, we (SeatGeek) am unable to have a fully localized test environment leveraging a self signed CA that I create on the fly to test / validate our integration with the fastly custom tls cert upload process.

Rationale

  • This attribute allow_untrusted_root is available on the API, so it should also be present on the official sdk.
  • This attribute is off by default, with a full description of the drawbacks to using this flag.
  • Even if clients were to set this flag, this feature is still gated behind an account level flag that must be turned on by someone internal to Fastly.

Given the multiple layers of protection and the "default off" behavior of this flag, I think that this attribute can be safely reflected in the schema without consequence to customers

Without this, we wont be able to leverage the go-fastly client which would be a bummer.

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  • Does your submission pass tests?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully run tests with your changes locally?

User Impact

  • What is the user impact of this change?

This change is backwards compatible with existing behavior.

This change will allow clients to enable allow_untrusted_root when uploading a custom TLS cert

Are there any considerations that need to be addressed for release?

None

We have a situation where we would like to upload self-signed certs in a local development flow. The Fastly API supports this attribute, yet it is not present on the official sdk spec.

Although this field is potentially dangerous, it should still be exposed as an option.

In the current situation, we are forced to make a decision between not having a fully localized development flow and hand writing our fastly api interactions. If this change is accepted we can continue to use this fastly sdk as well as have a fully localized development flow.
@SahibYar
Copy link
Contributor

SahibYar commented Feb 6, 2025

While this PR is intended for local development, it introduces significant security risks, especially if misused in a production environment. If enabled in a production setting, it could allow self-signed or untrusted certificates to be uploaded, potentially leading to MITM (Man-in-the-Middle) attacks. TLS relies on certificates being signed by trusted CAs. Allowing self-signed certificates weakens this trust model, making it easier for attackers to introduce malicious certificates.

PR mentions an account-level flag that Fastly must enable, but it’s unclear how strictly this is enforced or who has control over it. may be @kpfleming can comment better. my concern is, if an attacker gains access to an account with this flag enabled, they could upload a rogue certificate.

May be you can

  1. try using mkcert for Local Certificates (as a safer alternative) for local development.
  2. create a separate fork for this.
  3. use a local reverse proxy (e.g., Nginx with a self-signed CA)

@kpfleming
Copy link
Contributor

Please refrain from tagging individuals (myself or any others) bring them into conversations. The Fastly team will respond to issues in this repository as we have time to do so.

@kpfleming
Copy link
Contributor

This attribute is only documented as part of the Bulk Certificates API, not the Custom Certificates API. If it is accepted by the Custom Certificates API that is either an error, or the API owners have not documented it. Given that, we can't add it to the CreateCustomTLSCertificate function, at least not today.

If you want this to be available in a properly documented fashion, you'll need to create a support ticket to request that; if it team ends up documenting this attribute as part of this API, then we can add it to go-fastly.

For what it's worth, I agree that if the attribute is part of the publicly documented API, there is no reason it cannot be included into go-fastly (it would probably be included in our generated API clients by default), regardless of any risks associated with using it.

@jdw6359
Copy link
Author

jdw6359 commented Feb 11, 2025

@kpfleming I have an enterprise case open with Fastly to take a look at this problem, it sounds like that is where the engineering team will decide if this field will be a part of the publicly documented API

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants