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.

Sign up for GitHub

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

Jump to bottom

proposal: crypto/tls: on-disk configuration #60790

Closed
derekparker opened this issue Jun 14, 2023 · 20 comments
Closed

proposal: crypto/tls: on-disk configuration #60790

derekparker opened this issue Jun 14, 2023 · 20 comments
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone
Proposal

Comments

@derekparker
Copy link
Contributor

Abstract

This proposal introduces a new mechanism for configuring the standard library default crypto/tls.Config settings using a file on disk. The addition of this on-disk configuration mechanism enables consistent and auditable cryptographic policies to be deployed at scale. This configuration file would be scoped to only the configuration of the crypto/tls default ciphers, curves, and min/max TLS versions allowed. Allowing for this type of on-disk configuration will give those deploying Go applications using the standard library TLS stack greater control over the cryptographic settings used by an application, especially when the one deploying the software is not in direct control of the source code.

Background

Many operating systems allow for the creation of on-disk cryptographic policies which are then consumed by processes running on that system in order to apply default system-wide configuration [1][2]. Allowing for this type of centralized configuration enables those on the operations side to have peace of mind knowing that their systems are secure and auditable.

Proposal

This proposal describes both a way to read an on-disk configuration file and subsequently apply that configuration via the Config object by leveraging the unexported methods of that Config object in order to enforce the loaded cryptographic policy. This section will describe the technical details of the implementation.

Configuration format

The on-disk configuration file will be a JSON file, allowing use of the standard library encoding/json package to easily read and parse the file.

Locating the On-Disk Configuration

This proposal introduces two ways to specify the location of the on disk configuration file. The first would be by way of the environment variable GOTLSCONFIG having been set to a full path to the configuration file, which would then be used to set the value of a variable within the crypto/tls package to be used to load and parse the configuration. The second option is to specify the path directly via -ldflags=”-X crypto/tls.configFilepath=/path/to/config.json”.

Loading the Configuration From Disk

The configuration would be read and parsed using the encoding/json package and would be used to set an internal default configuration variable.

Using the Configuration

The internal configuration would be applied automatically to the lists of default curves, signature algorithms and cipher suites, like so:

// applyAlgorithmPolicyToDefault applies the policy to library defaults.
func applyAlgorithmPolicyToDefault() {
	defaultCurvePreferences =
		filterCurves(defaultCurvePreferences)

	defaultSupportedSignatureAlgorithms =
		filterSignatureAlgorithms(defaultSupportedSignatureAlgorithms)

	defaultCipherSuites =
		filterCipherSuites(defaultCipherSuites)
	defaultCipherSuitesLen = len(defaultCipherSuites)

	defaultCipherSuitesTLS13 =
		filterCipherSuitesTLS13(defaultCipherSuitesTLS13)
	defaultCipherSuitesTLS13NoAES =
		filterCipherSuitesTLS13(defaultCipherSuitesTLS13NoAES)

	if defaultFIPSCurvePreferences != nil {
		defaultFIPSCurvePreferences =
			filterCurves(defaultFIPSCurvePreferences)
	}
	if fipsSupportedSignatureAlgorithms != nil {
		fipsSupportedSignatureAlgorithms =
			filterSignatureAlgorithms(fipsSupportedSignatureAlgorithms)
	}
	if defaultCipherSuitesFIPS != nil {
		defaultCipherSuitesFIPS =
			filterCipherSuites(defaultCipherSuitesFIPS)
	}
}

The configuration would also be applied to any user-created config object via a method (c *Config) applyAlgorithmPolicy(). This would be called when initializing a new TLS server or client, and would be used to enforce the on-disk policy for custom config objects created by users.

Rationale

This approach is a non-invasive way to ensure any Go application using the standard library TLS implementation can be configured using an on-disk cryptographic configuration file. An alternative implementation would be to create an external package which could read and parse a configuration file and produce a corresponding tls.Config object, however that approach is only applicable to code you have access to modify. For those wishing to apply a consistent configuration across all applications on a given system, this alternative solution would still fall short.

Compatibility

This change would maintain backward compatibility by skipping any newly introduced code path in the absence of an on-disk configuration.

Implementation

There is a WIP implementation from a coworker that may be helpful to look at while considering this proposal [3].

The representation of the configuration would be:

type onDiskConfig struct {
	SupportedVersions               []string `json:"supportedVersions"`
	SupportedSignatureAlgorithms    []string `json:"supportedSignatureAlgorithms"`
	SupportedCurves                 []string `json:"supportedCurves"`
	SupportedKeyAgreementAlgorithms []string `json:"supportedKeyAgreementAlgorithms"`
	SupportedHashes                 []string `json:"supportedHashes"`
	SupportedCiphers                []string `json:"supportedCiphers"`
	SupportedMACs                   []string `json:"supportedMACs"`
}

Along with a parsed policy object:

// An algorithmPolicy defines a set of algorithms can be used in TLS,
// populated from the onDiskConfig struct.
type algorithmPolicy struct {
	versions               []uint16
	curves                 []CurveID
	signatureAlgorithms    []SignatureScheme
	keyAgreementAlgorithms []keyAgreementType
	hashes                 []crypto.Hash
	ciphers                []*cipherWithKeyLen
	macs                   []macType
	aeads                  []*aeadWithKeyLen
}

Then, within the crypto/tls package there would be an unexported variable named something along the lines of policy which would be a pointer to the above algorithmPolicy struct. If that variable is non-nil it will be used to provide default configuration values.

This policy would be loaded via an init function in the crypto/tls package, populating the aforementioned unexported variables. This would apply the policy to all internal defaults such as curves, signature algorithms and cipher suites. Additionally, when a user passes a config object into tls.Server or tls.Client, the implementation would call config.applyAlgorithmPolicy to ensure that the policy is enforced.

Open Issues

None.

Links

1: https://access.redhat.com/articles/3666211
2: https://manpages.ubuntu.com/manpages/focal/en/man7/crypto-policies.7.html
3: master...ueno:go:wip/tls-policy
@gopherbot gopherbot added this to the Proposal milestone Jun 14, 2023
@ianlancetaylor ianlancetaylor added the Proposal-Crypto Proposal related to crypto packages or other security issues label Jun 14, 2023
@ianlancetaylor
Copy link
Contributor

CC @golang/security

@rsc
Copy link
Contributor

rsc commented Dec 4, 2023

TLS reading from local disk seems like a non-starter.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rolandshoemaker
Copy link
Member

I think one of the nice things about crypto/tls currently is that its relatively easy to reason about, in part because there is an extremely limited set of knobs that can be turned by users (for better or worse). Something like this makes that a lot more complicated.

Similarly in the past we've had informal proposals about allowing programs to directly set the default TLS configuration (i.e. via a tls.SetDefaultConfig(*tls.Config) shape API), which I'm opposed to, since it again makes it a lot harder for users to figure out what is actually happening (did a package somewhere in my import graph set a default config? etc).

I think if we were to adopt something like this, it'd need to be strictly controlled by the person running the binary (i.e. via a GODEBUG or other env var), rather than in the source/build options. Perhaps if this is strictly focused on compliance type stuff, would it be reasonable to scope this only to FIPS mode?

@simo5
Copy link

simo5 commented Dec 4, 2023

This is something that Security Information Officers want to control, regardless of certifications, as a matter of deployment policy.

Being able to disable ciphers that may cause issues or are effected by a sudden vulnerability or are simply considered too weak for the specific deployment without assuming that the user has the sources and/or knows how to change them and/or has the ability to recompile the application is the capability that this proposal enables.

It is a somewhat basic security control most other TLS libraries (outside the Go ecosystem) offer.

@tomato42
Copy link

tomato42 commented Dec 4, 2023

There are two groups of people, on one hand you have the system administrators (IT staff) that are responsible for ensuring that the deployed systems work together, according to requirements (PCI-DSS, FIPS, CC), and on the other hand you have users, that sometimes do want to override the settings, but most often that not, just don't want to think about it.

And since go is fairly new, and with little baggage, there generally are no interoperability issues, so "language default" is generally good enough, but even now there are situations where that changes, like with RSA key exchange ciphersuites. There definitely will be deployments that do depend on them.

@rsc
Copy link
Contributor

rsc commented Dec 13, 2023

In addition to the disk concerns, the "Using the Configuration" section seems to change the defaults but does nothing for programs that are explicitly configuring other settings themselves. If this is about sysadmin security policy, it seems like it should be able to force specific settings, not just change defaults. I'm not sure what the path forward here is.

@simo5
Copy link

simo5 commented Dec 15, 2023

In addition to the disk concerns, the "Using the Configuration" section seems to change the defaults but does nothing for programs that are explicitly configuring other settings themselves. If this is about sysadmin security policy, it seems like it should be able to force specific settings, not just change defaults. I'm not sure what the path forward here is.

The admins set the expected policy, but applications generally always need to have the last say because they may have specific reason to make an exception.

In theory you may have a "default" policy and an "enforced" policy, but while having a "default" policy is sufficient, only having an "enforced" policy is detrimental as it will either force to relax it for all applications when an exception is needed or it breaks applications that need specific exceptions.

In my experience with Fedora/RHEL Crypto Policy, setting defaults is sufficient to raise the general level of security via policy without impacting adversely most applications.

@FiloSottile
Copy link
Contributor

FiloSottile commented Jan 9, 2024 •
edited

As @rolandshoemaker said, we try really hard to avoid "action at a distance" with global configuration changing the local behavior of packages. We also generally try to abstract from the environment to provide reproducible behavior across platforms and, although it is not always possible (i.e. Linux root stores), we're very reticent to do unexpected reads from disk at runtime. Such reads might actually be unexpected, and if they lead to relaxing the defaults might be seen as security-relevant unexpected behavior.

It is my understanding you already carry a set of patches to the standard library. Maybe we could move all the functions that determine the defaults of crypto/tls to a single, low-churn file such as src/crypto/tls/defaults.go so you can override that, and replace it with any local behavior that's expected on your platform, including reading from disk?

@simo5
Copy link

simo5 commented Jan 9, 2024

what if this was some sort of feature that package builders can opt in to ?

@FiloSottile
Copy link
Contributor

Since the behavior is likely to be different per-packager, and packagers usually have robust methods to apply patches, moving the defaults to src/crypto/tls/defaults.go feels like a good way to let packagers opt-in to exactly the behavior they want by patching that file, rather than maintain each distro-specific mechanism upstream.

@simo5
Copy link

simo5 commented Jan 15, 2024

I was not thinking about distro-specific behaviors, but one overloading behavior established by upstream but not active by default, activated via feature/config flag.

Basically whoever is building downstream can opt into it, but it is not enabled by default?
Is that a think in golang land?

Per distro patching is kind of last resort, and I am pretty sure there are ISVs that would like the behavior w/o having to patch their tooling to achieve it.

@derekparker
Copy link
Contributor Author

I was not thinking about distro-specific behaviors, but one overloading behavior established by upstream but not active by default, activated via feature/config flag.

Basically whoever is building downstream can opt into it, but it is not enabled by default? Is that a think in golang land?

This would basically be enabled via build tag.

Per distro patching is kind of last resort, and I am pretty sure there are ISVs that would like the behavior w/o having to patch their tooling to achieve it.

I agree, I think that this would be a viable solution and could be easily done downstream if this feature isn't enabled via an opt-in mechanism using build tags. I also appreciate the simplicity of Go by default and we don't want to disrupt that with this feature.

@rsc
Copy link
Contributor

rsc commented Jan 18, 2024

It sounds like moving the relevant definitions to defaults.go so that vendors can replace them more easily is the best we can do right now, in which case this would be a likely decline.

@rsc rsc mentioned this issue Jan 24, 2024
Open
@rsc
Copy link
Contributor

rsc commented Jan 24, 2024

Filed #65265 for creating defaults.go, which doesn't need to be a proposal.

@gopherbot
Copy link

Change https://go.dev/cl/558375 mentions this issue: crypto/tls: move defaults into defaults.go

@rsc
Copy link
Contributor

rsc commented Jan 26, 2024

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@cuishuang
Copy link
Contributor

Some default values are only used when building with the boringcrypto flag. Do we need another file for borrowing like default_boring.go?

@rsc
Copy link
Contributor

rsc commented Jan 31, 2024

If the defaults are currently //go:build boringcrypto then it's fine to have a defaults_boring.go as well.

@rsc
Copy link
Contributor

rsc commented Feb 1, 2024

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Proposals
Status: Declined
Milestone
Proposal
Development

No branches or pull requests
9 participants
@rsc @tomato42 @FiloSottile @derekparker @ianlancetaylor @rolandshoemaker @simo5 @gopherbot @cuishuang