Azure / azure-powershell

Microsoft Azure PowerShell
Other
4.21k stars 3.81k forks source link

Az 12.1.0 - Az.KeyVault - Import-AzKeyVaultCertificate - Import of .p7b files has been broken #25843

Open nickwb opened 3 weeks ago

nickwb commented 3 weeks ago

Description

Hi @BethanyZhou,

There appears to be a regression in relation to this PR: https://github.com/Azure/azure-powershell/pull/25333, and this issue: https://github.com/Azure/azure-powershell/issues/24323

Specifically, the Az.KeyVault module previously supported certificate import, via Import-AzKeyVaultCertificate, of a PKCS7 .p7b file.

However, the new code in ImportAzureKeyVaultCertificate.GetEnumerableBytes seems to assume PEM format, as it is expecting the -----BEGIN CERTIFICATE----- and -----END CERTIFICATE-----, which are not present in PKCS7. Instead, -----BEGIN PKCS7----- and -----END PKCS7----- are expected.

Issue script & Debug output

Import-AzKeyVaultCertificate -VaultName somevault -Name somecert -FilePath somefile.p7b

Environment data

Name                           Value
----                           -----
PSVersion                      7.4.4
PSEdition                      Core
GitCommitId                    7.4.4
OS                             Ubuntu 22.04.4 LTS
Platform                       Unix
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Module versions

Using the Github Azure/powershell@v1 action:

{
  "Success": "true",
  "AzVersion": "12.1.0"
}

Error output

Import-AzKeyVaultCertificate: /path/to.ps1:136
Line |
 136 |      Import-AzKeyVaultCertificate -VaultName $Vault -Name $($PSKeyVaul …
     |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | X5C must have at least one valid item  Status: 400 (Bad Request)
     | ErrorCode: BadParameter  Content:
     | {"error":{"code":"BadParameter","message":"X5C must have at least one
     | valid item\r\n"}}  Headers: Cache-Control: no-cache Pragma: no-cache
     | x-ms-keyvault-region: australiaeast x-ms-client-request-id:
     | 19237a88-00ac-485a-b9ed-0c1a483c487f x-ms-request-id:
     | 42fa87c0-9848-4d06-8209-60a0fa130f38 x-ms-keyvault-service-version:
     | 1.9.1652.1 x-ms-keyvault-network-info:
     | conn_type=Ipv4;addr=172.177.75.81;act_addr_fam=InterNetwork;
     | X-Content-Type-Options: REDACTED Strict-Transport-Security: REDACTED
     | Date: Mon, 19 Aug 2024 01:28:45 GMT Content-Length: 87 Content-Type:
     | application/json; charset=utf-8 Expires: -1
BethanyZhou commented 3 weeks ago

Hi @nickwb, thanks for reporting this. Tracking this issue now.

BethanyZhou commented 3 weeks ago

Hi @nickwb,

nickwb commented 3 weeks ago

Hi @BethanyZhou,

No, we are using public keys/certs only. No private key material.

I believe it is considered a merge, because the private key is already in KeyVault, but the signed certificates are not.

Our process is:

  1. Use KeyVault to generate the key-pair
  2. Use KeyVault to generate a certificate signing request
  3. Sign the certificate using ACME protocol (letsencrypt)
  4. Merge signed certificate back to KeyVault, including its full trust chain

We started using .p7b because Import-AzKeyVaultCertificate only supported single certificates when importing from PEM, rather than the entire trust chain. When we hit this issue originally, Microsoft support advised us to use .p7b.

As an aside, I am interested if it is now possible to import multiple certificates as PEM in a single file?

I have a test certificate .p7b you can use that has been signed using the process. Is there a way I can send it to you without posting it here publicly? I don't want to broadcast which domains I'm in control of.

BethanyZhou commented 3 weeks ago

Hi @nickwb, Import-AzKeyVaultCertificate has only supported to merge multiple certificates as PEM in single file now. That's why we raised the PR you mentioned in the description section.

Please try to use PEM file to work around this issue. And let me know if this way works for you. We need to discuss with Key Vault team to decide if we need to support p7b file for merge operation.

Notice that do not use old Az.KeyVault to work around this issue because the content sent to service is read in wrong way. The certificate should not work even no error is thrown per my understanding.

nickwb commented 3 weeks ago

Hi @BethanyZhou - yes, we will try with PEM, I will let you know how it goes.

I will note that .p7b is still working correctly in Az 12.0.0 - We have temporarily pinned our existing automation to this version, and it is working currently.