diff --git a/docs/BugReports.md b/docs/BugReports.md new file mode 100644 index 0000000..5fbcbdf --- /dev/null +++ b/docs/BugReports.md @@ -0,0 +1,209 @@ +# Bug Reports + +## `@azure/keyvault-certificates` — `CertificateClient.importCertificate` silently drops `policy.contentType` + +**Package:** `@azure/keyvault-certificates` +**Confirmed versions:** 4.10.3, current `main` branch +**Source file:** `sdk/keyvault/keyvault-certificates/src/index.ts` +**Serializer file:** `sdk/keyvault/keyvault-certificates/src/models/models.ts` + +--- + +### Summary + +`CertificateClient.importCertificate()` accepts `ImportCertificateOptions.policy.contentType` as part of its public API but never sends it to Azure. The value is silently dropped due to a key name mismatch between the high-level client and the generated REST serializer. As a result, Azure falls back to the existing stored policy for that certificate name. When that policy specifies a different format than the bytes being imported (e.g. importing PFX into a certificate previously stored as PEM), Azure rejects the request with: + +> `The specified PEM X.509 certificate content is in an unexpected format.` + +--- + +### Root Cause + +In `index.ts`, `importCertificate` builds the parameters object for the low-level generated client by spreading `updatedOptions`: + +```typescript +const result = await this.client.importCertificate( + certificateName, + { + base64EncodedCertificate, + preserveCertOrder: updatedOptions.preserveCertificateOrder, + ...updatedOptions, // contributes: policy, password, tags, ... + }, + updatedOptions, +); +``` + +The spread adds the property as `policy` (the public API name from `ImportCertificateOptions`). + +In `models/models.ts`, the generated serializer reads a **different** key name: + +```typescript +export function certificateImportParametersSerializer(item: CertificateImportParameters): any { + return { + value: item["base64EncodedCertificate"], + pwd: item["password"], + policy: !item["certificatePolicy"] // reads "certificatePolicy", not "policy" + ? item["certificatePolicy"] + : certificatePolicySerializer(item["certificatePolicy"]), + attributes: !item["certificateAttributes"] + ? item["certificateAttributes"] + : certificateAttributesSerializer(item["certificateAttributes"]), + tags: item["tags"], + preserveCertOrder: item["preserveCertOrder"], + }; +} +``` + +`item["certificatePolicy"]` is always `undefined` because the spread only populated `item["policy"]`. The REST body is sent with `policy: null`, regardless of what the caller specified. + +**Note:** `password` is not affected — it is read as `item["password"]` which matches the spread key and is correctly transmitted. + +**Note:** The Python SDK does not have this bug — it uses a different serialization architecture. + +--- + +### Effect + +`ImportCertificateOptions.policy` is effectively a no-op. Any value passed is ignored. The observable consequence: + +- Importing a certificate for the first time works, because Azure auto-detects the format from the bytes (PEM headers are recognisable; PFX ASN.1 magic bytes may also be detected). +- Importing a **new version** of an existing certificate in a **different format** fails: Azure validates the incoming bytes against the stored policy's `content_type`, which no longer matches. + +--- + +### Reproduction + +```typescript +import { DefaultAzureCredential } from "@azure/identity"; +import { CertificateClient } from "@azure/keyvault-certificates"; +import { readFileSync } from "fs"; + +const credential = new DefaultAzureCredential(); +const client = new CertificateClient( + "https://.vault.azure.net", + credential, +); + +// Step 1: import a PEM certificate (works — Azure auto-detects PEM) +const pemBytes = Buffer.from(readFileSync("cert.pem", "utf8")); +await client.importCertificate("MyCert", pemBytes, { + policy: { contentType: "application/x-pem-file" }, +}); + +// Step 2: import the same certificate as PFX (fails — policy.contentType is dropped, +// Azure uses existing PEM policy and rejects the binary PFX bytes) +const pfxBytes = readFileSync("cert.pfx"); +await client.importCertificate("MyCert", pfxBytes, { + password: "pfx-password", + policy: { contentType: "application/x-pkcs12" }, +}); +// ^ throws: "The specified PEM X.509 certificate content is in an unexpected format." +``` + +--- + +### Fix + +In `sdk/keyvault/keyvault-certificates/src/index.ts`, explicitly map the public `policy` field to the internal `certificatePolicy` key after the spread: + +```typescript +// BEFORE +const result = await this.client.importCertificate( + certificateName, + { + base64EncodedCertificate, + preserveCertOrder: updatedOptions.preserveCertificateOrder, + ...updatedOptions, + }, + updatedOptions, +); + +// AFTER +const result = await this.client.importCertificate( + certificateName, + { + base64EncodedCertificate, + preserveCertOrder: updatedOptions.preserveCertificateOrder, + ...updatedOptions, + certificatePolicy: updatedOptions.policy, // map public name → serializer key + }, + updatedOptions, +); +``` + +No changes required to the serializer, the public `ImportCertificateOptions` interface, or any other file. + +--- + +### Test + +Place in `sdk/keyvault/keyvault-certificates/test/` alongside the existing import tests. The test intercepts the outgoing HTTP request and asserts that `policy.secret_props.content_type` is present in the body. + +```typescript +import { assert } from "@azure/test-utils"; +import { CertificateClient } from "../../src/index.js"; +import { createTestCredential } from "@azure-tools/test-credential"; +import { HttpClient, PipelineRequest, PipelineResponse } from "@azure/core-rest-pipeline"; + +function makeFakeResponse(): PipelineResponse { + return { + status: 200, + headers: { get: () => "application/json", set: () => {}, has: () => true, delete: () => {}, toJSON: () => ({}) } as any, + bodyAsText: JSON.stringify({ + id: "https://vault.azure.net/certificates/MyCert/abc123", + cer: Buffer.alloc(0).toString("base64"), + attributes: { enabled: true }, + policy: { + secret_props: { contentType: "application/x-pkcs12" }, + issuer: { name: "Unknown" }, + }, + }), + request: null as any, + }; +} + +it("importCertificate sends policy.contentType in the REST body", async () => { + let capturedBody: any; + + const fakeHttpClient: HttpClient = { + sendRequest: async (request: PipelineRequest): Promise => { + capturedBody = JSON.parse(request.body as string); + return makeFakeResponse(); + }, + }; + + const client = new CertificateClient( + "https://fakevault.vault.azure.net", + createTestCredential(), + { httpClient: fakeHttpClient }, + ); + + const pfxBytes = Buffer.from("fakepfxbytes"); + + await client.importCertificate("MyCert", pfxBytes, { + password: "secret", + policy: { contentType: "application/x-pkcs12" }, + }); + + assert.isDefined( + capturedBody?.policy?.secret_props?.contentType, + "policy.secret_props.contentType must be present in the REST body", + ); + assert.strictEqual( + capturedBody.policy.secret_props.contentType, + "application/x-pkcs12", + "contentType must match the value passed in ImportCertificateOptions.policy", + ); +}); +``` + +--- + +### Contribution Steps + +1. Search [azure-sdk-for-js issues](https://github.com/Azure/azure-sdk-for-js/issues) for `importCertificate policy contentType` — file a new issue if none exists. +2. Fork `Azure/azure-sdk-for-js` on GitHub. +3. Clone your fork locally. +4. Apply the one-line fix in `sdk/keyvault/keyvault-certificates/src/index.ts`. +5. Add the test above to the existing import test suite. +6. Open a PR referencing the issue, with this document as the description basis. diff --git a/docs/bug-reproduce-openssl.mjs b/docs/bug-reproduce-openssl.mjs new file mode 100644 index 0000000..e84ba7a --- /dev/null +++ b/docs/bug-reproduce-openssl.mjs @@ -0,0 +1,70 @@ +// Reproduces the bug in @azure/keyvault-certificates: +// CertificateClient.importCertificate() silently drops policy.contentType, +// so Azure validates incoming bytes against the existing stored policy. +// Importing PFX into a certificate previously stored as PEM fails with: +// "The specified PEM X.509 certificate content is in an unexpected format." +// +// Requirements: openssl in PATH +// Usage: KEYVAULT_NAME= node docs/bug-reproduce.mjs + +import { execSync } from 'node:child_process'; +import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { DefaultAzureCredential } from '@azure/identity'; +import { CertificateClient } from '@azure/keyvault-certificates'; + +const vaultName = process.env.KEYVAULT_NAME; +if (!vaultName) { + console.error('Set KEYVAULT_NAME'); + process.exit(1); +} +const vaultUrl = `https://${vaultName}.vault.azure.net`; + +const CERT_NAME = 'bug-reproduce-test'; +const PFX_PASSWORD = 'test-password-123'; + +const tmp = mkdtempSync(join(tmpdir(), 'kv-bug-')); +try { + console.log('Generating self-signed certificate...'); + execSync( + `openssl req -x509 -newkey rsa:2048 -keyout "${join(tmp, 'key.pem')}" -out "${join(tmp, 'cert.pem')}" -days 365 -nodes -subj "/CN=bug-reproduce-test"`, + { stdio: 'pipe' }, + ); + execSync( + `openssl pkcs12 -export -out "${join(tmp, 'cert.pfx')}" -inkey "${join(tmp, 'key.pem')}" -in "${join(tmp, 'cert.pem')}" -passout pass:${PFX_PASSWORD}`, + { stdio: 'pipe' }, + ); + + const pemBytes = Buffer.concat([readFileSync(join(tmp, 'cert.pem')), readFileSync(join(tmp, 'key.pem'))]); + const pfxBytes = readFileSync(join(tmp, 'cert.pfx')); + + const credential = new DefaultAzureCredential(); + const client = new CertificateClient(vaultUrl, credential); + + console.log(`\nStep 1: importing '${CERT_NAME}' as PEM...`); + try { + await client.importCertificate(CERT_NAME, pemBytes, { + policy: { contentType: 'application/x-pem-file', issuerName: 'Unknown', subject: 'CN=bug-reproduce-test' }, + }); + console.log(' OK — PEM import succeeded.'); + } catch (err) { + console.error(` FAILED — ${err.message}`); + process.exit(1); + } + + console.log(`\nStep 2: re-importing '${CERT_NAME}' as PFX (policy.contentType should tell Azure to expect PFX)...`); + try { + await client.importCertificate(CERT_NAME, pfxBytes, { + password: PFX_PASSWORD, + policy: { contentType: 'application/x-pkcs12', issuerName: 'Unknown', subject: 'CN=bug-reproduce-test' }, + }); + console.log(' OK — PFX import succeeded (bug may be fixed in this SDK version).'); + } catch (err) { + console.error(` FAILED — ${err.message}`); + console.error('\n This confirms the bug: policy.contentType was silently dropped.'); + console.error(' Azure validated the PFX bytes against the existing PEM policy and rejected them.'); + } +} finally { + rmSync(tmp, { recursive: true }); +} diff --git a/docs/bug-workaround-openssl.mjs b/docs/bug-workaround-openssl.mjs new file mode 100644 index 0000000..ad6af08 --- /dev/null +++ b/docs/bug-workaround-openssl.mjs @@ -0,0 +1,72 @@ +// Demonstrates the workaround for the bug in @azure/keyvault-certificates: +// Since importCertificate() silently drops policy.contentType, we call +// updateCertificatePolicy() first to shift the stored content_type to the +// new format before importing. Azure then validates incoming bytes correctly. +// +// Requirements: openssl in PATH +// Usage: KEYVAULT_NAME= node docs/bug-workaround.mjs + +import { execSync } from 'node:child_process'; +import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { DefaultAzureCredential } from '@azure/identity'; +import { CertificateClient } from '@azure/keyvault-certificates'; + +const vaultName = process.env.KEYVAULT_NAME; +if (!vaultName) { + console.error('Set KEYVAULT_NAME'); + process.exit(1); +} +const vaultUrl = `https://${vaultName}.vault.azure.net`; + +const CERT_NAME = 'bug-workaround-test'; +const PFX_PASSWORD = 'test-password-123'; + +const tmp = mkdtempSync(join(tmpdir(), 'kv-workaround-')); +try { + console.log('Generating self-signed certificate...'); + execSync( + `openssl req -x509 -newkey rsa:2048 -keyout "${join(tmp, 'key.pem')}" -out "${join(tmp, 'cert.pem')}" -days 365 -nodes -subj "/CN=bug-workaround-test"`, + { stdio: 'pipe' }, + ); + execSync( + `openssl pkcs12 -export -out "${join(tmp, 'cert.pfx')}" -inkey "${join(tmp, 'key.pem')}" -in "${join(tmp, 'cert.pem')}" -passout pass:${PFX_PASSWORD}`, + { stdio: 'pipe' }, + ); + + const pemBytes = Buffer.concat([readFileSync(join(tmp, 'cert.pem')), readFileSync(join(tmp, 'key.pem'))]); + const pfxBytes = readFileSync(join(tmp, 'cert.pfx')); + + const credential = new DefaultAzureCredential(); + const client = new CertificateClient(vaultUrl, credential); + + console.log(`\nStep 1: importing '${CERT_NAME}' as PEM...`); + try { + await client.importCertificate(CERT_NAME, pemBytes, { + policy: { contentType: 'application/x-pem-file', issuerName: 'Unknown', subject: 'CN=bug-workaround-test' }, + }); + console.log(' OK — PEM import succeeded.'); + } catch (err) { + console.error(` FAILED — ${err.message}`); + process.exit(1); + } + + console.log(`\nStep 2: updating policy to application/x-pkcs12 before importing PFX...`); + await client.updateCertificatePolicy(CERT_NAME, { + contentType: 'application/x-pkcs12', + issuerName: 'Unknown', + subject: 'CN=bug-workaround-test', + }); + console.log(' OK — policy updated.'); + + console.log(`\nStep 3: importing '${CERT_NAME}' as PFX...`); + await client.importCertificate(CERT_NAME, pfxBytes, { + password: PFX_PASSWORD, + }); + console.log(' OK — PFX import succeeded.'); + + console.log('\nWorkaround confirmed: updateCertificatePolicy() before import allows format change.'); +} finally { + rmSync(tmp, { recursive: true }); +}