Merge pull request #5597 from k8s-infra-cherrypick-robot/cherry-pick-…

…5594-to-release-1.27 [release-1.27] chore: support service principal with cert auth for multi tenant scenario
kubernetes-sigs · Mar 7, 2024 · 2287ba5 · 2287ba5
2 parents 51206a2 + d37be38
commit 2287ba5
Show file tree

Hide file tree

Showing 3 changed files with 108 additions and 26 deletions.
diff --git a/pkg/provider/config/azure_auth.go b/pkg/provider/config/azure_auth.go
@@ -17,8 +17,6 @@ limitations under the License.
 package config
 
 import (
-	"crypto/rsa"
-	"crypto/x509"
 	"errors"
 	"fmt"
 	"io"
@@ -30,8 +28,6 @@ import (
 	"github.com/Azure/go-autorest/autorest/adal"
 	"github.com/Azure/go-autorest/autorest/azure"
 
-	"golang.org/x/crypto/pkcs12"
-
 	"k8s.io/klog/v2"
 
 	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
@@ -143,13 +139,13 @@ func GetServicePrincipalToken(config *AzureAuthConfig, env *azure.Environment, r
 			resource)
 	}
 
-	if len(config.AADClientCertPath) > 0 && len(config.AADClientCertPassword) > 0 {
+	if len(config.AADClientCertPath) > 0 {
 		klog.V(2).Infoln("azure: using jwt client_assertion (client_cert+client_private_key) to retrieve access token")
 		certData, err := os.ReadFile(config.AADClientCertPath)
 		if err != nil {
 			return nil, fmt.Errorf("reading the client certificate from file %s: %w", config.AADClientCertPath, err)
 		}
-		certificate, privateKey, err := decodePkcs12(certData, config.AADClientCertPassword)
+		certificate, privateKey, err := adal.DecodePfxCertificateData(certData, config.AADClientCertPassword)
 		if err != nil {
 			return nil, fmt.Errorf("decoding the client certificate: %w", err)
 		}
@@ -193,8 +189,22 @@ func GetMultiTenantServicePrincipalToken(config *AzureAuthConfig, env *azure.Env
 			env.ServiceManagementEndpoint)
 	}
 
-	if len(config.AADClientCertPath) > 0 && len(config.AADClientCertPassword) > 0 {
-		return nil, fmt.Errorf("AAD Application client certificate authentication is not supported in getting multi-tenant service principal token")
+	if len(config.AADClientCertPath) > 0 {
+		klog.V(2).Infoln("azure: using jwt client_assertion (client_cert+client_private_key) to retrieve multi-tenant access token")
+		certData, err := os.ReadFile(config.AADClientCertPath)
+		if err != nil {
+			return nil, fmt.Errorf("reading the client certificate from file %s: %w", config.AADClientCertPath, err)
+		}
+		certificate, privateKey, err := adal.DecodePfxCertificateData(certData, config.AADClientCertPassword)
+		if err != nil {
+			return nil, fmt.Errorf("decoding the client certificate: %w", err)
+		}
+		return adal.NewMultiTenantServicePrincipalTokenFromCertificate(
+			multiTenantOAuthConfig,
+			config.AADClientID,
+			certificate,
+			privateKey,
+			env.ServiceManagementEndpoint)
 	}
 
 	return nil, ErrorNoAuth
@@ -226,8 +236,22 @@ func GetNetworkResourceServicePrincipalToken(config *AzureAuthConfig, env *azure
 			env.ServiceManagementEndpoint)
 	}
 
-	if len(config.AADClientCertPath) > 0 && len(config.AADClientCertPassword) > 0 {
-		return nil, fmt.Errorf("AAD Application client certificate authentication is not supported in getting network resources service principal token")
+	if len(config.AADClientCertPath) > 0 {
+		klog.V(2).Infoln("azure: using jwt client_assertion (client_cert+client_private_key) to retrieve access token for network resources tenant")
+		certData, err := os.ReadFile(config.AADClientCertPath)
+		if err != nil {
+			return nil, fmt.Errorf("reading the client certificate from file %s: %w", config.AADClientCertPath, err)
+		}
+		certificate, privateKey, err := adal.DecodePfxCertificateData(certData, config.AADClientCertPassword)
+		if err != nil {
+			return nil, fmt.Errorf("decoding the client certificate: %w", err)
+		}
+		return adal.NewServicePrincipalTokenFromCertificate(
+			*oauthConfig,
+			config.AADClientID,
+			certificate,
+			privateKey,
+			env.ServiceManagementEndpoint)
 	}
 
 	return nil, ErrorNoAuth
@@ -299,21 +323,6 @@ func (config *AzureAuthConfig) UsesNetworkResourceInDifferentSubscription() bool
 	return len(config.NetworkResourceSubscriptionID) > 0 && !strings.EqualFold(config.NetworkResourceSubscriptionID, config.SubscriptionID)
 }
 
-// decodePkcs12 decodes a PKCS#12 client certificate by extracting the public certificate and
-// the private RSA key
-func decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.PrivateKey, error) {
-	privateKey, certificate, err := pkcs12.Decode(pkcs, password)
-	if err != nil {
-		return nil, nil, fmt.Errorf("decoding the PKCS#12 client certificate: %w", err)
-	}
-	rsaPrivateKey, isRsaKey := privateKey.(*rsa.PrivateKey)
-	if !isRsaKey {
-		return nil, nil, fmt.Errorf("PKCS#12 certificate must contain a RSA private key")
-	}
-
-	return certificate, rsaPrivateKey, nil
-}
-
 // azureStackOverrides ensures that the Environment matches what AKSe currently generates for Azure Stack
 func azureStackOverrides(env *azure.Environment, resourceManagerEndpoint, identitySystem string) {
 	env.ManagementPortalURL = strings.Replace(resourceManagerEndpoint, "https://management.", "https://portal.", -1)

diff --git a/pkg/provider/config/azure_auth_test.go b/pkg/provider/config/azure_auth_test.go
@@ -262,13 +262,60 @@ func TestGetServicePrincipalTokenFromCertificate(t *testing.T) {
 	assert.NoError(t, err)
 	pfxContent, err := os.ReadFile("./testdata/test.pfx")
 	assert.NoError(t, err)
-	certificate, privateKey, err := decodePkcs12(pfxContent, "id")
+	certificate, privateKey, err := adal.DecodePfxCertificateData(pfxContent, "id")
 	assert.NoError(t, err)
 	spt, err := adal.NewServicePrincipalTokenFromCertificate(*oauthConfig, config.AADClientID, certificate, privateKey, env.ServiceManagementEndpoint)
 	assert.NoError(t, err)
 	assert.Equal(t, token, spt)
 }
 
+func TestGetServicePrincipalTokenFromCertificateWithoutPassword(t *testing.T) {
+	config := &AzureAuthConfig{
+		TenantID:          "TenantID",
+		AADClientID:       "AADClientID",
+		AADClientCertPath: "./testdata/testnopassword.pfx",
+	}
+	env := &azure.PublicCloud
+	token, err := GetServicePrincipalToken(config, env, "")
+	assert.NoError(t, err)
+
+	oauthConfig, err := adal.NewOAuthConfigWithAPIVersion(env.ActiveDirectoryEndpoint, config.TenantID, nil)
+	assert.NoError(t, err)
+	pfxContent, err := os.ReadFile("./testdata/testnopassword.pfx")
+	assert.NoError(t, err)
+	certificate, privateKey, err := adal.DecodePfxCertificateData(pfxContent, "")
+	assert.NoError(t, err)
+	spt, err := adal.NewServicePrincipalTokenFromCertificate(*oauthConfig, config.AADClientID, certificate, privateKey, env.ServiceManagementEndpoint)
+	assert.NoError(t, err)
+	assert.Equal(t, token, spt)
+}
+
+func TestGetMultiTenantServicePrincipalTokenFromCertificate(t *testing.T) {
+	config := &AzureAuthConfig{
+		TenantID:                      "TenantID",
+		NetworkResourceTenantID:       "NetworkResourceTenantID",
+		AADClientID:                   "AADClientID",
+		AADClientCertPath:             "./testdata/testnopassword.pfx",
+		NetworkResourceSubscriptionID: "NetworkResourceSubscriptionID",
+	}
+	env := &azure.PublicCloud
+
+	multiTenantToken, err := GetMultiTenantServicePrincipalToken(config, env)
+	assert.NoError(t, err)
+
+	multiTenantOAuthConfig, err := adal.NewMultiTenantOAuthConfig(env.ActiveDirectoryEndpoint, config.TenantID, []string{config.NetworkResourceTenantID}, adal.OAuthOptions{})
+	assert.NoError(t, err)
+
+	pfxContent, err := os.ReadFile("./testdata/testnopassword.pfx")
+	assert.NoError(t, err)
+	certificate, privateKey, err := adal.DecodePfxCertificateData(pfxContent, "")
+	assert.NoError(t, err)
+	spt, err := adal.NewMultiTenantServicePrincipalTokenFromCertificate(multiTenantOAuthConfig, config.AADClientID, certificate, privateKey, env.ServiceManagementEndpoint)
+	assert.NoError(t, err)
+
+	assert.Equal(t, multiTenantToken, spt)
+}
+
 func TestGetMultiTenantServicePrincipalTokenNegative(t *testing.T) {
 	env := &azure.PublicCloud
 	for _, config := range CrossTenantNetworkResourceNegativeConfig {
@@ -299,6 +346,32 @@ func TestGetNetworkResourceServicePrincipalToken(t *testing.T) {
 	assert.Equal(t, token, spt)
 }
 
+func TestGetNetworkResourceServicePrincipalTokenFromCertificate(t *testing.T) {
+	config := &AzureAuthConfig{
+		TenantID:                      "TenantID",
+		NetworkResourceTenantID:       "NetworkResourceTenantID",
+		AADClientID:                   "AADClientID",
+		AADClientCertPath:             "./testdata/testnopassword.pfx",
+		NetworkResourceSubscriptionID: "NetworkResourceSubscriptionID",
+	}
+	env := &azure.PublicCloud
+
+	token, err := GetNetworkResourceServicePrincipalToken(config, env)
+	assert.NoError(t, err)
+
+	oauthConfig, err := adal.NewOAuthConfigWithAPIVersion(env.ActiveDirectoryEndpoint, config.NetworkResourceTenantID, nil)
+	assert.NoError(t, err)
+
+	pfxContent, err := os.ReadFile("./testdata/testnopassword.pfx")
+	assert.NoError(t, err)
+	certificate, privateKey, err := adal.DecodePfxCertificateData(pfxContent, "")
+	assert.NoError(t, err)
+	spt, err := adal.NewServicePrincipalTokenFromCertificate(*oauthConfig, config.AADClientID, certificate, privateKey, env.ServiceManagementEndpoint)
+	assert.NoError(t, err)
+
+	assert.Equal(t, token, spt)
+}
+
 func TestGetNetworkResourceServicePrincipalTokenNegative(t *testing.T) {
 	env := &azure.PublicCloud
 	for _, config := range CrossTenantNetworkResourceNegativeConfig {

diff --git a/pkg/provider/config/testdata/testnopassword.pfx b/pkg/provider/config/testdata/testnopassword.pfx