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

Updated certificate store name for intermediate CA #216

Closed
wants to merge 1 commit into from
Closed

Updated certificate store name for intermediate CA #216

wants to merge 1 commit into from

Conversation

quatauta
Copy link

@quatauta quatauta commented Apr 30, 2021

SUMMARY

Module ansible.windows.win_certificate_store documentation stated Windows certificate store name (store_name: "CertificateAuthority") for intermediate certificate authorities.

Importing a certificate to store name CertificateAuthority raises error message unable to find store ''CertificateAuthority'': (CertOpenStore failed (The system cannot find the file specified, Win32ErrorCode 2 - 0x00000002)).

Using store name CA allowed me to import a certificate to certificate store for intermediate certificate authorities.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

Module ansible.windows.win_certificate_store

ADDITIONAL INFORMATION

store_name documentation stated Windows certificate store name "CertificateAuthority" for intermediate certificate authorities. Importing a certificate to store name "CertificateAuthority" raises error message 'unable to find store ''CertificateAuthority'': (CertOpenStore failed (The system cannot find the file specified, Win32ErrorCode 2 - 0x00000002))'

Using store name "CA" allowed me to import a certificate to certificate store for intermediate certificate authorities.
@jborean93
Copy link
Collaborator

I'll have to test this tomorrow as previously we just used the values from StoreName to denote what store to open and that uses CertificateAuthority. Potentially the changes I've made in the refactor recently has broken this and we need to create a fix to allow both names for backwards compatibility.

Also just as an FYI documentation changes should be made int the relevant <module>.py file https://github.com/ansible-collections/ansible.windows/blob/main/plugins/modules/win_certificate_store.py instead of the rst docs. The rst files in docs are automatically generated from the module documentation around each release.

@jborean93
Copy link
Collaborator

Yep that seems to be the case, #192 changed the way the module opened the certificate store away from one done by .NET to one where it calls the necessary Win32 APIs. We can see in the .NET reference code it manually maps the StoreName enum values to a string where CertificateAuthority becomes CA as you've noticed https://github.com/microsoft/referencesource/blob/5697c29004a34d80acdaf5742d7e699022c64ecd/System/security/system/security/cryptography/x509/x509store.cs#L67-L91.

I'll update the module so that the old name will continue to work for backwards compatibility. Thanks for picking this up!

@jborean93
Copy link
Collaborator

I've opened a PR to bring back the old behaviour #218, just to note CA will still continue to work going forward. The PR just makes sure that store_name: CertificateAuthority is translated to CA internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants