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

configureTLS() does not handle map[string]interface{} that contains a boolean #51

Open
bnevis-i opened this issue Feb 1, 2021 · 1 comment

Comments

@bnevis-i
Copy link

bnevis-i commented Feb 1, 2021

The InitVaultKMS() function in vault/vault.go takes a map. One of the parameters that it can take is vaultCAVerify: (string-bool).

Example:

	kmsID := "vault-kms"
	config := map[string]interface{}{
		"vaultCAVerify":       "false",
	}
	secrets := map[string]string{}

	kms, err := InitVaultKMS(kmsID, config, secrets)

An upstream package, github.com/ceph/ceph-csi expects this value to be a string and returns the error failed to initialize Vault connection: configuration option not valid: expected string for "vaultCAVerify", but got bool if a boolean is supplied. The upstream project parses the string into a boolean and then inverts it, passing a bool to InitVaultKMS. (This can be considered its own bug and I will also file one there.)

The first few lines of configureTLS read:

func configureTLS(config *api.Config, secretConfig map[string]interface{}) error {
	tlsConfig := api.TLSConfig{}
	skipVerify := getVaultParam(secretConfig, api.EnvVaultInsecure)
	if skipVerify != "" {
		insecure, err := strconv.ParseBool(skipVerify)
		if err != nil {
			return ErrInvalidSkipVerify
		}
		tlsConfig.Insecure = insecure
	}

Clearly, getVaultParam() is expected to return a string, regardless of the type in secretConfig, which is declared as map[string]interface{}. A few lines later, ParseBool() is used to parse the value. In the failure scenario, skipVerify is returned as neither "true" nor "false" but the empty string.

This is caused by the following error in getVaultParam()

		tokenStr, ok := tokenIntf.(string)
		if !ok {
			return ""
		}

The cast to string does not stringize a boolean, but simply fails the cast.

Suggested remedy is to attempt cast to bool, and run strconv.FormatBool to return a string boolean.

@adityadani
Copy link
Collaborator

@bnevis-i Thanks for reporting this issue.

The getVaultParam() function is a misnomer since it always returns a string and could possibly be renamed to getVaultParamString(). For the api.EnvVaultInsecure parameter, I would suggest not using this function and instead directly handle the bool/string value.

Feel free to send out a PR to address this issue or we will fix it in a few days.

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

No branches or pull requests

2 participants