Enforce strict teleport.yaml validation (#6520)

* Enforce strict teleport.yaml validation

Strict validation was added in warning mode in
https://github.com/gravitational/teleport/pull/5057 and released in 6.0.

For 7.0, we can drop the legacy custom validation logic, with the
assumption that all bad configs were migrated.

* Implement 'teleport configure --test' command

This command tests an existing config for errors.
This commit is contained in:
Andrew Lytvynov 2021-04-21 22:10:55 +00:00 committed by GitHub
parent b66bda89d0
commit 13eb433fc5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 23 additions and 214 deletions

View file

@ -19,7 +19,6 @@ package config
import (
"bytes"
"encoding/base64"
"errors"
"fmt"
"io"
"io/ioutil"
@ -46,150 +45,9 @@ import (
"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"gopkg.in/yaml.v2"
)
var (
// all possible valid YAML config keys
// true = has sub-keys
// false = does not have sub-keys (a leaf)
validKeys = map[string]bool{
"proxy_protocol": false,
"namespace": true,
"cluster_name": true,
"trusted_clusters": true,
"pid_file": true,
"cert_file": true,
"private_key_file": true,
"cert": true,
"private_key": true,
"checking_keys": true,
"checking_key_files": true,
"signing_keys": true,
"signing_key_files": true,
"allowed_logins": true,
"teleport": true,
"enabled": true,
"ssh_service": true,
"proxy_service": true,
"auth_service": true,
"kubernetes": true,
"kubeconfig_file": true,
"auth_token": true,
"auth_servers": true,
"domain_name": true,
"storage": false,
"nodename": true,
"log": true,
"period": true,
"connection_limits": true,
"max_connections": true,
"max_users": true,
"rates": true,
"commands": true,
"labels": false,
"output": true,
"severity": true,
"role": true,
"name": true,
"type": true,
"data_dir": true,
"web_listen_addr": true,
"tunnel_listen_addr": true,
"ssh_listen_addr": true,
"listen_addr": true,
"ca_cert_file": false,
"https_key_file": true,
"https_cert_file": true,
"advertise_ip": true,
"authorities": true,
"keys": true,
"reverse_tunnels": true,
"addresses": true,
"oidc_connectors": true,
"id": true,
"issuer_url": true,
"client_id": true,
"client_secret": true,
"redirect_url": true,
"acr_values": true,
"provider": true,
"tokens": true,
"region": true,
"table_name": true,
"access_key": true,
"secret_key": true,
"u2f": true,
"app_id": true,
"facets": true,
"device_attestation_cas": true,
"authentication": true,
"second_factor": false,
"oidc": true,
"display": false,
"scope": false,
"claims_to_roles": true,
"dynamic_config": false,
"seed_config": false,
"public_addr": false,
"ssh_public_addr": false,
"tunnel_public_addr": false,
"cache": true,
"ttl": false,
"issuer": false,
"permit_user_env": false,
"ciphers": false,
"kex_algos": false,
"mac_algos": false,
"ca_signature_algo": false,
"connector_name": false,
"session_recording": false,
"read_capacity_units": false,
"write_capacity_units": false,
"license_file": false,
"proxy_checks_host_keys": false,
"audit_table_name": false,
"audit_sessions_uri": false,
"audit_events_uri": false,
"pam": true,
"use_pam_auth": false,
"service_name": false,
"client_idle_timeout": false,
"session_control_timeout": false,
"disconnect_expired_cert": false,
"ciphersuites": false,
"ca_pin": false,
"keep_alive_interval": false,
"keep_alive_count_max": false,
"local_auth": false,
"enhanced_recording": false,
"command_buffer_size": false,
"disk_buffer_size": false,
"network_buffer_size": false,
"cgroup_path": false,
"kubernetes_service": true,
"kube_cluster_name": false,
"kube_listen_addr": false,
"kube_public_addr": false,
"app_service": true,
"db_service": true,
"protocol": false,
"uri": false,
"apps": false,
"databases": false,
"https_keypairs": true,
"key_file": false,
"insecure_skip_verify": false,
"rewrite": false,
"redirect": false,
"debug_app": false,
"acme": true,
"email": false,
"mysql_listen_addr": false,
}
)
var validCASigAlgos = []string{
ssh.SigAlgoRSA,
ssh.SigAlgoRSASHA2256,
@ -216,8 +74,6 @@ type FileConfig struct {
Databases Databases `yaml:"db_service,omitempty"`
}
type YAMLMap map[interface{}]interface{}
// ReadFromFile reads Teleport configuration from a file. Currently only YAML
// format is supported
func ReadFromFile(filePath string) (*FileConfig, error) {
@ -248,78 +104,13 @@ func ReadConfig(reader io.Reader) (*FileConfig, error) {
}
var fc FileConfig
// New validation in 6.0:
//
// Try strict unmarshal first (fails if any yaml entry doesn't map to a
// FileConfig field).
//
// If strict unmarshal failed, there may be some innocent mis-placed config
// fields. Fall back to the old validation first.
//
// If the old validation fails too, then we'll report the above error
// because the config is definitely invalid.
//
// If the old validation succeeds, we'll log the above error, but won't
// enforce it yet to let users fix the problem
strictUnmarshalErr := yaml.UnmarshalStrict(bytes, &fc)
if strictUnmarshalErr == nil {
// don't start Teleport with invalid ciphers, kex algorithms, or mac algorithms.
if err = fc.CheckAndSetDefaults(); err != nil {
return nil, trace.BadParameter("failed to parse Teleport configuration: %v", err)
}
return &fc, nil
if err := yaml.UnmarshalStrict(bytes, &fc); err != nil {
// Remove all newlines in the YAML error, to avoid escaping when printing.
return nil, trace.BadParameter("failed parsing the config file: %s", strings.Replace(err.Error(), "\n", "", -1))
}
// Remove all newlines in the YAML error, to avoid escaping when printing.
strictUnmarshalErr = errors.New(strings.Replace(strictUnmarshalErr.Error(), "\n", "", -1))
// DELETE IN 7.0: during 6.0, users should notice any issues that passed
// old validation but not the new strict one. With 7.0, we should always
// enforce the strict validation.
if err = yaml.Unmarshal(bytes, &fc); err != nil {
return nil, trace.BadParameter("failed to parse Teleport configuration: %v", strictUnmarshalErr)
}
// don't start Teleport with invalid ciphers, kex algorithms, or mac algorithms.
err = fc.CheckAndSetDefaults()
if err != nil {
if err := fc.CheckAndSetDefaults(); err != nil {
return nil, trace.BadParameter("failed to parse Teleport configuration: %v", err)
}
// now check for unknown (misspelled) config keys:
var validateKeys func(m YAMLMap) error
validateKeys = func(m YAMLMap) error {
var recursive, ok bool
var key string
for k, v := range m {
if key, ok = k.(string); ok {
if recursive, ok = validKeys[key]; !ok {
return trace.BadParameter("unrecognized configuration key: '%v'", key)
}
if recursive {
if m2, ok := v.(YAMLMap); ok {
if err := validateKeys(m2); err != nil {
return err
}
}
}
}
}
return nil
}
// validate configuration keys:
var tmp YAMLMap
if err = yaml.Unmarshal(bytes, &tmp); err != nil {
return nil, trace.BadParameter("error parsing YAML config: %v", err)
}
if err = validateKeys(tmp); err != nil {
// Both old an new validations failed. Report the new strict validation
// error.
return nil, trace.Wrap(strictUnmarshalErr)
}
// New strict validation failed but old one succeeded. There's something
// wrong with the config, but don't prevent it from starting up.
logrus.Errorf("Teleport configuration is invalid: %v.", strictUnmarshalErr)
logrus.Error("This error will be enforced in the next Teleport release.")
// Also add a short but noticeable sleep, to nudge users to pay attention
// to logs.
time.Sleep(5 * time.Second)
return &fc, nil
}

View file

@ -178,6 +178,7 @@ func Run(options Options) (executedCommand string, conf *service.Config) {
"Get automatic certificate from Letsencrypt.org using ACME.").BoolVar(&dumpFlags.ACMEEnabled)
dump.Flag("acme-email",
"Email to receive updates from Letsencrypt.org.").StringVar(&dumpFlags.ACMEEmail)
dump.Flag("test", "Path to a configuration file to test.").ExistingFileVar(&dumpFlags.testConfigFile)
// parse CLI commands+flags:
command, err := app.Parse(options.Args)
@ -253,10 +254,14 @@ func onStatus() error {
type dumpFlags struct {
config.SampleFlags
output string
output string
testConfigFile string
}
func (flags *dumpFlags) CheckAndSetDefaults() error {
if flags.testConfigFile != "" && flags.output != teleport.SchemeStdout {
return trace.BadParameter("only --output or --test can be set, not both")
}
if flags.output == "" || flags.output == teleport.SchemeFile {
flags.output = teleport.SchemeFile + "://" + defaults.ConfigFilePath
} else if flags.output == teleport.SchemeStdout {
@ -270,6 +275,19 @@ func onConfigDump(flags dumpFlags) error {
if err := flags.CheckAndSetDefaults(); err != nil {
return trace.Wrap(err)
}
if flags.testConfigFile != "" {
// Test an existing config.
_, err := config.ReadFromFile(flags.testConfigFile)
if err != nil {
fmt.Fprintf(os.Stderr, "FAIL %s\n", flags.testConfigFile)
return trace.Wrap(err)
}
fmt.Fprintf(os.Stderr, "OK %s\n", flags.testConfigFile)
return nil
}
// Generate a new config.
uri, err := url.Parse(flags.output)
if err != nil {
return trace.BadParameter("could not parse output value %q, use --output=%q",