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

GetPropertiesOfCertificates needs to be resilient and not fail with JSON error whenever the x5t property is missing from the response #6237

Open
ahsonkhan opened this issue Nov 18, 2024 · 0 comments
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. KeyVault

Comments

@ahsonkhan
Copy link
Member

We cannot always assume that an x5t property will be present within the JSON response:

// x5t
properties.X509Thumbprint = Base64Url::Base64UrlDecode(jsonResponse[X5tName].get<std::string>());

This needs to be within a check:

    if (certificate.contains(X5tName))
    {
      properties.X509Thumbprint
          = Base64Url::Base64UrlDecode(certificate[X5tName].get<std::string>());
    }

// const operator[] only works for objects
if (_azure_JSON_HEDLEY_LIKELY(is_object()))
{
auto it = m_data.m_value.object->find(key);
_azure_JSON_ASSERT(it != m_data.m_value.object->end());
return it->second;
}

Otherwise, the JSON library's [] operator behavior is undefined (and in debug the test/app crashes with an assertion failure and abort).

C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.41.34120\include\xtree(182) : Assertion failed: cannot dereference end map/set iterator

Steps to repo:
Right after a certificate is first created (and it is still pending), if the customer calls GetPropertiesOfCertificates with options.IncludePending = true;, the application could fail due to the x5t property missing.

      auto keyVaultUrl = "https://<keyvault-name>.vault.azure.net/";
      auto cred = std::make_shared<AzureCliCredential>();
      CertificateClient client(keyVaultUrl, cred);

      // Create a certificate
      std::cout << "Creating certificate atest-cert99" << std::endl;
      std::string certificateName = "atest-cert99";

      CertificateCreateOptions options;
      // create a lifetime action
      LifetimeAction action;
      action.LifetimePercentage = 80;
      action.Action = CertificatePolicyAction::AutoRenew;

      options.Properties.Enabled = true;
      options.Policy.Subject = "CN=sample1";
      options.Policy.ValidityInMonths = 12;
      options.Policy.Enabled = true;
      options.Policy.ContentType = CertificateContentType::Pkcs12;
      options.Policy.IssuerName = "Self";
      options.Policy.LifetimeActions.emplace_back(action);

      options.Properties.Name = certificateName;
      client.StartCreateCertificate(certificateName, options);

      // Fetch list of certificates
      // Observe that this will fail
      GetPropertiesOfCertificatesOptions options;
      options.IncludePending = true;
      std::cout << "Certificates in the key vault (includePending = true):" << std::endl;
      for (auto cert = client.GetPropertiesOfCertificates(options); cert.HasPage();
           cert.MoveToNextPage())
      {
        std::cout << "Found " << cert.Items.size() << " certificates." << std::endl;
        for (auto item : cert.Items)
        {
          std::cout <<  item.Name << std::endl;
        }
      }
@ahsonkhan ahsonkhan added bug This issue requires a change to an existing behavior in the product in order to be resolved. KeyVault labels Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. KeyVault
Projects
Status: Untriaged
Development

No branches or pull requests

1 participant