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

xDSCWebService certificate location #468

Open
pkoelemij opened this issue Nov 24, 2018 · 8 comments
Open

xDSCWebService certificate location #468

pkoelemij opened this issue Nov 24, 2018 · 8 comments
Labels
bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community.

Comments

@pkoelemij
Copy link

pkoelemij commented Nov 24, 2018

Hi there! Is there a reason for hard coding Cert:\LocalMachine\MY for the DSC config?.

This caused some extra configuration because for example win-acme doesnt use the Cert:\LocalMachine\MY as default, but I could think of a couple more use cases. For our particular situation it would be preferable to have this configurable in the DSC config. The $Store parameter is already implemented in the method. Would this be something useful to implement? I wouldn't mind expanding the module and sending a pull request.

@stale
Copy link

stale bot commented Dec 25, 2018

This issue has been automatically marked as stale because it has not had activity from the community in the last 30 days. It will be closed if no further activity occurs within 10 days. If the issue is labelled with any of the work labels (e.g bug, enhancement, documentation, or tests) then the issue will not auto-close.

@stale stale bot added the stale The issue or pull request was marked as stale because there hasn't been activity from the community. label Dec 25, 2018
@PlagueHO PlagueHO added the discussion The issue is a discussion. label Jan 18, 2019
@stale stale bot removed the stale The issue or pull request was marked as stale because there hasn't been activity from the community. label Jan 18, 2019
@PlagueHO
Copy link
Member

Thanks for raising this @lemoneydes . I think being able to override the cert store to use would definitely be useful. It appears that this might have been intended at some point but the dots weren't connected.

I'll assign this as an enhancement.

@PlagueHO PlagueHO added enhancement The issue is an enhancement request. help wanted The issue is up for grabs for anyone in the community. and removed discussion The issue is a discussion. labels Jan 18, 2019
@tmeckel
Copy link
Contributor

tmeckel commented Jan 28, 2019

@PlagueHO it's not only an enhancement; the code at

https://github.com/PowerShell/xPSDesiredStateConfiguration/blob/ecaa8ac48307f55c0272edaccdf612d16000b442/DSCResources/MSFT_xDSCWebService/MSFT_xDSCWebService.psm1#L164

won't work well when there are more certificates with the same subject and or with thumbprint. Especially when we release the restriction on Cert:\LocalMachine\MY as mentioned by the initial author, because you can place the same certificate (regarding it's Thumbprint) without any hassle into different certificate store locations.

In addition Set-TargetResource locates the specified certificate differently than Get-TargetResource especially Get-TargetResource doesn't honer the parameter CertificateSubject at all, whereas Get-TargetResource does.

https://github.com/PowerShell/xPSDesiredStateConfiguration/blob/ecaa8ac48307f55c0272edaccdf612d16000b442/DSCResources/MSFT_xDSCWebService/MSFT_xDSCWebService.psm1#L285

So from my point of view 2 additional bugs here at least. Perhaps even more by a deeper investigation.

@tmeckel
Copy link
Contributor

tmeckel commented Jan 28, 2019

@PlagueHO Another catch :-D

https://github.com/PowerShell/xPSDesiredStateConfiguration/blob/ecaa8ac48307f55c0272edaccdf612d16000b442/DSCResources/MSFT_xDSCWebService/MSFT_xDSCWebService.psm1#L645

Test-TargetResource doesn't check if the current certificate used for the current binding is the same as defined in the current DSC configuration.

@PlagueHO PlagueHO added bug The issue is a bug. and removed enhancement The issue is an enhancement request. labels Jan 28, 2019
@PlagueHO
Copy link
Member

Good catches there @tmeckel ! Changed to bug.

@tmeckel
Copy link
Contributor

tmeckel commented Jan 28, 2019

Implies the label 'bug' any priority or is it just a label? I'm working on the MSFT_xDSCWebService.psm1module anyway (issues: #463, #460) I could try to fix this here as well.
@mhendric

@mhendric
Copy link
Contributor

I would think Bug > Enhancement as far as priority goes. But I'm not aware of any way that we're currently tracking Issue priorities relative to each other. It would be good to throw these on a board of some sort and come up with priorities of what we want addressed first, and what is dependent on other issues being addressed. I'm open to suggestions on how to accomplish that. It also might be good to have more priority labels than just "High Priority" (i.e. have medium and low too).

@tmeckel
Copy link
Contributor

tmeckel commented Jan 29, 2019

@mhendric After reading your message I thought about what's my opinion about this and I came to the conclusion that because this is a community driven project and we thus can't rely on permanently assigned resources it might not be worth putting the effort in prioritizing all issues based on it's type.

In a community project a people pick up issues already on the list (or even provide PRs directly for issues they came accross while using the code) with which they feel comfortable and think they can provide a benefit for the project. If you try to align this with a priority list you might only drive away those people. I do agree that somebody should do a quick triage on incoming issues (is this a task for a maintainer? at least you and @PlagueHO) and because of this we might need more detailed labels. This also would be helpful in the community calls while "reporting" eg to @kwirkykat

Microsoft is such huge company and is overseeing so many projects here on GitHub, has no best practice crystallized here yet that could be adopted? How are the other DSC projects handle this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community.
Projects
None yet
Development

No branches or pull requests

4 participants