Merge branch 'master' into hyperone

CHANGELOG.md

@@ -9,6 +9,8 @@
 * builder/amazon: don't Cleanup Temp Keys when there is no communicator to
     avoid a panic [GH-7100] [GH-7095]
 * builder/azure: allow to configure disk caching [GH-7061]
+* builder/azure: add certificate authentication [GH-7189]
+* builder/azure: use deallocate instead of just power-off [GH-7203]
 * builder/openstack: Don't require network v2 [GH-6933]
 * builder/openstack: Support for tagging new images [GH-7037]
 * core/shell: Add env vars "PACKER_HTTP_IP" and "PACKER_HTTP_PORT" to shell

builder/azure/arm/authenticate.go

@@ -2,44 +2,9 @@ package arm
 
 import (
 	"github.com/Azure/go-autorest/autorest/adal"
-	"github.com/Azure/go-autorest/autorest/azure"
 )
 
-type Authenticate struct {
+type oAuthTokenProvider interface {
-	env          azure.Environment
+	getServicePrincipalToken() (*adal.ServicePrincipalToken, error)
-	clientID     string
+	getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error)
-	clientSecret string
-	tenantID     string
-}
-
-func NewAuthenticate(env azure.Environment, clientID, clientSecret, tenantID string) *Authenticate {
-	return &Authenticate{
-		env:          env,
-		clientID:     clientID,
-		clientSecret: clientSecret,
-		tenantID:     tenantID,
-	}
-}
-
-func (a *Authenticate) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) {
-	return a.getServicePrincipalTokenWithResource(a.env.ResourceManagerEndpoint)
-}
-
-func (a *Authenticate) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) {
-	oauthConfig, err := adal.NewOAuthConfig(a.env.ActiveDirectoryEndpoint, a.tenantID)
-	if err != nil {
-		return nil, err
-	}
-
-	if a.clientID == "" && a.clientSecret == "" {
-		return adal.NewServicePrincipalTokenFromMSI("http://169.254.169.254/metadata/identity/oauth2/token", resource)
-	}
-
-	spt, err := adal.NewServicePrincipalToken(
-		*oauthConfig,
-		a.clientID,
-		a.clientSecret,
-		resource)
-
-	return spt, err
 }

builder/azure/arm/authenticate_cert.go

@@ -0,0 +1,155 @@
+package arm
+
+import (
+	"crypto/ecdsa"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/sha1"
+	"crypto/x509"
+	"encoding/base64"
+	"encoding/pem"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"time"
+
+	"github.com/Azure/go-autorest/autorest/azure"
+	jwt "github.com/dgrijalva/jwt-go"
+)
+
+func NewCertOAuthTokenProvider(env azure.Environment, clientID, clientCertPath, tenantID string) (oAuthTokenProvider, error) {
+	cert, key, err := readCert(clientCertPath)
+	if err != nil {
+		return nil, fmt.Errorf("Error reading certificate: %v", err)
+	}
+
+	audience := fmt.Sprintf("%s%s/oauth2/token", env.ActiveDirectoryEndpoint, tenantID)
+	jwt, err := makeJWT(clientID, audience, cert, key, time.Hour, true)
+	if err != nil {
+		return nil, fmt.Errorf("Error generating JWT: %v", err)
+	}
+
+	return NewJWTOAuthTokenProvider(env, clientID, jwt, tenantID), nil
+}
+
+// Creates a new JSON Web Token to be used as bearer JWT to authenticate
+// to the Azure AD token endpoint to retrieve an access token for `audience`.
+// If the full certificate is included in the token, then issuer/subject name
+// could be used to authenticate if configured by the identity provider (AAD).
+func makeJWT(clientID string, audience string,
+	cert *x509.Certificate, privatekey interface{},
+	validFor time.Duration, includeFullCertificate bool) (string, error) {
+
+	// The jti (JWT ID) claim provides a unique identifier for the JWT.
+	// See https://tools.ietf.org/html/rfc7519#section-4.1.7
+	jti := make([]byte, 20)
+	_, err := rand.Read(jti)
+	if err != nil {
+		return "", err
+	}
+
+	var token *jwt.Token
+	if cert.PublicKeyAlgorithm == x509.RSA {
+		token = jwt.New(jwt.SigningMethodRS256)
+	} else if cert.PublicKeyAlgorithm == x509.ECDSA {
+		token = jwt.New(jwt.SigningMethodES256)
+	} else {
+		return "", fmt.Errorf("Don't know how to handle this type of key algorithm: %v", cert.PublicKeyAlgorithm)
+	}
+
+	hasher := sha1.New()
+	if _, err := hasher.Write(cert.Raw); err != nil {
+		return "", err
+	}
+	thumbprint := base64.URLEncoding.EncodeToString(hasher.Sum(nil))
+
+	// X.509 thumbprint, see https://tools.ietf.org/html/rfc7515#section-4.1.7
+	token.Header["x5t"] = thumbprint
+	if includeFullCertificate {
+		// X.509 certificate (chain), see https://tools.ietf.org/html/rfc7515#section-4.1.6
+		token.Header["x5c"] = []string{base64.StdEncoding.EncodeToString(cert.Raw)}
+	}
+
+	token.Claims = jwt.MapClaims{
+		// See https://tools.ietf.org/html/rfc7519#section-4.1
+		"aud": audience,
+		"iss": clientID,
+		"sub": clientID,
+		"jti": base64.URLEncoding.EncodeToString(jti),
+		"nbf": time.Now().Unix(),
+		"exp": time.Now().Add(validFor).Unix(),
+	}
+
+	return token.SignedString(privatekey)
+}
+
+func readCert(file string) (cert *x509.Certificate, key interface{}, err error) {
+	f, err := os.Open(file)
+	if err != nil {
+		return nil, nil, err
+	}
+	defer f.Close()
+	d, err := ioutil.ReadAll(f)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	blocks := []*pem.Block{}
+	for len(d) > 0 {
+		var b *pem.Block
+		b, d = pem.Decode(d)
+		if b == nil {
+			break
+		}
+		blocks = append(blocks, b)
+	}
+
+	certs := []*x509.Certificate{}
+	for _, block := range blocks {
+		if block.Type == "CERTIFICATE" {
+			c, err := x509.ParseCertificate(block.Bytes)
+			if err != nil {
+				return nil, nil, fmt.Errorf(
+					"Failed to read certificate block: %v", err)
+			}
+			certs = append(certs, c)
+		} else if block.Type == "PRIVATE KEY" {
+			key, err = x509.ParsePKCS8PrivateKey(block.Bytes)
+			if err != nil {
+				return nil, nil, fmt.Errorf(
+					"Failed to read private key block: %v", err)
+			}
+		}
+		// Don't care about other types of blocks, ignore
+	}
+
+	if key == nil {
+		return nil, nil, fmt.Errorf("Did not find private key in pem file")
+	}
+
+	// find the certificate that belongs to the private key by comparing the public keys
+	switch key := key.(type) {
+	case *rsa.PrivateKey:
+		for _, c := range certs {
+			if cp, ok := c.PublicKey.(*rsa.PublicKey); ok &&
+				(cp.N.Cmp(key.PublicKey.N) == 0) {
+				cert = c
+			}
+		}
+
+	case *ecdsa.PrivateKey:
+		for _, c := range certs {
+			if cp, ok := c.PublicKey.(*ecdsa.PublicKey); ok &&
+				(cp.X.Cmp(key.PublicKey.X) == 0) &&
+				(cp.Y.Cmp(key.PublicKey.Y) == 0) {
+				cert = c
+			}
+		}
+	}
+
+	if cert == nil {
+		return nil, nil, fmt.Errorf("Did not find certificate belonging to private key in pem file")
+	}
+
+	return cert, key, nil
+}

builder/azure/arm/authenticate_devicewflow.go

@@ -0,0 +1,36 @@
+package arm
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/Azure/go-autorest/autorest/adal"
+	"github.com/Azure/go-autorest/autorest/azure"
+	packerAzureCommon "github.com/hashicorp/packer/builder/azure/common"
+)
+
+func NewDeviceFlowOAuthTokenProvider(env azure.Environment, say func(string), tenantID string) oAuthTokenProvider {
+	return &deviceflowOauthTokenProvider{}
+}
+
+type deviceflowOauthTokenProvider struct {
+	env      azure.Environment
+	say      func(string)
+	tenantID string
+}
+
+func (tp *deviceflowOauthTokenProvider) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) {
+	return tp.getServicePrincipalTokenWithResource(tp.env.ResourceManagerEndpoint)
+}
+
+func (tp *deviceflowOauthTokenProvider) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) {
+	if resource == tp.env.ServiceManagementEndpoint {
+		tp.say("Getting auth token for Service management endpoint")
+	} else if resource == strings.TrimRight(tp.env.KeyVaultEndpoint, "/") {
+		tp.say("Getting token for Vault resource")
+	} else {
+		tp.say(fmt.Sprintf("Getting token for %s", resource))
+	}
+
+	return packerAzureCommon.Authenticate(tp.env, tp.tenantID, tp.say, resource)
+}

builder/azure/arm/authenticate_jwt.go

@@ -0,0 +1,43 @@
+package arm
+
+import (
+	"net/url"
+
+	"github.com/Azure/go-autorest/autorest/adal"
+	"github.com/Azure/go-autorest/autorest/azure"
+)
+
+// 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 (tp *jwtOAuthTokenProvider) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) {
+	return tp.getServicePrincipalTokenWithResource(tp.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 github.com/Azure/go-autorest/autorest/adal.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
+}

builder/azure/arm/authenticate_msi.go

@@ -0,0 +1,23 @@
+package arm
+
+import (
+	"github.com/Azure/go-autorest/autorest/adal"
+	"github.com/Azure/go-autorest/autorest/azure"
+)
+
+// for managed identity auth
+type msiOAuthTokenProvider struct {
+	env azure.Environment
+}
+
+func NewMSIOAuthTokenProvider(env azure.Environment) oAuthTokenProvider {
+	return &msiOAuthTokenProvider{env}
+}
+
+func (tp *msiOAuthTokenProvider) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) {
+	return tp.getServicePrincipalTokenWithResource(tp.env.ResourceManagerEndpoint)
+}
+
+func (tp *msiOAuthTokenProvider) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) {
+	return adal.NewServicePrincipalTokenFromMSI("http://169.254.169.254/metadata/identity/oauth2/token", resource)
+}

builder/azure/arm/authenticate_secret.go

@@ -0,0 +1,35 @@
+package arm
+
+import (
+	"github.com/Azure/go-autorest/autorest/adal"
+	"github.com/Azure/go-autorest/autorest/azure"
+)
+
+// for clientID/secret auth
+type secretOAuthTokenProvider struct {
+	env                              azure.Environment
+	clientID, clientSecret, tenantID string
+}
+
+func NewSecretOAuthTokenProvider(env azure.Environment, clientID, clientSecret, tenantID string) oAuthTokenProvider {
+	return &secretOAuthTokenProvider{env, clientID, clientSecret, tenantID}
+}
+
+func (tp *secretOAuthTokenProvider) getServicePrincipalToken() (*adal.ServicePrincipalToken, error) {
+	return tp.getServicePrincipalTokenWithResource(tp.env.ResourceManagerEndpoint)
+}
+
+func (tp *secretOAuthTokenProvider) getServicePrincipalTokenWithResource(resource string) (*adal.ServicePrincipalToken, error) {
+	oauthConfig, err := adal.NewOAuthConfig(tp.env.ActiveDirectoryEndpoint, tp.tenantID)
+	if err != nil {
+		return nil, err
+	}
+
+	spt, err := adal.NewServicePrincipalToken(
+		*oauthConfig,
+		tp.clientID,
+		tp.clientSecret,
+		resource)
+
+	return spt, err
+}

builder/azure/arm/authenticate_secret_test.go

@@ -9,8 +9,8 @@ import (
 // Behavior is the most important thing to assert for ServicePrincipalToken, but
 // 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) {
+func TestNewSecretOAuthTokenProvider(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())

builder/azure/arm/builder.go

@@ -385,52 +385,7 @@ func (b *Builder) setImageParameters(stateBag multistep.StateBag) {
 }
 
 func (b *Builder) getServicePrincipalTokens(say func(string)) (*adal.ServicePrincipalToken, *adal.ServicePrincipalToken, error) {
-	var servicePrincipalToken *adal.ServicePrincipalToken
+	return b.config.ClientConfig.getServicePrincipalTokens(say)
-	var servicePrincipalTokenVault *adal.ServicePrincipalToken
-
-	var err error
-
-	if b.config.useDeviceLogin {
-		say("Getting auth token for Service management endpoint")
-		servicePrincipalToken, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.TenantID, say, b.config.cloudEnvironment.ServiceManagementEndpoint)
-		if err != nil {
-			return nil, nil, err
-		}
-		say("Getting token for Vault resource")
-		servicePrincipalTokenVault, err = packerAzureCommon.Authenticate(*b.config.cloudEnvironment, b.config.TenantID, say, strings.TrimRight(b.config.cloudEnvironment.KeyVaultEndpoint, "/"))
-		if err != nil {
-			return nil, nil, err
-		}
-
-	} else {
-		auth := NewAuthenticate(*b.config.cloudEnvironment, b.config.ClientID, b.config.ClientSecret, b.config.TenantID)
-
-		servicePrincipalToken, err = auth.getServicePrincipalToken()
-		if err != nil {
-			return nil, nil, err
-		}
-
-		servicePrincipalTokenVault, err = auth.getServicePrincipalTokenWithResource(
-			strings.TrimRight(b.config.cloudEnvironment.KeyVaultEndpoint, "/"))
-		if err != nil {
-			return nil, nil, err
-		}
-
-	}
-
-	err = servicePrincipalToken.EnsureFresh()
-
-	if err != nil {
-		return nil, nil, err
-	}
-
-	err = servicePrincipalTokenVault.EnsureFresh()
-
-	if err != nil {
-		return nil, nil, err
-	}
-
-	return servicePrincipalToken, servicePrincipalTokenVault, nil
 }
 
 func getObjectIdFromToken(ui packer.Ui, token *adal.ServicePrincipalToken) string {

builder/azure/arm/clientconfig.go

@@ -0,0 +1,223 @@
+package arm
+
+import (
+	"fmt"
+	"os"
+	"strings"
+	"time"
+
+	"github.com/Azure/go-autorest/autorest/adal"
+	"github.com/Azure/go-autorest/autorest/azure"
+	jwt "github.com/dgrijalva/jwt-go"
+	"github.com/hashicorp/packer/packer"
+)
+
+// ClientConfig allows for various ways to authenticate Azure clients
+type ClientConfig struct {
+	// Describes where API's are
+
+	CloudEnvironmentName string `mapstructure:"cloud_environment_name"`
+	cloudEnvironment     *azure.Environment
+
+	// Authentication fields
+
+	// Client ID
+	ClientID string `mapstructure:"client_id"`
+	// Client secret/password
+	ClientSecret string `mapstructure:"client_secret"`
+	// Certificate path for client auth
+	ClientCertPath string `mapstructure:"client_cert_path"`
+	// 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"`
+}
+
+const DefaultCloudEnvironmentName = "Public"
+
+func (c *ClientConfig) provideDefaultValues() {
+	if c.CloudEnvironmentName == "" {
+		c.CloudEnvironmentName = DefaultCloudEnvironmentName
+	}
+}
+
+func (c *ClientConfig) setCloudEnvironment() error {
+	lookup := map[string]string{
+		"CHINA":           "AzureChinaCloud",
+		"CHINACLOUD":      "AzureChinaCloud",
+		"AZURECHINACLOUD": "AzureChinaCloud",
+
+		"GERMAN":           "AzureGermanCloud",
+		"GERMANCLOUD":      "AzureGermanCloud",
+		"AZUREGERMANCLOUD": "AzureGermanCloud",
+
+		"GERMANY":           "AzureGermanCloud",
+		"GERMANYCLOUD":      "AzureGermanCloud",
+		"AZUREGERMANYCLOUD": "AzureGermanCloud",
+
+		"PUBLIC":           "AzurePublicCloud",
+		"PUBLICCLOUD":      "AzurePublicCloud",
+		"AZUREPUBLICCLOUD": "AzurePublicCloud",
+
+		"USGOVERNMENT":           "AzureUSGovernmentCloud",
+		"USGOVERNMENTCLOUD":      "AzureUSGovernmentCloud",
+		"AZUREUSGOVERNMENTCLOUD": "AzureUSGovernmentCloud",
+	}
+
+	name := strings.ToUpper(c.CloudEnvironmentName)
+	envName, ok := lookup[name]
+	if !ok {
+		return fmt.Errorf("There is no cloud environment matching the name '%s'!", c.CloudEnvironmentName)
+	}
+
+	env, err := azure.EnvironmentFromName(envName)
+	c.cloudEnvironment = &env
+	return err
+}
+
+func (c ClientConfig) assertRequiredParametersSet(errs *packer.MultiError) {
+	/////////////////////////////////////////////
+	// Authentication via OAUTH
+
+	// Check if device login is being asked for, and is allowed.
+	//
+	// Device login is enabled if the user only defines SubscriptionID and not
+	// ClientID, ClientSecret, and TenantID.
+	//
+	// Device login is not enabled for Windows because the WinRM certificate is
+	// 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.useMSI() {
+		return
+	}
+
+	if c.useDeviceLogin() {
+		return
+	}
+
+	if c.SubscriptionID != "" && c.ClientID != "" &&
+		c.ClientSecret != "" &&
+		c.ClientCertPath == "" &&
+		c.ClientJWT == "" {
+		// Service principal using secret
+		return
+	}
+
+	if c.SubscriptionID != "" && c.ClientID != "" &&
+		c.ClientSecret == "" &&
+		c.ClientCertPath != "" &&
+		c.ClientJWT == "" {
+		// Service principal using certificate
+
+		if _, err := os.Stat(c.ClientCertPath); err != nil {
+			errs = packer.MultiErrorAppend(errs, fmt.Errorf("client_cert_path is not an accessible file: %v", err))
+		}
+		return
+	}
+
+	if c.SubscriptionID != "" && c.ClientID != "" &&
+		c.ClientSecret == "" &&
+		c.ClientCertPath == "" &&
+		c.ClientJWT != "" {
+		// Service principal using JWT
+		// Check that JWT 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"))
+			}
+		}
+
+		return
+	}
+
+	errs = packer.MultiErrorAppend(errs, fmt.Errorf("No valid set of authentication values specified:\n"+
+		"  to use the Managed Identity of the current machine, do not specify any of the fields below\n"+
+		"  to use interactive user authentication, specify only subscription_id\n"+
+		"  to use an Azure Active Directory service principal, specify either:\n"+
+		"  - subscription_id, client_id and client_secret\n"+
+		"  - subscription_id, client_id and client_cert_path\n"+
+		"  - subscription_id, client_id and client_jwt."))
+}
+
+func (c ClientConfig) useDeviceLogin() bool {
+	return c.SubscriptionID != "" &&
+		c.ClientID == "" &&
+		c.ClientSecret == "" &&
+		c.ClientJWT == "" &&
+		c.ClientCertPath == "" &&
+		c.TenantID == ""
+}
+
+func (c ClientConfig) useMSI() bool {
+	return c.SubscriptionID == "" &&
+		c.ClientID == "" &&
+		c.ClientSecret == "" &&
+		c.ClientJWT == "" &&
+		c.ClientCertPath == "" &&
+		c.TenantID == ""
+}
+
+func (c ClientConfig) getServicePrincipalTokens(
+	say func(string)) (
+	servicePrincipalToken *adal.ServicePrincipalToken,
+	servicePrincipalTokenVault *adal.ServicePrincipalToken,
+	err error) {
+
+	tenantID := c.TenantID
+
+	var auth oAuthTokenProvider
+
+	if c.useDeviceLogin() {
+		say("Getting tokens using device flow")
+		auth = NewDeviceFlowOAuthTokenProvider(*c.cloudEnvironment, say, tenantID)
+	} else if c.useMSI() {
+		say("Getting tokens using Managed Identity for Azure")
+		auth = NewMSIOAuthTokenProvider(*c.cloudEnvironment)
+	} else if c.ClientSecret != "" {
+		say("Getting tokens using client secret")
+		auth = NewSecretOAuthTokenProvider(*c.cloudEnvironment, c.ClientID, c.ClientSecret, tenantID)
+	} else if c.ClientCertPath != "" {
+		say("Getting tokens using client certificate")
+		auth, err = NewCertOAuthTokenProvider(*c.cloudEnvironment, c.ClientID, c.ClientCertPath, tenantID)
+		if err != nil {
+			return nil, nil, err
+		}
+	} else {
+		say("Getting tokens using client bearer JWT")
+		auth = NewJWTOAuthTokenProvider(*c.cloudEnvironment, c.ClientID, c.ClientJWT, tenantID)
+	}
+
+	servicePrincipalToken, err = auth.getServicePrincipalToken()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	err = servicePrincipalToken.EnsureFresh()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	servicePrincipalTokenVault, err = auth.getServicePrincipalTokenWithResource(
+		strings.TrimRight(c.cloudEnvironment.KeyVaultEndpoint, "/"))
+	if err != nil {
+		return nil, nil, err
+	}
+
+	err = servicePrincipalTokenVault.EnsureFresh()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return servicePrincipalToken, servicePrincipalTokenVault, nil
+}

builder/azure/arm/clientconfig_test.go

@@ -0,0 +1,397 @@
+package arm
+
+import (
+	crand "crypto/rand"
+	"crypto/rsa"
+	"encoding/base64"
+	"encoding/binary"
+	"fmt"
+	"io"
+	mrand "math/rand"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/Azure/go-autorest/autorest/azure"
+	jwt "github.com/dgrijalva/jwt-go"
+	"github.com/hashicorp/packer/packer"
+)
+
+func Test_ClientConfig_RequiredParametersSet(t *testing.T) {
+
+	tests := []struct {
+		name    string
+		config  ClientConfig
+		wantErr bool
+	}{
+		{
+			name:    "no client_id, client_secret or subscription_id should enable MSI auth",
+			config:  ClientConfig{},
+			wantErr: false,
+		},
+		{
+			name: "subscription_id is set will trigger device flow",
+			config: ClientConfig{
+				SubscriptionID: "error",
+			},
+			wantErr: false,
+		},
+		{
+			name: "client_id without client_secret, client_cert_path or client_jwt should error",
+			config: ClientConfig{
+				ClientID: "error",
+			},
+			wantErr: true,
+		},
+		{
+			name: "client_secret without client_id should error",
+			config: ClientConfig{
+				ClientSecret: "error",
+			},
+			wantErr: true,
+		},
+		{
+			name: "client_cert_path without client_id should error",
+			config: ClientConfig{
+				ClientCertPath: "/dev/null",
+			},
+			wantErr: true,
+		},
+		{
+			name: "client_jwt without client_id should error",
+			config: ClientConfig{
+				ClientJWT: "error",
+			},
+			wantErr: true,
+		},
+		{
+			name: "missing subscription_id when using secret",
+			config: ClientConfig{
+				ClientID:     "ok",
+				ClientSecret: "ok",
+			},
+			wantErr: true,
+		},
+		{
+			name: "missing subscription_id when using certificate",
+			config: ClientConfig{
+				ClientID:       "ok",
+				ClientCertPath: "ok",
+			},
+			wantErr: true,
+		},
+		{
+			name: "missing subscription_id when using JWT",
+			config: ClientConfig{
+				ClientID:  "ok",
+				ClientJWT: "ok",
+			},
+			wantErr: true,
+		},
+		{
+			name: "too many client_* values",
+			config: ClientConfig{
+				SubscriptionID: "ok",
+				ClientID:       "ok",
+				ClientSecret:   "ok",
+				ClientCertPath: "error",
+			},
+			wantErr: true,
+		},
+		{
+			name: "too many client_* values (2)",
+			config: ClientConfig{
+				SubscriptionID: "ok",
+				ClientID:       "ok",
+				ClientSecret:   "ok",
+				ClientJWT:      "error",
+			},
+			wantErr: true,
+		},
+		{
+			name: "tenant_id alone should fail",
+			config: ClientConfig{
+				TenantID: "ok",
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+
+			errs := &packer.MultiError{}
+			tt.config.assertRequiredParametersSet(errs)
+			if (len(errs.Errors) != 0) != tt.wantErr {
+				t.Errorf("newConfig() error = %v, wantErr %v", errs, tt.wantErr)
+				return
+			}
+		})
+	}
+}
+
+func Test_ClientConfig_DeviceLogin(t *testing.T) {
+	getEnvOrSkip(t, "AZURE_DEVICE_LOGIN")
+	cfg := ClientConfig{
+		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 {
+		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 non-nil 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 non-nil refresh token")
+	}
+}
+
+func Test_ClientConfig_ClientPassword(t *testing.T) {
+	cfg := ClientConfig{
+		SubscriptionID:   getEnvOrSkip(t, "AZURE_SUBSCRIPTION"),
+		ClientID:         getEnvOrSkip(t, "AZURE_CLIENTID"),
+		ClientSecret:     getEnvOrSkip(t, "AZURE_CLIENTSECRET"),
+		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_ClientCert(t *testing.T) {
+	cfg := ClientConfig{
+		SubscriptionID:   getEnvOrSkip(t, "AZURE_SUBSCRIPTION"),
+		ClientID:         getEnvOrSkip(t, "AZURE_CLIENTID"),
+		ClientCertPath:   getEnvOrSkip(t, "AZURE_CLIENTCERT"),
+		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 {
+		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 getEnvOrSkip(t *testing.T, envVar string) string {
+	v := os.Getenv(envVar)
+	if v == "" {
+		t.Skipf("%s is empty, skipping", envVar)
+	}
+	return v
+}
+
+func getCloud() *azure.Environment {
+	cloudName := os.Getenv("AZURE_CLOUD")
+	if cloudName == "" {
+		cloudName = "AZUREPUBLICCLOUD"
+	}
+	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 newRandReader() io.Reader {
+	var seed int64
+	binary.Read(crand.Reader, binary.LittleEndian, &seed)
+
+	return mrand.New(mrand.NewSource(seed))
+}
+
+func getJWT(validFor time.Duration, withX5tHeader bool) string {
+	token := jwt.New(jwt.SigningMethodRS256)
+	key, _ := rsa.GenerateKey(newRandReader(), 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"))
+	}
+
+	jwt, _ := token.SignedString(key)
+	return jwt
+}

builder/azure/arm/config.go

@@ -15,7 +15,6 @@ import (
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute"
-	"github.com/Azure/go-autorest/autorest/azure"
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/masterzen/winrm"
 
@@ -32,7 +31,6 @@ import (
 )
 
 const (
-	DefaultCloudEnvironmentName              = "Public"
 	DefaultImageVersion                      = "latest"
 	DefaultUserName                          = "packer"
 	DefaultPrivateVirtualNetworkWithPublicIp = false
@@ -79,11 +77,7 @@ type Config struct {
 	common.PackerConfig `mapstructure:",squash"`
 
 	// Authentication via OAUTH
-	ClientID       string `mapstructure:"client_id"`
+	ClientConfig `mapstructure:",squash"`
-	ClientSecret   string `mapstructure:"client_secret"`
-	ObjectID       string `mapstructure:"object_id"`
-	TenantID       string `mapstructure:"tenant_id"`
-	SubscriptionID string `mapstructure:"subscription_id"`
 
 	// Capture
 	CaptureNamePrefix    string `mapstructure:"capture_name_prefix"`
@@ -122,8 +116,6 @@ type Config struct {
 	TempResourceGroupName             string             `mapstructure:"temp_resource_group_name"`
 	BuildResourceGroupName            string             `mapstructure:"build_resource_group_name"`
 	storageAccountBlobEndpoint        string
-	CloudEnvironmentName              string `mapstructure:"cloud_environment_name"`
-	cloudEnvironment                  *azure.Environment
 	PrivateVirtualNetworkWithPublicIp bool   `mapstructure:"private_virtual_network_with_public_ip"`
 	VirtualNetworkName                string `mapstructure:"virtual_network_name"`
 	VirtualNetworkSubnetName          string `mapstructure:"virtual_network_subnet_name"`
@@ -157,8 +149,6 @@ type Config struct {
 	tmpVirtualNetworkName  string
 	tmpWinRMCertificateUrl string
 
-	useDeviceLogin bool
-
 	// Authentication with the VM via SSH
 	sshAuthorizedKey string
 
@@ -287,7 +277,7 @@ func newConfig(raws ...interface{}) (*Config, []string, error) {
 	provideDefaultValues(&c)
 	setRuntimeValues(&c)
 	setUserNamePassword(&c)
-	err = setCloudEnvironment(&c)
+	err = c.ClientConfig.setCloudEnvironment()
 	if err != nil {
 		return nil, nil, err
 	}
@@ -416,40 +406,6 @@ func setUserNamePassword(c *Config) {
 	}
 }
 
-func setCloudEnvironment(c *Config) error {
-	lookup := map[string]string{
-		"CHINA":           "AzureChinaCloud",
-		"CHINACLOUD":      "AzureChinaCloud",
-		"AZURECHINACLOUD": "AzureChinaCloud",
-
-		"GERMAN":           "AzureGermanCloud",
-		"GERMANCLOUD":      "AzureGermanCloud",
-		"AZUREGERMANCLOUD": "AzureGermanCloud",
-
-		"GERMANY":           "AzureGermanCloud",
-		"GERMANYCLOUD":      "AzureGermanCloud",
-		"AZUREGERMANYCLOUD": "AzureGermanCloud",
-
-		"PUBLIC":           "AzurePublicCloud",
-		"PUBLICCLOUD":      "AzurePublicCloud",
-		"AZUREPUBLICCLOUD": "AzurePublicCloud",
-
-		"USGOVERNMENT":           "AzureUSGovernmentCloud",
-		"USGOVERNMENTCLOUD":      "AzureUSGovernmentCloud",
-		"AZUREUSGOVERNMENTCLOUD": "AzureUSGovernmentCloud",
-	}
-
-	name := strings.ToUpper(c.CloudEnvironmentName)
-	envName, ok := lookup[name]
-	if !ok {
-		return fmt.Errorf("There is no cloud environment matching the name '%s'!", c.CloudEnvironmentName)
-	}
-
-	env, err := azure.EnvironmentFromName(envName)
-	c.cloudEnvironment = &env
-	return err
-}
-
 func setCustomData(c *Config) error {
 	if c.CustomDataFile == "" {
 		return nil
@@ -481,9 +437,7 @@ func provideDefaultValues(c *Config) {
 		c.ImageVersion = DefaultImageVersion
 	}
 
-	if c.CloudEnvironmentName == "" {
+	c.provideDefaultValues()
-		c.CloudEnvironmentName = DefaultCloudEnvironmentName
-	}
 }
 
 func assertTagProperties(c *Config, errs *packer.MultiError) {
@@ -502,37 +456,7 @@ func assertTagProperties(c *Config, errs *packer.MultiError) {
 }
 
 func assertRequiredParametersSet(c *Config, errs *packer.MultiError) {
-	/////////////////////////////////////////////
+	c.ClientConfig.assertRequiredParametersSet(errs)
-	// Authentication via OAUTH
-
-	// Check if device login is being asked for, and is allowed.
-	//
-	// Device login is enabled if the user only defines SubscriptionID and not
-	// ClientID, ClientSecret, and TenantID.
-	//
-	// Device login is not enabled for Windows because the WinRM certificate is
-	// 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.
-	isUseDeviceLogin := func(c *Config) bool {
-
-		return c.SubscriptionID != "" &&
-			c.ClientID == "" &&
-			c.ClientSecret == "" &&
-			c.TenantID == ""
-	}
-
-	if isUseDeviceLogin(c) {
-		c.useDeviceLogin = true
-	} else {
-		if c.ClientID == "" && c.ClientSecret != "" || c.ClientID != "" && c.ClientSecret == "" {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("A client_id and client_secret must be specified together or not specified at all"))
-		}
-
-		if c.ClientID != "" && c.SubscriptionID == "" {
-			errs = packer.MultiErrorAppend(errs, fmt.Errorf("A subscription_id must be specified when client_id & client_secret are"))
-		}
-
-	}
 
 	/////////////////////////////////////////////
 	// Capture

builder/azure/arm/config_test.go

@@ -133,87 +133,6 @@ func TestConfigShouldNotDefaultImageVersionIfCustomImage(t *testing.T) {
 	}
 }
 
-func Test_newConfig_MSI(t *testing.T) {
-	baseConfig := map[string]string{
-		"capture_name_prefix":    "ignore",
-		"capture_container_name": "ignore",
-		"location":               "ignore",
-		"image_url":              "ignore",
-		"storage_account":        "ignore",
-		"resource_group_name":    "ignore",
-		"os_type":                constants.Target_Linux,
-	}
-
-	tests := []struct {
-		name    string
-		args    []interface{}
-		wantErr bool
-	}{
-		{
-			name: "no client_id and no client_secret should enable MSI auth",
-			args: []interface{}{
-				baseConfig,
-				getPackerConfiguration(),
-			},
-			wantErr: false,
-		},
-		{
-			name: "subscription_id is will be taken from MSI",
-			args: []interface{}{
-				baseConfig,
-				map[string]string{
-					"subscription_id": "error",
-				},
-				getPackerConfiguration(),
-			},
-			wantErr: false,
-		},
-		{
-			name: "client_id without client_secret should error",
-			args: []interface{}{
-				baseConfig,
-				map[string]string{
-					"client_id": "error",
-				},
-				getPackerConfiguration(),
-			},
-			wantErr: true,
-		},
-		{
-			name: "client_secret without client_id should error",
-			args: []interface{}{
-				baseConfig,
-				map[string]string{
-					"client_secret": "error",
-				},
-				getPackerConfiguration(),
-			},
-			wantErr: true,
-		},
-		{
-			name: "missing subscription_id",
-			args: []interface{}{
-				baseConfig,
-				map[string]string{
-					"client_id":     "ok",
-					"client_secret": "ok",
-				},
-				getPackerConfiguration(),
-			},
-			wantErr: true,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			_, _, err := newConfig(tt.args...)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("newConfig() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-		})
-	}
-}
-
 func TestConfigShouldNormalizeOSTypeCase(t *testing.T) {
 	config := map[string]string{
 		"capture_name_prefix":    "ignore",

builder/azure/arm/step_power_off_compute.go

@@ -28,7 +28,7 @@ func NewStepPowerOffCompute(client *AzureClient, ui packer.Ui) *StepPowerOffComp
 }
 
 func (s *StepPowerOffCompute) powerOffCompute(ctx context.Context, resourceGroupName string, computeName string) error {
-	f, err := s.client.VirtualMachinesClient.PowerOff(ctx, resourceGroupName, computeName)
+	f, err := s.client.VirtualMachinesClient.Deallocate(ctx, resourceGroupName, computeName)
 	if err == nil {
 		err = f.WaitForCompletion(ctx, s.client.VirtualMachinesClient.Client)
 	}

builder/hyperv/common/step_output_dir_test.go

@@ -1,146 +0,0 @@
-package common
-
-import (
-	"context"
-	"os"
-	"testing"
-
-	"github.com/hashicorp/packer/helper/multistep"
-)
-
-func TestStepOutputDir_imp(t *testing.T) {
-	var _ multistep.Step = new(StepOutputDir)
-}
-
-func TestStepOutputDir_Default(t *testing.T) {
-	state := testState(t)
-	step := new(StepOutputDir)
-
-	step.Path = genTestDirPath("packerHypervOutput")
-
-	// Test the run
-	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
-		t.Fatalf("Bad action: %v", action)
-	}
-	if _, ok := state.GetOk("error"); ok {
-		t.Fatal("Should NOT have error")
-	}
-
-	// The directory should have been created
-	if _, err := os.Stat(step.Path); err != nil {
-		t.Fatal("Should have created output directory")
-	}
-
-	// Remove the directory created due to test
-	err := os.RemoveAll(step.Path)
-	if err != nil {
-		t.Fatalf("Error encountered removing directory created by test: %s", err)
-	}
-}
-
-func TestStepOutputDir_DirectoryAlreadyExistsNoForce(t *testing.T) {
-	state := testState(t)
-	step := new(StepOutputDir)
-
-	step.Path = genTestDirPath("packerHypervOutput")
-
-	// Create the directory so that we can test
-	err := os.Mkdir(step.Path, 0755)
-	if err != nil {
-		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
-	}
-	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
-
-	step.Force = false // Default
-	// Test the run
-	if action := step.Run(context.Background(), state); action != multistep.ActionHalt {
-		t.Fatalf("Should halt when directory exists and 'Force' is false. Bad action: %v", action)
-	}
-	if _, ok := state.GetOk("error"); !ok {
-		t.Fatal("Should error when directory exists and 'Force' is false")
-	}
-}
-
-func TestStepOutputDir_DirectoryAlreadyExistsForce(t *testing.T) {
-	state := testState(t)
-	step := new(StepOutputDir)
-
-	step.Path = genTestDirPath("packerHypervOutput")
-
-	// Create the directory so that we can test
-	err := os.Mkdir(step.Path, 0755)
-	if err != nil {
-		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
-	}
-	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
-
-	step.Force = true // User specified that existing directory and contents should be discarded
-	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
-		t.Fatalf("Should complete when directory exists and 'Force' is true. Bad action: %v", action)
-	}
-	if _, ok := state.GetOk("error"); ok {
-		t.Fatalf("Should NOT error when directory exists and 'Force' is true: %s", err)
-	}
-}
-
-func TestStepOutputDir_CleanupBuildCancelled(t *testing.T) {
-	state := testState(t)
-	step := new(StepOutputDir)
-
-	step.Path = genTestDirPath("packerHypervOutput")
-
-	// Create the directory so that we can test the cleanup
-	err := os.Mkdir(step.Path, 0755)
-	if err != nil {
-		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
-	}
-	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
-
-	// 'Cancel' the build
-	state.Put(multistep.StateCancelled, true)
-
-	// Ensure the directory isn't removed if the cleanup flag is false
-	step.cleanup = false
-	step.Cleanup(state)
-	if _, err := os.Stat(step.Path); err != nil {
-		t.Fatal("Output dir should NOT be removed if on 'Cancel' if cleanup flag is unset/false")
-	}
-
-	// Ensure the directory is removed if the cleanup flag is true
-	step.cleanup = true
-	step.Cleanup(state)
-	if _, err := os.Stat(step.Path); err == nil {
-		t.Fatalf("Output directory should NOT exist after 'Cancel' and Cleanup: %s", step.Path)
-	}
-}
-
-func TestStepOutputDir_CleanupBuildHalted(t *testing.T) {
-	state := testState(t)
-	step := new(StepOutputDir)
-
-	step.Path = genTestDirPath("packerHypervOutput")
-
-	// Create the directory so that we can test the cleanup
-	err := os.Mkdir(step.Path, 0755)
-	if err != nil {
-		t.Fatal("Test failed to create directory for test of Cancel and Cleanup")
-	}
-	defer os.RemoveAll(step.Path) // Ensure we clean up if something goes wrong
-
-	// 'Halt' the build and test the directory is removed
-	state.Put(multistep.StateHalted, true)
-
-	// Ensure the directory isn't removed if the cleanup flag is false
-	step.cleanup = false
-	step.Cleanup(state)
-	if _, err := os.Stat(step.Path); err != nil {
-		t.Fatal("Output dir should NOT be removed if on 'Halt' if cleanup flag is unset/false")
-	}
-
-	// Ensure the directory is removed if the cleanup flag is true
-	step.cleanup = true
-	step.Cleanup(state)
-	if _, err := os.Stat(step.Path); err == nil {
-		t.Fatalf("Output directory should NOT exist after 'Halt' and Cleanup: %s", step.Path)
-	}
-}

builder/hyperv/iso/builder.go

@@ -376,7 +376,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&hypervcommon.StepCreateBuildDir{
 			TempPath: b.config.TempPath,
 		},
-		&hypervcommon.StepOutputDir{
+		&common.StepOutputDir{
 			Force: b.config.PackerForce,
 			Path:  b.config.OutputDir,
 		},

builder/hyperv/vmcx/builder.go

@@ -392,7 +392,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 		&hypervcommon.StepCreateBuildDir{
 			TempPath: b.config.TempPath,
 		},
-		&hypervcommon.StepOutputDir{
+		&common.StepOutputDir{
 			Force: b.config.PackerForce,
 			Path:  b.config.OutputDir,
 		},

builder/parallels/common/driver_9.go

@@ -154,6 +154,7 @@ func (d *Parallels9Driver) DeviceAddCDROM(name string, image string) (string, er
 		"set", name,
 		"--device-add", "cdrom",
 		"--image", image,
+		"--enable", "--connect",
 	}
 
 	out, err := exec.Command(d.PrlctlPath, command...).Output()

builder/virtualbox/common/step_output_dir.go

@@ -1,86 +0,0 @@
-package common
-
-import (
-	"context"
-	"fmt"
-	"log"
-	"os"
-	"path/filepath"
-	"time"
-
-	"github.com/hashicorp/packer/helper/multistep"
-	"github.com/hashicorp/packer/packer"
-)
-
-// StepOutputDir sets up the output directory by creating it if it does
-// not exist, deleting it if it does exist and we're forcing, and cleaning
-// it up when we're done with it.
-type StepOutputDir struct {
-	Force bool
-	Path  string
-
-	cleanup bool
-}
-
-func (s *StepOutputDir) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
-	ui := state.Get("ui").(packer.Ui)
-
-	if _, err := os.Stat(s.Path); err == nil {
-		if !s.Force {
-			err := fmt.Errorf(
-				"Output directory exists: %s\n\n"+
-					"Use the force flag to delete it prior to building.",
-				s.Path)
-			state.Put("error", err)
-			return multistep.ActionHalt
-		}
-
-		ui.Say("Deleting previous output directory...")
-		os.RemoveAll(s.Path)
-	}
-
-	// Enable cleanup
-	s.cleanup = true
-
-	// Create the directory
-	if err := os.MkdirAll(s.Path, 0755); err != nil {
-		state.Put("error", err)
-		return multistep.ActionHalt
-	}
-
-	// Make sure we can write in the directory
-	f, err := os.Create(filepath.Join(s.Path, "_packer_perm_check"))
-	if err != nil {
-		err = fmt.Errorf("Couldn't write to output directory: %s", err)
-		state.Put("error", err)
-		return multistep.ActionHalt
-	}
-	f.Close()
-	os.Remove(f.Name())
-
-	return multistep.ActionContinue
-}
-
-func (s *StepOutputDir) Cleanup(state multistep.StateBag) {
-	if !s.cleanup {
-		return
-	}
-
-	_, cancelled := state.GetOk(multistep.StateCancelled)
-	_, halted := state.GetOk(multistep.StateHalted)
-
-	if cancelled || halted {
-		ui := state.Get("ui").(packer.Ui)
-
-		ui.Say("Deleting output directory...")
-		for i := 0; i < 5; i++ {
-			err := os.RemoveAll(s.Path)
-			if err == nil {
-				break
-			}
-
-			log.Printf("Error removing output dir: %s", err)
-			time.Sleep(2 * time.Second)
-		}
-	}
-}

builder/virtualbox/iso/builder.go

@@ -209,7 +209,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			TargetPath:   b.config.TargetPath,
 			Url:          b.config.ISOUrls,
 		},
-		&vboxcommon.StepOutputDir{
+		&common.StepOutputDir{
 			Force: b.config.PackerForce,
 			Path:  b.config.OutputDir,
 		},

builder/virtualbox/ovf/builder.go

@@ -50,7 +50,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 
 	// Build the steps.
 	steps := []multistep.Step{
-		&vboxcommon.StepOutputDir{
+		&common.StepOutputDir{
 			Force: b.config.PackerForce,
 			Path:  b.config.OutputDir,
 		},

common/download.go

@@ -191,8 +191,9 @@ func (d *DownloadClient) Get() (string, error) {
 			}
 
 			err = fmt.Errorf(
-				"checksums didn't match expected: %s",
+				"checksums didn't match. expected %s and got %s",
-				hex.EncodeToString(d.config.Checksum))
+				hex.EncodeToString(d.config.Checksum),
+				hex.EncodeToString(d.config.Hash.Sum(nil)))
 		}
 	}
 

common/download_test.go

@@ -411,7 +411,7 @@ func TestDownloadFileUrl(t *testing.T) {
 
 	// Verify that we fail to match the checksum
 	_, err = client.Get()
-	if err.Error() != "checksums didn't match expected: 6e6f7065" {
+	if err.Error() != "checksums didn't match. expected 6e6f7065 and got 606f1945f81a022d0ed0bd99edfd4f99081c1cb1f97fae087291ee14e945e608" {
 		t.Fatalf("Unexpected failure; expected checksum not to match. Error was \"%v\"", err)
 	}
 
@@ -442,7 +442,7 @@ func SimulateFileUriDownload(t *testing.T, uri string) (string, error) {
 	path, err := client.Get()
 
 	// ignore any non-important checksum errors if it's not a unc path
-	if !strings.HasPrefix(path, "\\\\") && err.Error() != "checksums didn't match expected: 6e6f7065" {
+	if !strings.HasPrefix(path, "\\\\") && err.Error() != "checksums didn't match. expected 6e6f7065 and got 606f1945f81a022d0ed0bd99edfd4f99081c1cb1f97fae087291ee14e945e608" {
 		t.Fatalf("Unexpected failure; expected checksum not to match")
 	}
 

common/step_output_dir_test.go

@@ -1,14 +1,25 @@
 package common
 
 import (
+	"bytes"
 	"context"
 	"io/ioutil"
 	"os"
 	"testing"
 
 	"github.com/hashicorp/packer/helper/multistep"
+	"github.com/hashicorp/packer/packer"
 )
 
+func testState(t *testing.T) multistep.StateBag {
+	state := new(multistep.BasicStateBag)
+	state.Put("ui", &packer.BasicUi{
+		Reader: new(bytes.Buffer),
+		Writer: new(bytes.Buffer),
+	})
+	return state
+}
+
 func testStepOutputDir(t *testing.T) *StepOutputDir {
 	td, err := ioutil.TempDir("", "packer")
 	if err != nil {

post-processor/amazon-import/post-processor.go

@@ -203,7 +203,20 @@ func (p *PostProcessor) PostProcess(ui packer.Ui, artifact packer.Artifact) (pac
 	ui.Message(fmt.Sprintf("Waiting for task %s to complete (may take a while)", *import_start.ImportTaskId))
 	err = awscommon.WaitUntilImageImported(aws.BackgroundContext(), ec2conn, *import_start.ImportTaskId)
 	if err != nil {
-		return nil, false, fmt.Errorf("Import task %s failed with error: %s", *import_start.ImportTaskId, err)
+
+		// Retrieve the status message
+		import_result, err2 := ec2conn.DescribeImportImageTasks(&ec2.DescribeImportImageTasksInput{
+			ImportTaskIds: []*string{
+				import_start.ImportTaskId,
+			},
+		})
+
+		statusMessage := "Error retrieving status message"
+
+		if err2 == nil {
+			statusMessage = *import_result.ImportImageTasks[0].StatusMessage
+		}
+		return nil, false, fmt.Errorf("Import task %s failed with status message: %s, error: %s", *import_start.ImportTaskId, statusMessage, err)
 	}
 
 	// Retrieve what the outcome was for the import task
@@ -216,7 +229,6 @@ func (p *PostProcessor) PostProcess(ui packer.Ui, artifact packer.Artifact) (pac
 	if err != nil {
 		return nil, false, fmt.Errorf("Failed to find import task %s: %s", *import_start.ImportTaskId, err)
 	}
-
 	// Check it was actually completed
 	if *import_result.ImportImageTasks[0].Status != "completed" {
 		// The most useful error message is from the job itself

website/source/docs/builders/azure-setup.html.md

@@ -204,37 +204,12 @@ Make sure that `GROUPNAME` and `LOCATION` are the same as above. Also, ensure
 that `GROUPNAME` is less than 24 characters long and contains only lowercase
 letters and numbers.
 
-### Create an Application
-
-An application represents a way to authorize access to the Azure API. Note that
-you will need to specify a URL for your application (this is intended to be
-used for OAuth callbacks) but these do not actually need to be valid URLs.
-
-First pick APPNAME, APPURL and PASSWORD:
-
-``` shell
-APPNAME=packer.test
-APPURL=packer.test
-PASSWORD=xxx
-```
-
-Password is your `client_secret` and can be anything you like. I recommend
-using `openssl rand -base64 24`.
-
-``` shell
-$ az ad app create \
-  --display-name $APPNAME \
-  --identifier-uris $APPURL \
-  --homepage $APPURL \
-  --password $PASSWORD
-```
-
 ### Create a Service Principal
 
-You cannot directly grant permissions to an application. Instead, you create a
+A service principal acts on behalf of an application (Packer) on your Azure
-service principal and assign permissions to the service principal. To create a
+subscription. To create an application and service principal for use with
-service principal for use with Packer, run the below command specifying the
+Packer, run the below command specifying the subscription. This will grant
-subscription. This will grant Packer the contributor role to the subscription.
+Packer the contributor role to the subscription.
 The output of this command is your service principal credentials, save these in
 a safe place as you will need these to configure Packer.
 
@@ -263,8 +238,13 @@ pre-configured roles via:
 $ az role definition list --output json | jq ".[] | {name:.roleName, description:.description}"
 ```
 
+If you would rather use a certificate to autenticate your service principal,
+please follow the [Azure Active Directory documentation](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-certificate-credentials#register-your-certificate-with-azure-ad).
+
 ### Configuring Packer
 
 Now (finally) everything has been setup in Azure and our service principal has
 been created. You can use the output from creating your service principal in
-your template.
+your template. Use the value from the `appId` field above as a value for
+`client_id` in your configuration and set `client_secret` to the `password`
+value from above.

website/source/docs/builders/azure.html.md

@@ -35,12 +35,15 @@ addition to the options listed here, a
 [communicator](/docs/templates/communicator.html) can be configured for this
 builder.
 
-### Required ( unless instance has [managed identities](/docs/builders/azure-setup.html#managed-identities-for-azure-resources) enabled):
+### Required options for authentication:
-
+If you're running packer on an Azure VM with a [managed identity](/docs/builders/azure-setup.html#managed-identities-for-azure-resources)
--   `client_id` (string) The Active Directory service principal associated with
+you don't need to specify any additional configuration options.
-    your builder.
+If you would like to use interactive user authentication, you should specify
-
+`subscription_id` only. Packer will use cached credentials or redirect you
--   `client_secret` (string) The password or secret for your service principal.
+to a website to log in.
+If you want to use a [service principal](/docs/builders/azure-setup.html#create-a-service-principal)
+you should specify `subscription_id`, `client_id` and one of `client_secret`,
+`client_cert_path` or `client_jwt`.
 
 -   `subscription_id` (string) Subscription under which the build will be
     performed. **The service principal specified in `client_id` must have full
@@ -48,6 +51,19 @@ builder.
     specified in which case it needs to have owner access to the existing
     resource group specified in build\_resource\_group\_name parameter.**
 
+-   `client_id` (string) The Active Directory service principal associated with
+    your builder.
+
+-   `client_secret` (string) The password or secret for your service principal.
+
+-   `client_cert_path` (string) The location of a PEM file containing a
+    certificate and private key for service principal.
+
+-   `client_jwt` (string) The bearer JWT assertion signed using a certificate
+    associated with your service principal principal. See [Azure Active
+    Directory docs](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-certificate-credentials)
+    for more information.
+
 ### Required:
 
 -   `image_publisher` (string) PublisherName for your base image. See