fleetdm / fleet

Open-source platform for IT, security, and infrastructure teams. (Linux, macOS, Chrome, Windows, cloud, data center)
https://fleetdm.com
Other
3.02k stars 419 forks source link

Validate that Windows MDM is fully configured before allowing to enable it #14446

Closed mna closed 10 months ago

mna commented 12 months ago

Fleet version: <!-- Copy this from the "My account" page in the Fleet UI, or run fleetctl --version --> As of current main: commit 2ad6fb3603a13a268f19b00bb0788cce16ce1783

Web browser and operating system: N/A


💥  Actual behavior

With just the environment variable set (FLEET_DEV_MDM_ENABLED=1), no WSTEP certificate configured, I was able to enable MDM for Windows via the UI.

fleet-windows-mdm-enabled

🧑‍💻  Steps to reproduce

FLEET_DEV_MDM_ENABLED=1 \
 ./build/fleet serve --dev --dev_license --logging_debug

Go to Settings->Integrations->MDM, click to enable Windows MDM.

🕯️ More info (optional)

This is the current validation that sets Windows MDM as enabled or not: https://github.com/fleetdm/fleet/blob/main/server/service/appconfig.go#L713-L718

Given that WSTEP certs are required to properly enable hosts, it should check that those certificates were provided before allowing the user to enable Windows MDM (a bit like we do to enable Apple MDM with SCEP certs).

Note that the docs do mention that the WSTEP cert must be configured: https://fleetdm.com/guides/windows-mdm-setup#basic-article, it's just not enforced by the code.

Current status

noahtalerman commented 12 months ago

Given that WSTEP certs are required to properly enable hosts, it should check that those certificates were provided before allowing the user to enable Windows MDM (a bit like we do to enable Apple MDM with SCEP certs).

@mna yes! 💯 Default to making is like macOS

mna commented 11 months ago

@noahtalerman I added the validation so that when trying to enable Windows MDM, it fails if the WSTEP certificates were not provided. However it will need some frontend changes to pick up the message from the server, as currently it looks like this:

Screenshot from 2023-11-01 10-06-37

On the server, it returns the validation error with this message:

Windows WSTEP configuration must be provided to enable Windows MDM

Note that it behaves differently from macOS because the mac MDM does not have to be explicitly enabled in the UI - if the certificate/key pairs are provided at Fleet startup, it is automatically enabled. We could do the same for Windows (if WSTEP certificate/key pair is provided at startup - and the MDM_ENABLED env var is set as for now it is still a beta feature - , Windows MDM is enabled), but that's not how the UX is designed at the moment, so the validation is done only when trying to turn it on in the UI.

mna commented 11 months ago

@noahtalerman I noticed my previous message was not super clear - I wanted to ask you if you think that this validation, with the frontend being changed to display the message returned by the server "Windows WSTEP configuration must be provided to enable Windows MDM" - works for you. If so I'll follow up with someone from the frontend to make the final adjustment.

noahtalerman commented 11 months ago

@mna I think we want to hide the Turn on Windows MDM UI card if the user can't turn it on (they don't have WSTEP configured).

It looks like this wasn't clear in the GitHub issue. Sorry about that.

cc @marko-lisica

mna commented 11 months ago

@noahtalerman @marko-lisica

we want to hide the Turn on Windows MDM UI card if the user can't turn it on

Gotcha, that's a bit more involved as there is currently no way for the frontend to know if WSTEP is configured server-side or not. Thinking out loud, maybe we can use the existing feature flag/environment variables to change if it gets displayed or not, but that would only be a solution for as long as we have those (and the goal is to remove them once it's out of beta).

Pinging @ghernandez345 or @gillespi314 for ideas on how we could let the frontend know without additional payload, otherwise we'll need to alter the GET /config endpoint response, I'm afraid...

noahtalerman commented 11 months ago

currently no way for the frontend to know if WSTEP is configured server-side or not.

Ah, I see.

I think we move forward w/ show the Turn on Windows MDM card w/ the error flash.

@marko-lisica what do you think the error should be? I'm thinking we should make it obvious that the user needs to plug in some certs first.

See Martin's comment here for what's possible.

Let's punt showing/hiding the Turn on Windows MDM card. I think this needs a bit more thought before we commit to a solution like this.

mna commented 11 months ago

While we wait for confirmation of the copy of the error message, I'll move the ticket back to Ready and add the frontend label. @ghernandez345 assuming you're the one who will tackle this, the change would be to display the server error message instead of the generic "Unable to turn on Windows MDM, please try again" when a status 422 is returned.

/cc @georgekarrv

noahtalerman commented 11 months ago

@mna @ghernandez345 to unblock us, let's use this copy for the error message:

Couldn't turn on Windows MDM. Please configure Fleet with a certificate and key pair first.

@marko-lisica please feel free to tweak this. Also, please check if it's consistent w/ other error messages when you get the chance.

marko-lisica commented 11 months ago

Couldn't turn on Windows MDM. Please configure Fleet with a certificate and key pair first.

@noahtalerman Looks good to me. It follows a principle we defined here

sabrinabuckets commented 11 months ago

Validated that with WSTEP cert & key pair missing I am unable to turn on Windows MDM:

Private Zenhub Video

Validated that Fleet will not start if only one or the other is provided. And validated still able to successfully turn on Windows MDM with valid cert & key pair.

fleet-release commented 10 months ago

No cert, no control, Fleet now guides with sureness, Through clouds, paths unfold.