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

Add support for azure workload identity authentication #155

Merged
merged 3 commits into from
Sep 6, 2023
Merged

Add support for azure workload identity authentication #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2900448
Add support for azure workload identity authentication
cldmstr Nov 30, 2022
416120c
use passed context in setup
cldmstr Mar 24, 2023
07c6997
Add Work around an azure HTTP/2 bug (#183)
cldmstr Sep 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 45 additions & 69 deletions wrappers/azurekeyvault/azurekeyvault.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import (

"golang.org/x/net/http2"

"github.com/Azure/azure-sdk-for-go/services/keyvault/v7.1/keyvault"
"github.com/Azure/go-autorest/autorest"
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/azure-sdk-for-go/sdk/keyvault/azkeys"
"github.com/Azure/go-autorest/autorest/azure"
"github.com/Azure/go-autorest/autorest/azure/auth"
"github.com/Azure/go-autorest/autorest/to"
"github.com/hashicorp/go-hclog"
wrapping "github.com/hashicorp/go-kms-wrapping/v2"
Expand Down Expand Up @@ -52,7 +52,7 @@ type Wrapper struct {

environment azure.Environment
resource string
client *keyvault.BaseClient
client *azkeys.Client
logger hclog.Logger
keyNotRequired bool
baseURL string
Expand All @@ -77,7 +77,7 @@ func NewWrapper() *Wrapper {
// * Environment variable
// * Passed in config map
// * Managed Service Identity for instance
func (v *Wrapper) SetConfig(_ context.Context, opt ...wrapping.Option) (*wrapping.WrapperConfig, error) {
func (v *Wrapper) SetConfig(ctx context.Context, opt ...wrapping.Option) (*wrapping.WrapperConfig, error) {
opts, err := getOpts(opt...)
if err != nil {
return nil, err
Expand Down Expand Up @@ -173,14 +173,14 @@ func (v *Wrapper) SetConfig(_ context.Context, opt ...wrapping.Option) (*wrappin

if !v.keyNotRequired {
// Test the client connection using provided key ID
keyInfo, err := client.GetKey(context.Background(), v.baseURL, v.keyName, "")
keyInfo, err := client.GetKey(ctx, v.keyName, "", nil)
if err != nil {
return nil, fmt.Errorf("error fetching Azure Key Vault wrapper key information: %w", err)
}
if keyInfo.Key == nil {
return nil, errors.New("no key information returned")
}
v.currentKeyId.Store(ParseKeyVersion(to.String(keyInfo.Key.Kid)))
v.currentKeyId.Store(ParseKeyVersion(to.String((*string)(keyInfo.Key.KID))))
}

v.client = client
Expand Down Expand Up @@ -219,28 +219,28 @@ func (v *Wrapper) Encrypt(ctx context.Context, plaintext []byte, opt ...wrapping
if err != nil {
return nil, fmt.Errorf("error wrapping dat: %w", err)
}

// Encrypt the DEK using Key Vault
params := keyvault.KeyOperationsParameters{
Algorithm: keyvault.RSAOAEP256,
Value: to.StringPtr(base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(env.Key)),
algo := azkeys.JSONWebKeyEncryptionAlgorithmRSAOAEP256
params := azkeys.KeyOperationsParameters{
Algorithm: &algo,
Value: env.Key,
}
// Wrap key with the latest version for the key name
resp, err := v.client.WrapKey(ctx, v.buildBaseURL(), v.keyName, "", params)
resp, err := v.client.WrapKey(ctx, v.keyName, "", params, nil)
if err != nil {
return nil, err
}

// Store the current key version
keyVersion := ParseKeyVersion(to.String(resp.Kid))
keyVersion := ParseKeyVersion(resp.KID.Version())
v.currentKeyId.Store(keyVersion)

ret := &wrapping.BlobInfo{
Ciphertext: env.Ciphertext,
Iv: env.Iv,
KeyInfo: &wrapping.KeyInfo{
KeyId: keyVersion,
WrappedKey: []byte(to.String(resp.Result)),
WrappedKey: resp.Result,
},
}

Expand All @@ -258,40 +258,25 @@ func (v *Wrapper) Decrypt(ctx context.Context, in *wrapping.BlobInfo, opt ...wra
}

// Unwrap the key
params := keyvault.KeyOperationsParameters{
Algorithm: keyvault.RSAOAEP256,
Value: to.StringPtr(string(in.KeyInfo.WrappedKey)),
}
resp, err := v.client.UnwrapKey(ctx, v.buildBaseURL(), v.keyName, in.KeyInfo.KeyId, params)
wrappedBytes, err := base64.RawURLEncoding.DecodeString(string(in.KeyInfo.WrappedKey))
if err != nil {
return nil, err
// legacy unwrap as the key used to be stored base64 encoded and this is now handled in the json marshalling
// if it fails, the key is not encoded and can be used directly
wrappedBytes = in.KeyInfo.WrappedKey
}
algo := azkeys.JSONWebKeyEncryptionAlgorithmRSAOAEP256
params := azkeys.KeyOperationsParameters{
Algorithm: &algo,
Value: wrappedBytes,
}

keyBytes, err := base64.URLEncoding.WithPadding(base64.NoPadding).DecodeString(to.String(resp.Result))
resp, err := v.client.UnwrapKey(ctx, v.keyName, in.KeyInfo.KeyId, params, nil)
if err != nil {
return nil, err
}

// XXX: Workaround: Azure Managed HSM KeyVault's REST API request parser
// changes the encrypted key to include an extra NULL byte at the end.
// This looks like the base64 of the symmetric AES wrapping key above is
// changed from ...= to ...A. You'll get the error (when running Vault
// init / unseal operation):
// > failed to unseal barrier: failed to check for keyring: failed to create cipher: crypto/aes: invalid key size 33
// until this is fixed.
// -> 16-byte / 128-bit AES key gets two padding characters, resulting
// in two null bytes.
// -> 24-byte / 196-bit AES key gets no padding and no null bytes.
// -> 32-byte / 256-bit AES key (default) gets one padding character,
// resulting in one null bytes.
if len(keyBytes) == 18 && keyBytes[16] == 0 && keyBytes[17] == 0 {
keyBytes = keyBytes[:16]
} else if len(keyBytes) == 33 && keyBytes[32] == 0 {
keyBytes = keyBytes[:32]
}

envInfo := &wrapping.EnvelopeInfo{
Key: keyBytes,
Key: resp.Result,
Iv: in.Iv,
Ciphertext: in.Ciphertext,
}
Expand All @@ -302,29 +287,22 @@ func (v *Wrapper) buildBaseURL() string {
return fmt.Sprintf("https://%s.%s/", v.vaultName, v.environment.KeyVaultDNSSuffix)
}

func (v *Wrapper) getKeyVaultClient(withCertPool *x509.CertPool) (*keyvault.BaseClient, error) {
var authorizer autorest.Authorizer
func (v *Wrapper) getKeyVaultClient(withCertPool *x509.CertPool) (*azkeys.Client, error) {
var err error
var cred azcore.TokenCredential

switch {
case v.clientID != "" && v.clientSecret != "":
config := auth.NewClientCredentialsConfig(v.clientID, v.clientSecret, v.tenantID)
config.AADEndpoint = v.environment.ActiveDirectoryEndpoint
config.Resource = strings.TrimSuffix(v.resource, "/")
authorizer, err = config.Authorizer()
// Use client secret if provided
case v.tenantID != "" && v.clientID != "" && v.clientSecret != "":
cred, err = azidentity.NewClientSecretCredential(v.tenantID, v.clientID, v.clientSecret, nil)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get client secret credentials %w", err)
}
// By default use MSI
// By default let Azure select existing credentials
default:
config := auth.NewMSIConfig()
config.Resource = strings.TrimSuffix(v.resource, "/")
if v.clientID != "" {
config.ClientID = v.clientID
}
authorizer, err = config.Authorizer()
cred, err = azidentity.NewDefaultAzureCredential(nil)
Comment on lines -320 to +303
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @cldmstr - can I suggest you check that this doesn't introduce a regression for this fix? #97
There were no tests added for it so it's quite possible it would.

It's possible for multiple managed service identities to exist, and in that situation when autorest/adal queried IMDS it would receive an error back if the client ID was not provided. I'm assuming that azure-sdk-for-go behaves similarly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @jtv8 . I can confirm that the behavior in azure-sdk-for-go is the same as the fix and does not introduce a regression. Internally, the call does almost exactly what the fix does.

if err != nil {
return nil, err
return nil, fmt.Errorf("failed to acquire managed identity credentials %w", err)
}
}

Expand Down Expand Up @@ -353,22 +331,20 @@ func (v *Wrapper) getKeyVaultClient(withCertPool *x509.CertPool) (*keyvault.Base
http2Transport.PingTimeout = 2 * time.Second
}

client := keyvault.New()
client.Authorizer = authorizer
client.SendDecorators = append(client.SendDecorators, func(s autorest.Sender) autorest.Sender {
if ar, ok := s.(autorest.Client); ok {
ar.Sender = &http.Client{
Transport: customTransport,
}
return ar
}
return s
})
return &client, nil
clientOpts := &azkeys.ClientOptions{
ClientOptions: azcore.ClientOptions{Transport: &http.Client{Transport: customTransport}},
}

client, err := azkeys.NewClient(v.baseURL, cred, clientOpts)
if err != nil {
return nil, fmt.Errorf("failed to create keyvault client %w", err)
}

return client, nil
}

// Client returns the AzureKeyVault client used by the wrapper.
func (v *Wrapper) Client() *keyvault.BaseClient {
func (v *Wrapper) Client() *azkeys.Client {
return v.client
}

Expand Down
79 changes: 7 additions & 72 deletions wrappers/azurekeyvault/azurekeyvault_acc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,12 @@ package azurekeyvault

import (
"context"
"crypto/tls"
"crypto/x509"
"fmt"
"net/http"
"net/http/httptest"
"os"
"reflect"
"testing"

"github.com/Azure/azure-sdk-for-go/services/keyvault/v7.1/keyvault"
"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/azure"
wrapping "github.com/hashicorp/go-kms-wrapping/v2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -46,9 +38,13 @@ func TestAzureKeyVault_SetConfig(t *testing.T) {
}

func TestAzureKeyVault_IgnoreEnv(t *testing.T) {
if os.Getenv("VAULT_ACC") == "" {
t.SkipNow()
}

expectedErr := `error fetching Azure Key Vault wrapper key information: Get "https://a-vault-name.a-resource/keys/a-key-name/?api-version=7.3": dial tcp: lookup a-vault-name.a-resource: no such host`

s := NewWrapper()
client := keyvault.New()
s.client = &client

// Setup environment values to ignore for the following values
for _, envVar := range []string{
Expand All @@ -71,7 +67,7 @@ func TestAzureKeyVault_IgnoreEnv(t *testing.T) {
"key_name": "a-key-name",
}
_, err := s.SetConfig(context.Background(), wrapping.WithConfigMap(config))
assert.NoError(t, err)
require.Equal(t, expectedErr, err.Error())
require.Equal(t, config["tenant_id"], s.tenantID)
require.Equal(t, config["client_id"], s.clientID)
require.Equal(t, config["client_secret"], s.clientSecret)
Expand Down Expand Up @@ -108,64 +104,3 @@ func TestAzureKeyVault_Lifecycle(t *testing.T) {
t.Fatalf("expected %s, got %s", input, pt)
}
}

func Test_getKeyVaultClient(t *testing.T) {
t.Parallel()
config := map[string]string{
"disallow_env_vars": "true",
"tenant_id": "a-tenant-id",
"client_id": "a-client-id",
"client_secret": "a-client-secret",
"environment": azure.PublicCloud.Name,
"resource": "a-resource",
"vault_name": "a-vault-name",
"key_name": "a-key-name",
}
s := NewWrapper()
_, err := s.SetConfig(
context.Background(),
wrapping.WithConfigMap(config),
WithKeyNotRequired(true),
)
require.NoError(t, err)
t.Run("send-decorators-set", func(t *testing.T) {
// let's at least ensure that the custom SendDecorator is being properly
// set.
t.Parallel()
got, err := s.getKeyVaultClient(nil)
require.NoError(t, err)
assert.NotEmpty(t, got.SendDecorators)
})
t.Run("force-tls-error", func(t *testing.T) {
// not great, but this test will at least ensure that the client's
// custom TLS transport is being used
t.Parallel()
ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte(fmt.Sprintf("version: %s", tls.VersionName(r.TLS.Version))))
}))
ts.TLS = &tls.Config{
MinVersion: tls.VersionTLS10,
MaxVersion: tls.VersionTLS10,
}
ts.StartTLS()
defer ts.Close()

certPool := x509.NewCertPool()
certPool.AddCert(ts.Certificate())

assert.NoError(t, err)
client, err := s.getKeyVaultClient(certPool)
require.NoError(t, err)
assert.NotEmpty(t, client.SendDecorators)
client.Authorizer = &authorizer{}
_, err = client.GetKey(context.Background(), ts.URL, "global", "1")
require.Error(t, err)
assert.ErrorContains(t, err, "tls: protocol version not supported")
})
}

type authorizer struct{}

func (*authorizer) WithAuthorization() autorest.PrepareDecorator {
return autorest.WithNothing()
}
15 changes: 9 additions & 6 deletions wrappers/azurekeyvault/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ module github.com/hashicorp/go-kms-wrapping/wrappers/azurekeyvault/v2
go 1.20

require (
github.com/Azure/azure-sdk-for-go v68.0.0+incompatible
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.4.0-beta.1
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.3.0-beta.3
github.com/Azure/azure-sdk-for-go/sdk/keyvault/azkeys v0.9.0
github.com/Azure/go-autorest/autorest v0.11.28
github.com/Azure/go-autorest/autorest/azure/auth v0.5.12
github.com/Azure/go-autorest/autorest/to v0.4.0
github.com/hashicorp/go-hclog v1.4.0
github.com/hashicorp/go-kms-wrapping/v2 v2.0.9-0.20230228100945-740d2999c798
Expand All @@ -14,22 +15,24 @@ require (
)

require (
github.com/Azure/azure-sdk-for-go/sdk/internal v1.1.2 // indirect
github.com/Azure/azure-sdk-for-go/sdk/keyvault/internal v0.7.0 // indirect
github.com/Azure/go-autorest v14.2.0+incompatible // indirect
github.com/Azure/go-autorest/autorest/adal v0.9.22 // indirect
github.com/Azure/go-autorest/autorest/azure/cli v0.4.6 // indirect
github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
github.com/Azure/go-autorest/autorest/validation v0.3.1 // indirect
github.com/Azure/go-autorest/logger v0.2.1 // indirect
github.com/Azure/go-autorest/tracing v0.6.0 // indirect
github.com/AzureAD/microsoft-authentication-library-for-go v0.8.1 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/dimchansky/utfbom v1.1.1 // indirect
github.com/fatih/color v1.14.1 // indirect
github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
github.com/google/uuid v1.1.1 // indirect
github.com/hashicorp/go-uuid v1.0.3 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/kylelemons/godebug v1.1.0 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-isatty v0.0.17 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/pkg/browser v0.0.0-20210115035449-ce105d075bb4 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rogpeppe/go-internal v1.6.1 // indirect
golang.org/x/crypto v0.6.0 // indirect
Expand Down
Loading