search

Azure / azure-powershell

Microsoft Azure PowerShell
Other
4.26k stars 3.86k forks source link

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

Closed FabianGonzalez-MSFT closed 4 months ago

FabianGonzalez-MSFT commented 8 months ago

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"
  }
}
VeroSegura commented 7 months ago

Any updates on this issue Team?

VeroSegura commented 7 months ago

Any updates on this issue Team?

BethanyZhou commented 6 months ago

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

BethanyZhou commented 6 months ago

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

VeroSegura commented 6 months ago

We will for sure.

Thank you!

Veronica Segura Carranza Partner Technical Advisor SCIM Office: +506 (4) 1161166 @.**@.> Work Hours Mon - Fri 7 AM - 4 PM MST @.**@.

From: Beisi Zhou @.> Sent: Tuesday, April 23, 2024 7:35 PM To: Azure/azure-powershell @.> Cc: Veronica Segura @.>; Mention @.> Subject: Re: [Azure/azure-powershell] Az.KeyVault - ImportAzureKeyVaultCertificate is using an older API version that might be causing cert mergingissues. (Issue #24323)

@FabianGonzalez-MSFThttps://github.com/FabianGonzalez-MSFT , @VeroSegurahttps://github.com/VeroSegura please let me know if the issue is fixed once new Az.KeyVault is available.

- Reply to this email directly, view it on GitHubhttps://github.com/Azure/azure-powershell/issues/24323#issuecomment-2073824718, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ATZQTTKMN7JZGDXQI6RU4NDY64D5PAVCNFSM6AAAAABEL6J7O2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZTHAZDINZRHA. You are receiving this because you were mentioned.Message ID: @.**@.>>

VeroSegura commented 6 months ago

Hello team,

Customer tested and informed the issue persists. Can you take a look on this?

Regards, Veronica Segura Carranza Partner Technical Advisor SCIM Office: +506 (4) 1161166 @.**@.> Work Hours Mon - Fri 7 AM - 4 PM MST @.**@.

From: Veronica Segura Sent: Wednesday, April 24, 2024 10:31 AM To: Azure/azure-powershell @.>; Azure/azure-powershell @.> Cc: Mention @.***> Subject: RE: [Azure/azure-powershell] Az.KeyVault - ImportAzureKeyVaultCertificate is using an older API version that might be causing cert mergingissues. (Issue #24323)

We will for sure.

Thank you!

Veronica Segura Carranza Partner Technical Advisor SCIM Office: +506 (4) 1161166 @.**@.> Work Hours Mon - Fri 7 AM - 4 PM MST @.**@.

From: Beisi Zhou @.**@.>> Sent: Tuesday, April 23, 2024 7:35 PM To: Azure/azure-powershell @.**@.>> Cc: Veronica Segura @.**@.>>; Mention @.**@.>> Subject: Re: [Azure/azure-powershell] Az.KeyVault - ImportAzureKeyVaultCertificate is using an older API version that might be causing cert mergingissues. (Issue #24323)

@FabianGonzalez-MSFThttps://github.com/FabianGonzalez-MSFT , @VeroSegurahttps://github.com/VeroSegura please let me know if the issue is fixed once new Az.KeyVault is available.

- Reply to this email directly, view it on GitHubhttps://github.com/Azure/azure-powershell/issues/24323#issuecomment-2073824718, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ATZQTTKMN7JZGDXQI6RU4NDY64D5PAVCNFSM6AAAAABEL6J7O2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZTHAZDINZRHA. You are receiving this because you were mentioned.Message ID: @.**@.>>

BethanyZhou commented 6 months ago

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

VeroSegura commented 6 months ago

Just for confirmation, you just need -verbose -debug output from customer right? I will ask SE to get this information from customer as soon as possible.

Regards, Veronica Segura Carranza Partner Technical Advisor SCIM Office: +506 (4) 1161166 @.**@.> Work Hours Mon - Fri 7 AM - 4 PM MST @.**@.

From: Beisi Zhou @.> Sent: Wednesday, May 8, 2024 10:27 PM To: Azure/azure-powershell @.> Cc: Veronica Segura @.>; Mention @.> Subject: Re: [Azure/azure-powershell] Az.KeyVault - ImportAzureKeyVaultCertificate is using an older API version that might be causing cert mergingissues. (Issue #24323)

@VeroSegurahttps://github.com/VeroSegura, could you ask customer to share debug log in new version?

- Reply to this email directly, view it on GitHubhttps://github.com/Azure/azure-powershell/issues/24323#issuecomment-2101910893, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ATZQTTMJXL64JICPVRJS543ZBL3IPAVCNFSM6AAAAABEL6J7O2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBRHEYTAOBZGM. You are receiving this because you were mentioned.Message ID: @.**@.>>

VeroSegura commented 6 months ago

Customer has shared the below screenshot and debug output is uploaded in the case DTM workspace. Do you have access to the case DTM? If not, please let me know how to share the logs with you.

[A blue screen with white text Description automatically generated]

Regards, Veronica

From: Veronica Segura Sent: Thursday, May 9, 2024 8:17 AM To: Azure/azure-powershell @.>; Azure/azure-powershell @.> Cc: Mention @.***> Subject: RE: [Azure/azure-powershell] Az.KeyVault - ImportAzureKeyVaultCertificate is using an older API version that might be causing cert mergingissues. (Issue #24323)

Just for confirmation, you just need -verbose -debug output from customer right? I will ask SE to get this information from customer as soon as possible.

Regards, Veronica Segura Carranza Partner Technical Advisor SCIM Office: +506 (4) 1161166 @.**@.> Work Hours Mon - Fri 7 AM - 4 PM MST @.**@.

From: Beisi Zhou @.**@.>> Sent: Wednesday, May 8, 2024 10:27 PM To: Azure/azure-powershell @.**@.>> Cc: Veronica Segura @.**@.>>; Mention @.**@.>> Subject: Re: [Azure/azure-powershell] Az.KeyVault - ImportAzureKeyVaultCertificate is using an older API version that might be causing cert mergingissues. (Issue #24323)

@VeroSegurahttps://github.com/VeroSegura, could you ask customer to share debug log in new version?

- Reply to this email directly, view it on GitHubhttps://github.com/Azure/azure-powershell/issues/24323#issuecomment-2101910893, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ATZQTTMJXL64JICPVRJS543ZBL3IPAVCNFSM6AAAAABEL6J7O2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBRHEYTAOBZGM. You are receiving this because you were mentioned.Message ID: @.**@.>>

BethanyZhou commented 6 months ago

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

BethanyZhou commented 6 months ago

@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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

BethanyZhou commented 5 months ago

it's in cer format.

heaths commented 5 months ago

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

BethanyZhou commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 4 months ago

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.