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

[New Resource] xCertificateExport #41

Closed
PlagueHO opened this issue Jan 7, 2017 · 14 comments
Closed

[New Resource] xCertificateExport #41

PlagueHO opened this issue Jan 7, 2017 · 14 comments
Assignees
Labels
resource proposal The issue is proposing a new resource in the resource module.

Comments

@PlagueHO
Copy link
Member

PlagueHO commented Jan 7, 2017

User Story: Need to be able to export certificates that are either already in the Windows Certificate Machine Store or have been requested via xCertReq.

This would require a certificate to be found in the machine store (via Thumbprint, Subject, Serial Number, KU, Issuer etc) and then exported to a file (if the file does not exist) as an x509 CER file.

This will be combined with the xPFXExport resource requested in #26

This will export x509 certificates or PKCS#12 certificates (with Private Key and optional trust chain).

@PlagueHO PlagueHO added help wanted The issue is up for grabs for anyone in the community. resource proposal The issue is proposing a new resource in the resource module. labels Jan 7, 2017
@johlju
Copy link
Member

johlju commented Jan 7, 2017

This would be a great addition. 👍

Could we also make these happen?

  • Can store the exported certificate to UNC path (when PsDscRunAsCredential is set to a username that has permission the the UNC path). I guess this would work just out-of-the-box.
  • That is has a parameter for overwrite an existing certificate file. When the server can auto-renew certificates on there own, it could lead to that we need to have the new certificate exporter, and need to overwrite the old one.
  • That is supports getting certificates from the user store if it is run with PsDscRunAsCredential. _This would be great if the xPFXExport also supported that.

Another thought. I might be wrong in this.
PFX is an old term, the right term should be PKCS #12 (Pkcs12), so the resource xPFXExport I think should be named differently. But I rather combine PFXExport with this one and have a parameter telling this resource to export the private key as well, and also need to provide the password when that option is used. Then it creates PKCS #12.
The user can choose, in the destination path, what the file extension should be ('.p12' or '.pfx') .
Would that work?

@PlagueHO
Copy link
Member Author

PlagueHO commented Jan 8, 2017

Hi @johlju

  1. Yep - sounds like a great idea!
  2. Another good idea - what we'd need to do is perform a hash compare on the certificate that is saved to disk and see if it requires to be rewritten. This would be similar in behavior to the MatchSource in xRemoteFile.

However, this might be more difficult with .PFX/.P12 files because they can contain more than one certificate (e.g. the entire certificate chain) and getting a hash on the certificate might be more difficult. It is easy with x509 certs but because the SHA-1 of the cert file is the thumbprint and therefore will be a trivial comparison. But the PKCS12 format would be different. I think this needs more investigation.

  1. Good thinking. This should be doable!

The biggest question on my mind is:
Will there be any difficulty combining this resource with the xExportPFX resource ( #26 ) into a single xExportCertificate resource that can export either .CER (x509) or .PFX/.P12 (PKCS #12) files?

I can't think of any unsolvable problems off the top of my head.

I reckon what we should start with is a proposed MOF file that covers x509 and PKCS12 exports and see if any problems jump out at us.

Does that sound like a good idea? I could probably put something together over the next few days?

I think the resource was named PFX to match the Export-PFXCertificate cmdlet that would be being used to export the PFX file - rather than the extension that would be used.

@johlju
Copy link
Member

johlju commented Jan 9, 2017

  1. For the .pfx/p12 files, maybe there should be an option to export the chain or just the certificate? I think you only need to verify the certificate ("export" it from the chain). If that has changed, you need to export it again. But if something in the chain has changed, then most likely the certificate has changed as well, and thus will export the entire chain again (because the option mentioned previously was set to $true).
    Would that work?

I can't see that it shoudl be any problem combining them. I thought it over when I wrote previously, I haven't since thought of any issues. Basically it is the same export (user chooses to export A or B).

A proposed MOF file would be a good start, sounds like a good idea. Are you on that?

@PlagueHO
Copy link
Member Author

@johlju
2. Good thinking - that makes sense.

I did a bit of digging and we can read the content of the PFX file reasonably easily with the following code:

$PFX = New-Object -TypeName 'System.Security.Cryptography.X509Certificates.X509Certificate2Collection'
$PFX.Import('cert.pfx','pass',[System.Security.Cryptography.X509Certificates.X509KeyStorageFlags]::PersistKeySet)
foreach($Cert in $PFX) { $Cert }

So this would allow us to compare the content of the current PFX with the certificate chain.

But yep I'm happy to put the MOF together.

@PlagueHO PlagueHO added in progress The issue is being actively worked on by someone. and removed help wanted The issue is up for grabs for anyone in the community. labels Jan 14, 2017
@PlagueHO
Copy link
Member Author

Hi @johlju - here is the proposed MOF:

[ClassVersion("1.0"), FriendlyName("xCertificateExport")]
class MSFT_xCertificateExport : OMI_BaseResource
{
    [Key,Description("The path to the file you that will contain the exported certificate.")] string Path;
    [Write,Description("Specifies whether the exported certificate should be present or absent."),ValueMap{"Present", "Absent"},Values{"Present", "Absent"}] string Ensure;
    [Write,Description("The thumbprint of the certificate to export.")] string Thumbprint;
    [Write,Description("The friendly name of the certificate to export.")] string FriendlyName;
    [Write,Description("The subject of the certificate to export.")] string Subject;
    [Write,Description("The issuer of the certiicate to export.")] string Issuer;
    [Write,Description("The key usage of the certificate to export must contain these values.")] string KeyUsage[];
    [Write,Description("The enhanced key usage of the certificate to export must contain these values.")] string EnhancedKeyUsage[];
    [Write,Description("The Windows Certificate Store Name to search for the certificate to export from.")] string Store;
    [Write,Description("Allow an expired certificate to be exported.")] boolean AllowExpired;
    [Write,Description("Causes an existing exported certificate to be compared with the certificate identified for export and re-exported if it does not match.")] boolean MatchSource;
    [Write,Description("Specifies the type of certificate to export."),ValueMap{"Cert", "P7B", "SST", "PFX"},Values{"Cert", "P7B", "SST", "PFX"}] string Type;
    [Write,Description("Specifies the options for building a chain when exporting a PFX certificate."),ValueMap{"BuildChain","EndEntityCertOnly"},Values{"BuildChain","EndEntityCertOnly"}] string ChainOption;
    [Write,Description("Specifies the password used to protect an exported PFX file."),EmbeddedInstance("MSFT_Credential")] String Password;
    [Write,Description("Specifies an array of strings for the username or group name that can access the private key of an exported PFX file without any password.")] string ProtectTo[];
};

Thumbprint, Store, Subject, Issuer, KeyUsage, EnhancedKeyUsage, Store and Allow Expired are all certificate identifier parameters. Not all combinations of these parameters would be useful, but there is no reason why someone couldn't specify odd combinations. E.g. Thumbprint + Subject would be a bit silly but still valid if you wanted to use that combination of identifier parameters.

By default expired certificates will not be considered for export unless the AllowExpired parameter is true.

Other identifier parameters could also be added later fairly easily if required.

The MatchSource parameter will be useful to replace an already exported certificate when a more appropriate one is found.

If the combination of Identifier parameters returns more than one certificate, then the certificate that expires latest will be selected (this supports the re-export when a certificate is renewed).

Care will need to be taken by users that chosen identifier parameters select a certificate they are certain they want. Specifying the requirements too loosely may result in the wrong certificate being selected for export. E.g. just selecting Store and Issuer would easily select the wrong certificate. So Store, Issuer and FriendlyName might be a far better combo.

As always, your thoughts are most welcome!
Cheers

@johlju
Copy link
Member

johlju commented Jan 16, 2017

That is more extensive schema than I had in my mind, impressive! This will become more than great! 👍

As you say, it will be up to the user to choose the right combination of identifier parameters. Maybe it would be good to group the "identifier parameters" together, maybe in a table, in the documentation (README.md). To say that the result of all of those together will be used as the "filter". Might be easier to understand then that the "filter" must be chosen carefully. 😄

When parameter Ensure is set to 'Absent' would it just delete the certificate at the Path? Or will it do more? If it just deletes the file, one could not use the File resource for that instead? And have less code to worry about in this one :)

You have my approval! 👍

@PlagueHO
Copy link
Member Author

Cool. Thanks @johlju - I think you're right. There is no real need for Ensure - I had it in there for consistency.

What I indent to do is add a new CertificateDsc support module to the resource module with a function Find-Certificate that will only identify a certificate based on identifier parameters passed in. I think it is best to separate the function into a support module so that in theory it could be used by other modules to identify a certificate (e.g. xWebAdministration) for binding.

I'll ensure that we're highlighting which parameters are identifier paramaters. Although in the near future I'm hoping to move this resource over to the automated documentation process that was just implemented in xNetworking (still in Dev only).

But cool! I'll get to work on this over the weekend! Thanks for your help and feedback @johlju

@PlagueHO
Copy link
Member Author

I've been working on this, but have been help up with the DSCResource.Tests changes. I'd like to get those nailed down and released before I finish this one - hopefully won't take too long.

@johlju
Copy link
Member

johlju commented Jan 23, 2017

No worries. One thing at a time :)

@PlagueHO
Copy link
Member Author

Getting back to work on this one because I'm waiting on some input/guidance on this issue - adding Markdown & Example validation and AppVeyor.psm1 to DSCResource.tests and I'm not sure how long it will take (would love your input BTW).

@PlagueHO
Copy link
Member Author

@johlju - made some good progress on this one. I've added a helper function find-certificate that will locate the appropriate certificate based on identifier parameters. You can see the function here: https://github.com/PlagueHO/xCertificate/blob/Issue-41/xCertificateExport/Modules/CertificateDsc.Common/CertificateDSc.Common.psm1#L205

I've got to add some unit tests and then the actual resource creation isn't too difficult.

I'm also moving over to using the new DSCResource.test shared tests/appveyor.psm1. I'm planning on moving over to use the Wiki/Harness methods at some point, but I'll leave that for another PR.

@johlju
Copy link
Member

johlju commented Feb 10, 2017

Awesome! I had this e-mail conversation flagged to get back to you. But it has been very long evenings at the day job recently :/ Good that you started this! :) I will look at the function this weekend and also try to help review PR's that you got going for the new comment tests etc. in the different resources.
Gonna be great to have a nice and relaxed weekend with DSC ;)

@johlju
Copy link
Member

johlju commented Feb 10, 2017

I was not ironical. Genuinely looking forward working with DSC. 😄

@PlagueHO
Copy link
Member Author

@johlju - haha I know you're like me and actually enjoy writing DSC resources 😆

PlagueHO added a commit that referenced this issue Mar 2, 2017
Added xCertificateExport Resource - Fixes #41
@PlagueHO PlagueHO removed the in progress The issue is being actively worked on by someone. label Jun 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resource proposal The issue is proposing a new resource in the resource module.
Projects
None yet
Development

No branches or pull requests

2 participants