search

fleetdm / fleet

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

Verify that Apple BM cert and token are valid at startup #11940

Open mna opened 1 year ago

mna commented 1 year ago

Goal

Follow-up to #11416 , this ticket is to investigate if we should add validation with Apple's API that the Apple BM cert+token are valid at startup.

Context

Currently, at startup, we validate that the Apple BM certificate + private key can successfully decrypt the encrypted server token, and that the token's "renewal date" is in the future (not expired).

However, the token could still fail to authenticate calls to Apple's BM API (e.g. it may be invalid due to the user downloading a new token from the Apple BM website, which invalidates the previous one). If that's the case, we'd want to prevent Fleet from starting and exit with an error. Note that the ticket is to investigate if we should add that validation, not to necessarily do it. The reason for this is that adding too much delays to startup could cause issues with deployments that use the healthcheck endpoints to return a success to identify a Fleet instance as healthy or failed.

More context here: https://github.com/fleetdm/fleet/issues/11416#issuecomment-1559594942 and https://github.com/fleetdm/fleet/issues/11416#issuecomment-1561222594.

Potential solutions

If we feel like it is worth adding this validation taking into account the concerns about startup delay, add a call to Apple's BM API at startup and check if it succeeds or returns an authentication error (which would indicate an invalid cert/token).

lukeheath commented 1 year ago

@georgekarrv This isn't exactly a bug, but that's the closest I can think of right now. I am assigning it to you for triage.

georgekarrv commented 1 year ago

Well i think it requires product input at this time. I wouldn't personally want the server to not start in the case the cert in invalid. I would want to access the ui and be shown a banner error / warning that the cert has expired or is no longer valid and needs to be fixed for mdm functionality to resume.

georgekarrv commented 1 year ago

@noahtalerman this sounds like a core or cleanup feature to periodically check cert validity and be able to present to the user when it is invalid and better yet when it's < N days from expiring present the warning ahead of time so they can preempt the break in functionality.

noahtalerman commented 1 year ago

@georgekarrv I think this is something we should take on now. I agree it's a bug. This issue is about preventing the Fleet server from starting if the user supplies invalid ABM cert/token. User is providing an invalid cert/token from the get go.

There's a separate cleanup feature to check if the cert becomes invalid or is about to become invalid (expire) and present a message to the user. This issue covers that separate feature: #11941

georgekarrv commented 1 year ago

I think we don't want this functionality when we have #11932 to supply the information to the user that this is not working from the api. It would be a headache for infra to handle and prevent the rest of the server functionality and I don't think we want that.

georgekarrv commented 1 year ago

@noahtalerman ^ Please let me know if this is what we do with other settings like this already or not but if we don't have an official stance yet I think runtime error is better than start up error 99% of the time.

lukeheath commented 1 year ago

We don't want to leave bugs in the inbox once they are looked at. I'm moving this over to the backlog. Feel free to close/respec if ya'll decide to go another route, but better for it to wait in the backlog than the inbox.

zhumo commented 1 year ago

@noahtalerman please bring to FF for prioritization if desired.