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

Jump to bottom

Az.KeyVault - ImportAzureKeyVaultCertificate is using an older API version that might be causing cert mergingissues. #24323

Closed
FabianGonzalez-MSFT opened this issue Mar 8, 2024 · 26 comments
Closed

Az.KeyVault - ImportAzureKeyVaultCertificate is using an older API version that might be causing cert mergingissues. #24323

FabianGonzalez-MSFT opened this issue Mar 8, 2024 · 26 comments
Assignees
@nisha-bhatia
Labels
Azure PS Team bug This issue requires a change to an existing behavior in the product in order to be resolved. KeyVault Tracking We will track status and follow internally

Comments

@FabianGonzalez-MSFT
Copy link

FabianGonzalez-MSFT commented Mar 8, 2024 •
edited
Loading

Description

When the customer tries to upload a certificate to AKV via "ImportAzureKeyVaultCertificate", they get a 400 BadRequest error. (Not visible on cx's audit logs). However, when they try the same operation using Azure CLI and the GUI, the certificate merge operation succeeds with the exact same certificate.

This is the PSH error:

Body: { "error": { "code": "BadParameter", "message": "Unable to parse X5c certificate chain and locate leaf certificate" } }

We think it has to do with the API version on the request since GUI and CLI use 7.4 (latest) while PSH uses 7.0

PSH:

HTTP Method:
POST

Absolute Uri:
https://ContosoKV.vault.azure.net//certificates/ContosoCertificate/pending/merge?api-version=**7.0**

CLI:

"POST /certificates/ContosoCertificate/pending/merge?api-version=7.4

Issue script & Debug output

DEBUG: ============================ HTTP RESPONSE ============================

Status Code:
BadRequest

Headers:
Cache-Control                 : no-cache
Pragma                        : no-cache
x-ms-keyvault-region          : eastus2
x-ms-client-request-id        : 95290a34-21a5-4371-92e2-df0549801d72
x-ms-request-id               : 073b677c-a5dd-4606-9aaf-ed9c8db11464
x-ms-keyvault-service-version : 1.9.1300.1
x-ms-keyvault-network-info    : conn_type=Ipv4;addr=xx.xx.xx.xx;act_addr_fam=InterNetwork;
X-Content-Type-Options        : nosniff
Strict-Transport-Security     : max-age=31536000;includeSubDomains
Date                          : Fri, 01 Mar 2024 19:13:21 GMT

Body:
{
  "error": {
    "code": "BadParameter",
    "message": "Unable to parse X5c certificate chain and locate leaf certificate"
  }
}


DEBUG: 7:13:21 PM - [ConfigManager] Got nothing from [DisableErrorRecordsPersistence], Module = [], Cmdlet = []. Returning default value [False].
DEBUG: 7:13:21 PM - [ConfigManager] Got [True] from [EnableDataCollection], Module = [], Cmdlet = [].
Import-AzKeyVaultCertificate: Operation returned an invalid status code 'BadRequest'
Code: BadParameter
Message: Unable to parse X5c certificate chain and locate leaf certificate
DEBUG: 7:13:21 PM - [ConfigManager] Got nothing from [DisplayBreakingChangeWarning], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: 7:13:21 PM - [ConfigManager] Got nothing from [DisplayRegionIdentified], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: 7:13:21 PM - [ConfigManager] Got nothing from [CheckForUpgrade], Module = [], Cmdlet = []. Returning default value [True].
DEBUG: AzureQoSEvent:  Module: Az.KeyVault:5.2.0; CommandName: Import-AzKeyVaultCertificate; PSVersion: 7.4.1; IsSuccess: False; Duration: 00:00:08.8643611; Exception: Operation returned an invalid status code 'BadRequest'
Code: BadParameter
Message: Unable to parse X5c certificate chain and locate leaf certificate;
DEBUG: 7:13:21 PM - ImportAzureKeyVaultCertificate end processing.

Environment data

Name                           Value
----                           -----
PSVersion                      7.4.1
PSEdition                      Core
GitCommitId                    7.4.1
OS                             CBL-Mariner/Linux
Platform                       Unix
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Module versions

Latest.

Error output

Body:
{
  "error": {
    "code": "BadParameter",
    "message": "Unable to parse X5c certificate chain and locate leaf certificate"
  }
}
@FabianGonzalez-MSFT FabianGonzalez-MSFT added bug This issue requires a change to an existing behavior in the product in order to be resolved. needs-triage This is a new issue that needs to be triaged to the appropriate team. labels Mar 8, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Mar 8, 2024
@BethanyZhou BethanyZhou added the Tracking We will track status and follow internally label Mar 12, 2024
@VeroSegura
Copy link

Any updates on this issue Team?

1 similar comment
@VeroSegura
Copy link

Any updates on this issue Team?

@BethanyZhou BethanyZhou mentioned this issue Apr 17, 2024
Merged
6 tasks
@BethanyZhou
Copy link
Contributor

Please expect new release available on 4/30 (Az.KeyVault 5.3.0, Az 11.6.0).

@BethanyZhou
Copy link
Contributor

BethanyZhou commented Apr 24, 2024 •
edited
Loading

@FabianGonzalez-MSFT , @VeroSegura please let me know if the issue is fixed once new Az.KeyVault is available.

@VeroSegura
Copy link

VeroSegura commented Apr 24, 2024 via email

@VeroSegura
Copy link

VeroSegura commented May 8, 2024 via email

@BethanyZhou
Copy link
Contributor

@VeroSegura, could you ask customer to share debug log in new version?

@VeroSegura
Copy link

VeroSegura commented May 9, 2024 via email

@VeroSegura
Copy link

VeroSegura commented May 9, 2024 via email

@BethanyZhou
Copy link
Contributor

@VeroSegura , could you share the link of DTM? let me try.

@BethanyZhou
Copy link
Contributor

@heaths , could you help have a look? Seems the error is reported by SDK. This issue persists after I upgraded API version to 7.5 (using Azure.Security.KeyVault.Certificates 4.6.0).

@heaths
Copy link
Member

heaths commented May 22, 2024

This is a service error response:

{
  "error": {
    "code": "BadParameter",
    "message": "Unable to parse X5c certificate chain and locate leaf certificate"
  }
}

@jlichwa can someone from your team take a look?

@jlichwa
Copy link

jlichwa commented May 22, 2024 •
edited
Loading

@heaths considering that it is working with Azure CLI and Portal UX, it does not seem like service issue. Also I believe that could be some confusion between Import and Merge (it seems like customers is trying to merge CSR) - not sure if PSH has specific operation for merge over import. - also no changes in this area for years, so version should not be an issue.

Can we compare exact payload between CLI and PSH?

@heaths
Copy link
Member

heaths commented May 22, 2024 •
edited
Loading

The API version is different. The only other thing I can think of is that Az.KeyVault is handling the binary data for x5c wrong. There was a case they were using SecureString incorrectly. PowerShell team, is this still the case? As discussed offline, SecureString encoded data in a binary format and you can't just send it as-is. It's also not necessary. Input from the command line or from files should be passed as UTF-8 encoded byte[] array and the SDK will base64url encode it.

https://learn.microsoft.com/dotnet/api/azure.security.keyvault.certificates.mergecertificateoptions.-ctor

The byte[] array (or enumerable of arrays for multiple certificates in a chain) should not be pre-encoded.

The SDK method is tested and works. This appears to be from passing the wrong data to the SDK.

@jlichwa
Copy link

jlichwa commented May 22, 2024

@heaths I can confirm on service side that for this operation regardless if 7.5 or 7.0 used is using exactly same controller version, same method.

@BethanyZhou
Copy link
Contributor

BethanyZhou commented May 23, 2024 •
edited
Loading

Thank you for quick response. @jlichwa, @heaths

I agree the issue is not from service side as CLI and Portal work well. It is probably caused by passing the wrong data to the SDK.

PowerShell reads certificate bytes from file directly, which doesn't matter with secure string. But I'm not sure if it is correct way to construct IEnumerable<byte[]> x509Certificates. @heaths could you kindly help confirm below method?

           byte[] bytes = File.ReadAllBytes(FilePath);
           var certificates = new List<byte[]> { bytes };
           var options = new MergeCertificateOptions(certName, certificates);
           var certClient = CreateCertificateClient(vaultName);
           var options = new MergeCertificateOptions(certName, certificates);
           var cert = certClient.MergeCertificate(options);

@BethanyZhou
Copy link
Contributor

BethanyZhou commented May 23, 2024 •
edited
Loading

I'm trying to catch and compare exact payload between CLI and PSH. Customer has to pay for new certificates for reproducing purpose so that we can't catch the payload for the time being.

@heaths
Copy link
Member

heaths commented May 23, 2024

While that's almost the right base (those bytes from the file aren't base64-encoded, so that local variable name is misleading), I just reread the original issue and I'm confused. It says they are trying to import a certificate, but then talks about the merge endpoint. Which is it?

/cc @nisha-bhatia @pallavit

@BethanyZhou
Copy link
Contributor

BethanyZhou commented May 24, 2024 •
edited
Loading

Corrected the variable name.

Sorry for confusion. We are talking about the merge endpoint. PowerShell combines the merge and import operation in one command Import-AzKeyVaultCertificate. If the certificate owns a private key, we invoke import operation. If not, we invoke merge operation.

@heaths
Copy link
Member

heaths commented May 24, 2024

In what format is the public key? PEM? DER?

@BethanyZhou
Copy link
Contributor

it's in cer format.

@heaths
Copy link
Member

heaths commented May 28, 2024

Any reason to think a self-signed cert wouldn't work in reproducing this? /cc @nisha-bhatia

@nisha-bhatia nisha-bhatia self-assigned this May 30, 2024
@BethanyZhou
Copy link
Contributor

What's the correct way to pass cert with following format for merge operation via dotnet sdk? @nisha-bhatia , @heaths

-----BEGIN CERTIFICATE-----
cert_content1
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
cert_content2
-----END CERTIFICATE-----

Checked the source code of az cli, they filter cert_content1 and cert_content2 out as a list [cert_content1, cert_content2] and pass the list to python sdk.

@heaths
Copy link
Member

heaths commented Jun 3, 2024

You should always pass raw bytes to the SDK - don't encode anything yourself. That's what the SDK will do as needed. We don't base64-encode PEM files since the service doesn't want that, and if you base64-encode PKCS12 files we'll end up base64-encoding them again. Just pass the file as raw bytes. /cc @nisha-bhatia

@BethanyZhou BethanyZhou mentioned this issue Jun 21, 2024
Merged
6 tasks
@BethanyZhou
Copy link
Contributor

Hi @heaths, really appreciate for your help.

Let's say I have a cert to be merged, and I will handle them by two ways separately.

-----BEGIN CERTIFICATE-----
CertBase64stringA
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
CertBase64stringB
-----END CERTIFICATE-----

Way 1: Pass the raw bytes directly, which read from file directly by File.ReadAllBytes(FilePath). SDK will send the request body with unknown string and the service reports error.

Way 2: Read certificate file as text, extract the certificate data texts between the header and footer, and convert the texts to bytes by Convert.FromBase64String(text). SDK will the request body like below and the service works.

{"x5c":["CertBase64stringA", "CertBase64stringB"]}

I verified with our customer yesterday, way 2 works for him. Also, CLI is using way 2 as well. I'm not sure if it is your expectation.

@heaths
Copy link
Member

heaths commented Jun 21, 2024

The SDK expects raw bytes. It sends whatever you pass to the service. If the service doesn't support PEM, then you have to extract the sections. The SDKs are most often lightweight clients to the services that offer auth, tracing, etc. to the pipeline. Basically, the SDK supports whatever the service expects. If way 2 is what the service expects, please make appropriate changes to the Az.KeyVault module.

@nickwb nickwb mentioned this issue Aug 19, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nisha-bhatia nisha-bhatia

Labels
Azure PS Team bug This issue requires a change to an existing behavior in the product in order to be resolved. KeyVault Tracking We will track status and follow internally
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@heaths @isra-fel @BethanyZhou @jlichwa @nisha-bhatia @VeroSegura @FabianGonzalez-MSFT