From 8daf2c00ba7f24b1d1f97f4525ad8779407f9f6d Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 22 Nov 2021 13:42:25 -0800 Subject: [PATCH 1/4] Respect WithWrappingToken option when secret ID is specified from string/env --- api/auth/approle/approle.go | 41 ++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/api/auth/approle/approle.go b/api/auth/approle/approle.go index ad0b371ae1de..b2e62cc42902 100644 --- a/api/auth/approle/approle.go +++ b/api/auth/approle/approle.go @@ -105,31 +105,34 @@ func (a *AppRoleAuth) Login(ctx context.Context, client *api.Client) (*api.Secre "role_id": a.roleID, } - if a.secretIDFile != "" { - secretIDValue, err := a.readSecretIDFromFile() + var secretIDValue string + + switch { + case a.secretIDFile != "": + s, err := a.readSecretIDFromFile() if err != nil { return nil, fmt.Errorf("error reading secret ID: %w", err) } - - // if it was indicated that the value in the file was actually a wrapping - // token, unwrap it first - if a.unwrap { - unwrappedToken, err := client.Logical().Unwrap(secretIDValue) - if err != nil { - return nil, fmt.Errorf("unable to unwrap token: %w. If the AppRoleAuth struct was initialized with the WithWrappingToken LoginOption, then the secret ID's filepath should be a path to a response-wrapping token", err) - } - loginData["secret_id"] = unwrappedToken.Data["secret_id"] - } else { - loginData["secret_id"] = secretIDValue + secretIDValue = s + case a.secretIDEnv != "": + s := os.Getenv(a.secretIDEnv) + if s == "" { + return nil, fmt.Errorf("secret ID was specified with an environment variable %q with an empty value", a.secretIDEnv) } - } else if a.secretIDEnv != "" { - secretIDValue := os.Getenv(a.secretIDEnv) - if secretIDValue == "" { - return nil, fmt.Errorf("secret ID was specified with an environment variable with an empty value") + secretIDValue = s + default: + secretIDValue = a.secretID + } + + // if the caller indicated that the value was actually a wrapping token, unwrap it first + if a.unwrap { + unwrappedToken, err := client.Logical().Unwrap(secretIDValue) + if err != nil { + return nil, fmt.Errorf("unable to unwrap response wrapping token: %w", err) } - loginData["secret_id"] = secretIDValue + loginData["secret_id"] = unwrappedToken.Data["secret_id"] } else { - loginData["secret_id"] = a.secretID + loginData["secret_id"] = secretIDValue } path := fmt.Sprintf("auth/%s/login", a.mountPath) From 09357cf0e8f949f1cc7a83b6660b4e923f32aa1b Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Mon, 22 Nov 2021 15:29:55 -0800 Subject: [PATCH 2/4] Add changelog --- changelog/13241.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/13241.txt diff --git a/changelog/13241.txt b/changelog/13241.txt new file mode 100644 index 000000000000..2f99db52cf56 --- /dev/null +++ b/changelog/13241.txt @@ -0,0 +1,3 @@ +```release-note:improvement +api: Respect WithWrappingToken() option during AppRole login authentication when used with secret ID specified from environment or from string +``` From c5851d25d0b9f18b221e4918871f929b884c7598 Mon Sep 17 00:00:00 2001 From: Anton Averchenkov <84287187+averche@users.noreply.github.com> Date: Mon, 22 Nov 2021 15:31:37 -0800 Subject: [PATCH 3/4] Update 13241.txt --- changelog/13241.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/13241.txt b/changelog/13241.txt index 2f99db52cf56..aa5ead908a3a 100644 --- a/changelog/13241.txt +++ b/changelog/13241.txt @@ -1,3 +1,3 @@ ```release-note:improvement -api: Respect WithWrappingToken() option during AppRole login authentication when used with secret ID specified from environment or from string +api: respect WithWrappingToken() option during AppRole login authentication when used with secret ID specified from environment or from string ``` From 748099d50f61a9e2ecfcee3ed2fe313fd0ce960d Mon Sep 17 00:00:00 2001 From: Anton Averchenkov Date: Tue, 23 Nov 2021 08:35:20 -0800 Subject: [PATCH 4/4] Update SecretID comments --- api/auth/approle/approle.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/api/auth/approle/approle.go b/api/auth/approle/approle.go index b2e62cc42902..61c380cd4fa6 100644 --- a/api/auth/approle/approle.go +++ b/api/auth/approle/approle.go @@ -22,13 +22,12 @@ type AppRoleAuth struct { var _ api.AuthMethod = (*AppRoleAuth)(nil) // SecretID is a struct that allows you to specify where your application is -// storing the secret ID required for login to the AppRole auth method. +// storing the secret ID required for login to the AppRole auth method. The +// recommended secure pattern is to use response-wrapping tokens rather than +// a plaintext value, by passing WithWrappingToken() to NewAppRoleAuth. +// https://learn.hashicorp.com/tutorials/vault/approle-best-practices?in=vault/auth-methods#secretid-delivery-best-practices type SecretID struct { - // Path on the file system where a trusted orchestrator has placed the - // application's secret ID. The recommended secure pattern is to use - // response-wrapping tokens rather than a plaintext value, by passing - // WithWrappingToken() to NewAppRoleAuth. - // https://learn.hashicorp.com/tutorials/vault/approle-best-practices?in=vault/auth-methods#secretid-delivery-best-practices + // Path on the file system where the secret ID can be found. FromFile string // The name of the environment variable containing the application's // secret ID.