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.

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

[BUG] Cannot import certificate to Keyvault with customized policy #11669

Closed
xiaoyang-connyun opened this issue Apr 29, 2020 · 12 comments
Closed
Assignees
Labels
blocking-release Blocks release bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. KeyVault needs-team-attention This issue needs attention from Azure service team or SDK team
Milestone

Comments

@xiaoyang-connyun
Copy link

Describe the bug
I am trying to import a certificate to Keyvault with a customized policy.
The certificate is self-signed with method CertificateRequest.CreateSelfSigned, and the key policy I need here is exportable = false.

Expected behavior
The certificate should be successfully imported and Keyvault should response 201.

Actual behavior (include Exception or Stack Trace)
The certificate was not imported successfully. Keyvault replies 400 with

{"error":{"code":"BadParameter","message":"Property policy has invalid value\r\n"}}

To Reproduce

// Prepare a certificate request.
            using var certificateKey = RSA.Create(2048);
            var certificateRequest = new CertificateRequest(<subject>, certificateKey, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
            // https://openvpn.net/community-resources/how-to/#setting-up-your-own-certificate-authority-ca-and-generating-certificates-and-keys-for-an-openvpn-server-and-multiple-clients
            certificateRequest.CertificateExtensions.Add(new X509BasicConstraintsExtension(true, true, 0, true));
            certificateRequest.CertificateExtensions.Add(new X509KeyUsageExtension(X509KeyUsageFlags.KeyCertSign | X509KeyUsageFlags.CrlSign, true));

            // Sign the certificate.
            using var certificate = certificateRequest.CreateSelfSigned(
                DateTimeOffset.UtcNow,
                DateTimeOffset.UtcNow.AddYears(1)
            );

            // Export the certificate.
            var pfx = certificate.Export(X509ContentType.Pfx);
            using (Stream file = File.OpenWrite(@"<path>"))
            {
                file.Write(pfx, 0, pfx.Length);
            }

            // Upload the certificate.
            var certificateOptions = new ImportCertificateOptions(<cert-name>, certificate.Export(X509ContentType.Pkcs12));
            certificateOptions.Policy = new CertificatePolicy(WellKnownIssuerNames.Self, <subject>);
            certificateOptions.Policy.KeySize = 2048;
            certificateOptions.Policy.KeyType = CertificateKeyType.Rsa;
            certificateOptions.Policy.Exportable = false;
            certificateOptions.Policy.ReuseKey = false;
            var certificateWithPolicy = _certificateClient.ImportCertificate(certificateOptions).Value;

Note that:

  1. Some values are masked with "<>" just to protect data that might be confidential.
  2. I export the same certificate as a file and import it using az cli command az keyvault certificate import --name --vault-name --file --policy with policy
{
    "issuerParameters": {
        "certificateTransparency": null,
        "certificateType": null,
        "name": "Self"
    },
    "keyProperties": {
        "curve": null,
        "exportable": false,
        "keySize": 2048,
        "keyType": "RSA",
        "reuseKey": false
    }
}

and it works as expected. So I suspect it is a bug, but maybe I did something wrong with .NET sdk.

Environment:

  • Azure.Security.KeyVault.Certificates 4.0.2
  • dotnet 3.1.102
  • az cli 2.1.0
@ghost ghost added needs-triage This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Apr 29, 2020
@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. KeyVault needs-team-attention This issue needs attention from Azure service team or SDK team labels Apr 29, 2020
@ghost ghost removed the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Apr 29, 2020
@heaths heaths added this to the [2020] June milestone Apr 29, 2020
@heaths heaths added bug This issue requires a change to an existing behavior in the product in order to be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Apr 30, 2020
@heaths
Copy link
Member

heaths commented Apr 30, 2020

Thank you for the repo. I believe I do see a problem. Is this blocking currently? We have an upcoming preview release to include other features.

@heaths
Copy link
Member

heaths commented Apr 30, 2020

I just wanted to double check that the sample code is exactly (barring masked inputs) what you're using:

certificateOptions.Policy = new CertificatePolicy(WellKnownIssuerNames.Self, <subject>);
certificateOptions.Policy.KeySize = 2048;
certificateOptions.Policy.KeyType = CertificateKeyType.Rsa;
certificateOptions.Policy.Exportable = false;
certificateOptions.Policy.ReuseKey = false;

I do see a problem where if only Exportable is set we don't serialize it, but if any other property is set we should, and as "exportable" (same as the CLI). I will repro this later and dig in further, but after a cursory glance I noticed this issue and wanted to double check.

@xiaoyang-connyun
Copy link
Author

Thank you for the repo. I believe I do see a problem. Is this blocking currently? We have an upcoming preview release to include other features.

It is blocking in a way that I cannot give the access to externals since they might accidentally export the private part. One naive workaround could be to secure it with a strong password and does not store such password anywhere so that nobody is able to export it.

@xiaoyang-connyun
Copy link
Author

xiaoyang-connyun commented Apr 30, 2020

I just wanted to double check that the sample code is exactly (barring masked inputs) what you're using:

certificateOptions.Policy = new CertificatePolicy(WellKnownIssuerNames.Self, <subject>);
certificateOptions.Policy.KeySize = 2048;
certificateOptions.Policy.KeyType = CertificateKeyType.Rsa;
certificateOptions.Policy.Exportable = false;
certificateOptions.Policy.ReuseKey = false;

I do see a problem where if only Exportable is set we don't serialize it, but if any other property is set we should, and as "exportable" (same as the CLI). I will repro this later and dig in further, but after a cursory glance I noticed this issue and wanted to double check.

Yes, the sample code is exactly what I am using right now.
You could also try to run the code only with

certificateOptions.Policy = new CertificatePolicy(WellKnownIssuerNames.Self, <subject>);

At least, I get the same failure with that.
So I am really curious about your result.

@heaths
Copy link
Member

heaths commented May 1, 2020

It is blocking in a way that I cannot give the access to externals since they might accidentally export the private part. One naive workaround could be to secure it with a strong password and does not store such password anywhere so that nobody is able to export it.

Do you need to create it via the SDK (e.g. need to repeat the process), or can you use the az CLI till we can get a fix into the upcoming preview?

@xiaoyang-connyun
Copy link
Author

Do you need to create it via the SDK (e.g. need to repeat the process), or can you use the az CLI till we can get a fix into the upcoming preview?

Currently, the target environment is Azure Function and the process is triggered on demand, so I need the SDK. For an intermediate solution, I can use az cli, this is acceptable as far as I do not need to wait the fix for too long. I see the milestone is set to June, I think this is fine to me.

@heaths heaths removed the needs-team-attention This issue needs attention from Azure service team or SDK team label May 20, 2020
@heaths heaths modified the milestones: [2020] June, [2020] July May 20, 2020
@heaths
Copy link
Member

heaths commented May 20, 2020

We're moving this to our July milestone, which represents the work we're doing during June and releasing in early July (typically the first week). Let me know if this is too late.

@xiaoyang-connyun
Copy link
Author

We're moving this to our July milestone, which represents the work we're doing during June and releasing in early July (typically the first week). Let me know if this is too late.

Thanks for the update.
Early July is acceptable to me.

@heaths heaths added the blocking-release Blocks release label Jun 15, 2020
@christothes christothes self-assigned this Jun 23, 2020
@christothes
Copy link
Member

@xiaoyang-connyun Are you able to reproduce the error if you add the following CertificatePolicy option?

certificateOptions.Policy.ContentType = CertificateContentType.Pkcs12;

In my testing the request succeeds with this added.

@xiaoyang-connyun
Copy link
Author

@xiaoyang-connyun Are you able to reproduce the error if you add the following CertificatePolicy option?

certificateOptions.Policy.ContentType = CertificateContentType.Pkcs12;

In my testing the request succeeds with this added.

No, I CANNOT reproduce the error if ContentType is defined, namely, the request succeeds.

So, what in the end determines the policy? Is it purely based on Policy, or some parts will be derived by the content of X509Certificate2? Also, if a value is missing, does it always have a default or not?

@ghost ghost added needs-team-attention This issue needs attention from Azure service team or SDK team and removed needs-author-feedback More information is needed from author to address the issue. labels Jun 26, 2020
@christothes
Copy link
Member

@xiaoyang-connyun thanks for confirming this works for you.

We are working with the service team to determine what the correct behavior for these options should be. The SDK was operating under the assumption that ContentType could be inferred based on the certificate content. However, the policy seems to expect a ContentType to always be set as it will not be inferred from the certificate content.

In addition, the July release of Azure.Security.KeyVault.Certificates will include a fix (linked to this issue) which prevented the ReuseKey and Exportable policy options from being serialized into the ImportCertificate request if specified in isolation.

I'm going to close this Issue since you now have a workable solution but we will loop back to link any changes to docs or samples addressing the behavior of certificate policy properties to this issue.

@xiaoyang-connyun
Copy link
Author

@xiaoyang-connyun thanks for confirming this works for you.

We are working with the service team to determine what the correct behavior for these options should be. The SDK was operating under the assumption that ContentType could be inferred based on the certificate content. However, the policy seems to expect a ContentType to always be set as it will not be inferred from the certificate content.

In addition, the July release of Azure.Security.KeyVault.Certificates will include a fix (linked to this issue) which prevented the ReuseKey and Exportable policy options from being serialized into the ImportCertificate request if specified in isolation.

I'm going to close this Issue since you now have a workable solution but we will loop back to link any changes to docs or samples addressing the behavior of certificate policy properties to this issue.

Thanks, I appreciate that.

openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-net that referenced this issue Nov 13, 2020
[T2] containerservice python automation fix (Azure#11669)

* python automation fix

* Update readme.python.md
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocking-release Blocks release bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. KeyVault needs-team-attention This issue needs attention from Azure service team or SDK team
Projects
None yet
Development

No branches or pull requests

4 participants