From 33f30a6e81259ba93511eb98162cc89a633e7714 Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Mon, 15 Oct 2018 14:32:39 +0000 Subject: [PATCH] Allow Bearer JWT client authentication This allows certificate based authentication without exposing the private key to packer using an expiring JWT containting the thumbprint (and sometimes the whole certificate), signed using the certificate key pair. --- builder/azure/arm/authenticate.go | 64 +++++++--- builder/azure/arm/authenticate_test.go | 2 +- builder/azure/arm/clientconfig.go | 70 ++++++++--- builder/azure/arm/clientconfig_test.go | 165 ++++++++++++++++++++++++- 4 files changed, 270 insertions(+), 31 deletions(-) diff --git a/builder/azure/arm/authenticate.go b/builder/azure/arm/authenticate.go index 7f0b5bdc9c8..202c9b80457 100644 --- a/builder/azure/arm/authenticate.go +++ b/builder/azure/arm/authenticate.go @@ -1,31 +1,32 @@ package arm import ( + "net/url" + "github.com/Azure/go-autorest/autorest/adal" "github.com/Azure/go-autorest/autorest/azure" ) -type Authenticate struct { - env azure.Environment - clientID string - clientSecret string - tenantID string +type oAuthTokenProvider interface { + getServicePrincipalToken() (*adal.ServicePrincipalToken, error) + getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) } -func NewAuthenticate(env azure.Environment, clientID, clientSecret, tenantID string) *Authenticate { - return &Authenticate{ - env: env, - clientID: clientID, - clientSecret: clientSecret, - tenantID: tenantID, - } +// for clientID/secret auth +type secretOAuthTokenProvider struct { + env azure.Environment + clientID, clientSecret, tenantID string } -func (a *Authenticate) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) { +func NewSecretOAuthTokenProvider(env azure.Environment, clientID, clientSecret, tenantID string) oAuthTokenProvider { + return &secretOAuthTokenProvider{env, clientID, clientSecret, tenantID} +} + +func (a *secretOAuthTokenProvider) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) { return a.getServicePrincipalTokenWithResource(a.env.ResourceManagerEndpoint) } -func (a *Authenticate) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) { +func (a *secretOAuthTokenProvider) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) { oauthConfig, err := adal.NewOAuthConfig(a.env.ActiveDirectoryEndpoint, a.tenantID) if err != nil { return nil, err @@ -39,3 +40,38 @@ func (a *Authenticate) getServicePrincipalTokenWithResource(resource string) (*a return spt, err } + +// for clientID/bearer JWT auth +type jwtOAuthTokenProvider struct { + env azure.Environment + clientID, clientJWT, tenantID string +} + +func NewJWTOAuthTokenProvider(env azure.Environment, clientID, clientJWT, tenantID string) oAuthTokenProvider { + return &jwtOAuthTokenProvider{env, clientID, clientJWT, tenantID} +} + +func (pt *jwtOAuthTokenProvider) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) { + return pt.getServicePrincipalTokenWithResource(pt.env.ResourceManagerEndpoint) +} + +func (tp *jwtOAuthTokenProvider) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) { + oauthConfig, err := adal.NewOAuthConfig(tp.env.ActiveDirectoryEndpoint, tp.tenantID) + if err != nil { + return nil, err + } + + return adal.NewServicePrincipalTokenWithSecret( + *oauthConfig, + tp.clientID, + resource, + tp) +} + +// implements ServicePrincipalSecret +func (tp *jwtOAuthTokenProvider) SetAuthenticationValues( + t *adal.ServicePrincipalToken, v *url.Values) error { + v.Set("client_assertion", tp.clientJWT) + v.Set("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer") + return nil +} diff --git a/builder/azure/arm/authenticate_test.go b/builder/azure/arm/authenticate_test.go index 63fd1c3aa9e..f6ef68e1ae8 100644 --- a/builder/azure/arm/authenticate_test.go +++ b/builder/azure/arm/authenticate_test.go @@ -10,7 +10,7 @@ import ( // that cannot be done in a unit test because it involves network access. Instead, // I assert the expected inertness of this class. func TestNewAuthenticate(t *testing.T) { - testSubject := NewAuthenticate(azure.PublicCloud, "clientID", "clientString", "tenantID") + testSubject := NewSecretOAuthTokenProvider(azure.PublicCloud, "clientID", "clientString", "tenantID") spn, err := testSubject.getServicePrincipalToken() if err != nil { t.Fatalf(err.Error()) diff --git a/builder/azure/arm/clientconfig.go b/builder/azure/arm/clientconfig.go index 9b2091e2b11..bb765e78d4b 100644 --- a/builder/azure/arm/clientconfig.go +++ b/builder/azure/arm/clientconfig.go @@ -3,9 +3,11 @@ package arm import ( "fmt" "strings" + "time" "github.com/Azure/go-autorest/autorest/adal" "github.com/Azure/go-autorest/autorest/azure" + jwt "github.com/dgrijalva/jwt-go" packerAzureCommon "github.com/hashicorp/packer/builder/azure/common" "github.com/hashicorp/packer/packer" ) @@ -19,8 +21,12 @@ type ClientConfig struct { // Authentication fields - ClientID string `mapstructure:"client_id"` - ClientSecret string `mapstructure:"client_secret"` + // Client ID + ClientID string `mapstructure:"client_id"` + // Client secret/password + ClientSecret string `mapstructure:"client_secret"` + // JWT bearer token for client auth (RFC 7523, Sec. 2.2) + ClientJWT string `mapstructure:"client_jwt"` ObjectID string `mapstructure:"object_id"` TenantID string `mapstructure:"tenant_id"` SubscriptionID string `mapstructure:"subscription_id"` @@ -81,17 +87,38 @@ func (c ClientConfig) assertRequiredParametersSet(errs *packer.MultiError) { // readable by the ObjectID of the App. There may be another way to handle // this case, but I am not currently aware of it - send feedback. - if !c.useDeviceLogin() { - if c.ClientID == "" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("A client_id must be specified")) - } + if c.SubscriptionID == "" { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("A subscription_id must be specified")) + } - if c.ClientSecret == "" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("A client_secret must be specified")) - } + if c.useDeviceLogin() { + // nothing else to check + return + } - if c.SubscriptionID == "" { - errs = packer.MultiErrorAppend(errs, fmt.Errorf("A subscription_id must be specified")) + if c.ClientID == "" || + (c.ClientSecret == "" && c.ClientJWT == "") || + (c.ClientSecret != "" && c.ClientJWT != "") { + // either client ID was not set, or neither or both secret and JWT are set + errs = packer.MultiErrorAppend(errs, fmt.Errorf("No valid set of authention methods specified: \n"+ + "specify either (client_id,client_secret) or (client_id,client_jwt) to use a service principal, \n"+ + "or specify none of these to use interactive user authentication.")) + } + + if c.ClientJWT != "" { + // should be a JWT that is valid for at least 5 more minutes + p := jwt.Parser{} + claims := jwt.StandardClaims{} + token, _, err := p.ParseUnverified(c.ClientJWT, &claims) + if err != nil { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("client_jwt is not a JWT: %v", err)) + } else { + if claims.ExpiresAt < time.Now().Add(5*time.Minute).Unix() { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("client_jwt will expire within 5 minutes, please use a JWT that is valid for at least 5 minutes")) + } + if t, ok := token.Header["x5t"]; !ok || t == "" { + errs = packer.MultiErrorAppend(errs, fmt.Errorf("client_jwt is missing the x5t header value, which is required for bearer JWT client authentication to Azure")) + } } } } @@ -100,6 +127,7 @@ func (c ClientConfig) useDeviceLogin() bool { return c.SubscriptionID != "" && c.ClientID == "" && c.ClientSecret == "" && + c.ClientJWT == "" && c.TenantID == "" } @@ -110,9 +138,6 @@ func (c ClientConfig) getServicePrincipalTokens( err error) { tenantID := c.TenantID - if tenantID == "" { - tenantID = "common" - } if c.useDeviceLogin() { say("Getting auth token for Service management endpoint") @@ -126,8 +151,23 @@ func (c ClientConfig) getServicePrincipalTokens( return nil, nil, err } + } else if c.ClientSecret != "" { + say("Getting tokens using client secret") + auth := NewSecretOAuthTokenProvider(*c.cloudEnvironment, c.ClientID, c.ClientSecret, tenantID) + + servicePrincipalToken, err = auth.getServicePrincipalToken() + if err != nil { + return nil, nil, err + } + + servicePrincipalTokenVault, err = auth.getServicePrincipalTokenWithResource( + strings.TrimRight(c.cloudEnvironment.KeyVaultEndpoint, "/")) + if err != nil { + return nil, nil, err + } } else { - auth := NewAuthenticate(*c.cloudEnvironment, c.ClientID, c.ClientSecret, tenantID) + say("Getting tokens using client bearer JWT") + auth := NewJWTOAuthTokenProvider(*c.cloudEnvironment, c.ClientID, c.ClientJWT, tenantID) servicePrincipalToken, err = auth.getServicePrincipalToken() if err != nil { diff --git a/builder/azure/arm/clientconfig_test.go b/builder/azure/arm/clientconfig_test.go index 6784e63cfad..5ee2b3b8eef 100644 --- a/builder/azure/arm/clientconfig_test.go +++ b/builder/azure/arm/clientconfig_test.go @@ -1,11 +1,17 @@ package arm import ( + "crypto/rand" + "crypto/rsa" + "encoding/base64" "fmt" "os" "testing" + "time" "github.com/Azure/go-autorest/autorest/azure" + jwt "github.com/dgrijalva/jwt-go" + "github.com/hashicorp/packer/packer" ) func Test_ClientConfig_DeviceLogin(t *testing.T) { @@ -14,6 +20,8 @@ func Test_ClientConfig_DeviceLogin(t *testing.T) { SubscriptionID: getEnvOrSkip(t, "AZURE_SUBSCRIPTION"), cloudEnvironment: getCloud(), } + assertValid(t, cfg) + spt, sptkv, err := cfg.getServicePrincipalTokens( func(s string) { fmt.Printf("SAY: %s\n", s) }) if err != nil { @@ -35,7 +43,7 @@ func Test_ClientConfig_DeviceLogin(t *testing.T) { } } -func Test_ClientConfig_ClientID_Password(t *testing.T) { +func Test_ClientConfig_ClientPassword(t *testing.T) { cfg := ClientConfig{ SubscriptionID: getEnvOrSkip(t, "AZURE_SUBSCRIPTION"), ClientID: getEnvOrSkip(t, "AZURE_CLIENTID"), @@ -43,6 +51,37 @@ func Test_ClientConfig_ClientID_Password(t *testing.T) { TenantID: getEnvOrSkip(t, "AZURE_TENANTID"), cloudEnvironment: getCloud(), } + assertValid(t, cfg) + + spt, sptkv, err := cfg.getServicePrincipalTokens(func(s string) { fmt.Printf("SAY: %s\n", s) }) + if err != nil { + t.Fatalf("Expected nil err, but got: %v", err) + } + token := spt.Token() + if token.AccessToken == "" { + t.Fatal("Expected management token to have non-nil access token") + } + if token.RefreshToken != "" { + t.Fatal("Expected management token to have no refresh token") + } + kvtoken := sptkv.Token() + if kvtoken.AccessToken == "" { + t.Fatal("Expected keyvault token to have non-nil access token") + } + if kvtoken.RefreshToken != "" { + t.Fatal("Expected keyvault token to have no refresh token") + } +} + +func Test_ClientConfig_ClientJWT(t *testing.T) { + cfg := ClientConfig{ + SubscriptionID: getEnvOrSkip(t, "AZURE_SUBSCRIPTION"), + ClientID: getEnvOrSkip(t, "AZURE_CLIENTID"), + ClientJWT: getEnvOrSkip(t, "AZURE_CLIENTJWT"), + TenantID: getEnvOrSkip(t, "AZURE_TENANTID"), + cloudEnvironment: getCloud(), + } + assertValid(t, cfg) spt, sptkv, err := cfg.getServicePrincipalTokens(func(s string) { fmt.Printf("SAY: %s\n", s) }) if err != nil { @@ -80,3 +119,127 @@ func getCloud() *azure.Environment { c, _ := azure.EnvironmentFromName(cloudName) return &c } + +// tests for assertRequiredParametersSet + +func Test_ClientConfig_CanUseDeviceCode(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + // TenantID is optional + + assertValid(t, cfg) +} + +func assertValid(t *testing.T, cfg ClientConfig) { + errs := &packer.MultiError{} + cfg.assertRequiredParametersSet(errs) + if len(errs.Errors) != 0 { + t.Fatal("Expected errs to be empty: ", errs) + } +} + +func assertInvalid(t *testing.T, cfg ClientConfig) { + errs := &packer.MultiError{} + cfg.assertRequiredParametersSet(errs) + if len(errs.Errors) == 0 { + t.Fatal("Expected errs to be non-empty") + } +} + +func Test_ClientConfig_CanUseClientSecret(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientSecret = "12345" + + assertValid(t, cfg) +} + +func Test_ClientConfig_CanUseClientSecretWithTenantID(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientSecret = "12345" + cfg.TenantID = "12345" + + assertValid(t, cfg) +} + +func Test_ClientConfig_CanUseClientJWT(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientJWT = getJWT(10*time.Minute, true) + + assertValid(t, cfg) +} + +func Test_ClientConfig_CanUseClientJWTWithTenantID(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientJWT = getJWT(10*time.Minute, true) + cfg.TenantID = "12345" + + assertValid(t, cfg) +} + +func Test_ClientConfig_CannotUseBothClientJWTAndSecret(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientSecret = "12345" + cfg.ClientJWT = getJWT(10*time.Minute, true) + + assertInvalid(t, cfg) +} + +func Test_ClientConfig_ClientJWTShouldBeValidForAtLeast5Minutes(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientJWT = getJWT(time.Minute, true) + + assertInvalid(t, cfg) +} + +func Test_ClientConfig_ClientJWTShouldHaveThumbprint(t *testing.T) { + cfg := emptyClientConfig() + cfg.SubscriptionID = "12345" + cfg.ClientID = "12345" + cfg.ClientJWT = getJWT(10*time.Minute, false) + + assertInvalid(t, cfg) +} + +func emptyClientConfig() ClientConfig { + cfg := ClientConfig{} + _ = cfg.setCloudEnvironment() + return cfg +} + +func Test_getJWT(t *testing.T) { + if getJWT(time.Minute, true) == "" { + t.Fatalf("getJWT is broken") + } +} + +func getJWT(validFor time.Duration, withX5tHeader bool) string { + token := jwt.New(jwt.SigningMethodRS256) + key, _ := rsa.GenerateKey(rand.Reader, 2048) + + token.Claims = jwt.MapClaims{ + "aud": "https://login.microsoftonline.com/tenant.onmicrosoft.com/oauth2/token?api-version=1.0", + "iss": "355dff10-cd78-11e8-89fe-000d3afd16e3", + "sub": "355dff10-cd78-11e8-89fe-000d3afd16e3", + "jti": base64.URLEncoding.EncodeToString([]byte{0}), + "nbf": time.Now().Unix(), + "exp": time.Now().Add(validFor).Unix(), + } + if withX5tHeader { + token.Header["x5t"] = base64.URLEncoding.EncodeToString([]byte("thumbprint")) + } + + signedString, _ := token.SignedString(key) + return signedString +}