Skip to content

[Bug] confidential.CertFromPEM fails with Azure Key Vault: "failed to parse private key (use ParsePKCS1PrivateKey instead for this key format)" #559

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.

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

Open
dagood opened this issue Mar 10, 2025 · 0 comments
Assignees
Labels
bug Something isn't working confidential-client p2

Comments

@dagood
Copy link

dagood commented Mar 10, 2025

Reopening this issue because I want to understand better why it was closed:

It was closed as won't-fix with a recommendation to use a workaround. It doesn't seem reasonable to me that a bug with AzureAD/microsoft-authentication-library-for-go interacting with Azure Key Vault won't ever be fixed.

I think it makes sense that it doesn't have high priority because the workarounds are relatively straightforward, but this wasn't the reason given for closing the issue, and it seems odd not to at least keep track of this bug with how the library talks to what should be a pretty ordinary Azure endpoint.


My recommended workaround for library users: modify blocks when iterating through it:

	// ...
	for _, block := range blocks {
		if block.Type == "PRIVATE KEY" {
			_, err = x509.ParsePKCS1PrivateKey(block.Bytes)
			if err == nil {
				block.Type = "RSA PRIVATE KEY"
			}
		}
		err := pem.Encode(&pemBuf, block)
		if err != nil {
			return fail("unable to encode PEM block")
		}
	}
	// ...

(My usage)


Which version of MSAL Go are you using?
github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 (latest)

Where is the issue?
Confidential client - client certificate

case "PRIVATE KEY":
if priv != nil {
return nil, nil, errors.New("found multiple private key blocks")
}
var err error
priv, err = x509.ParsePKCS8PrivateKey(block.Bytes)
if err != nil {
return nil, nil, fmt.Errorf("could not decode private key: %v", err)
}
case "RSA PRIVATE KEY":
if priv != nil {
return nil, nil, errors.New("found multiple private key blocks")
}
var err error
priv, err = x509.ParsePKCS1PrivateKey(block.Bytes)
if err != nil {
return nil, nil, fmt.Errorf("could not decode private key: %v", err)
}

This code assumes that all PRIVATE KEY blocks can be parsed by ParsePKCS8PrivateKey and all RSA PRIVATE KEY blocks can be parsed by ParsePKCS1PrivateKey, but Azure Key Vault gives me a PRIVATE KEY block that requires ParsePKCS1PrivateKey.

As far as I know, the block type in PEM files is technically only a hint, not well-specified. I'm not sure whether this bug is really on Azure Key Vault or this library, but it seems reasonable for this library to support Azure Key Vault in practice.

Is this a new or an existing app?
New experiment.

What version of Go are you using (go version)?
go1.22.3

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
set GOARCH=amd64
set GOHOSTARCH=amd64
set GOHOSTOS=windows
...

Repro
Get a JSON blob from Azure Key Vault using az keyvault secret show [...]. Pass the string as bytes into this function to try to decode and set it up as a confidential.Credential that uses its cert:

func NewCredFromAzureKeyVaultJSON(vaultJSON []byte) (confidential.Credential, error) {
	fail := func(err string) (confidential.Credential, error) {
		return confidential.Credential{}, errors.New(err)
	}
	var data struct {
		Value string `json:"value"`
	}
	if err := json.Unmarshal(vaultJSON, &data); err != nil {
		return fail("unable to decode JSON")
	}
	pfx, err := base64.StdEncoding.DecodeString(data.Value)
	if err != nil {
		return fail("unable to decode base64 value")
	}
	blocks, err := pkcs12.ToPEM(pfx, "")
	if err != nil {
		return fail("unable to convert PFX data to PEM blocks")
	}
	// Multiple blocks are expected. Find the private key and certificates.
	var pemBuf bytes.Buffer
	for _, block := range blocks {
		err := pem.Encode(&pemBuf, block)
		if err != nil {
			return fail("unable to encode PEM block")
		}
	}
	certs, priv, err := confidential.CertFromPEM(pemBuf.Bytes(), "")
	if err != nil {
		return fail("unable to create cert from PEM blocks")
	}
	return confidential.NewCredFromCert(certs, priv)
}

I don't have an example cert or Azure Key Vault response ready at the moment, unfortuantely. I'm not sure how the one I'm using was created. This may only affect a certain kind of certificate.

Expected behavior
Returns a usable confidential.Credential.

Actual behavior
Get an error from confidential.CertFromPEM:

could not decode private key: x509: failed to parse private key (use ParsePKCS1PrivateKey instead for this key format)

This comes from the case "PRIVATE KEY": case, not "RSA PRIVATE KEY".

Possible solution
Replacing confidential.CertFromPEM with a custom copy with this change works, and the resulting confidential.Credential is usable:

 		case "PRIVATE KEY":
[...]
 			priv, err = x509.ParsePKCS8PrivateKey(block.Bytes)
+			// Also try ParsePKCS1PrivateKey, because this might not be compatible with ParsePKCS8PrivateKey.
+			if err != nil {
+ 				priv, err = x509.ParsePKCS1PrivateKey(block.Bytes)
+			}
 			if err != nil {
 				return nil, nil, fmt.Errorf("could not decode private key: %v", err)
 			}

(Keeping track of both errors and returning both might be better, rather than overwriting one with the other.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confidential-client p2
Projects
None yet
Development

No branches or pull requests

3 participants