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 418 forks source link

`api/latest/fleet/mdm/apple_bm` returns 500 error if invalid key/cert provided #11416

Closed lukeheath closed 1 year ago

lukeheath commented 1 year ago

Fleet version: (head to the "My account" page in the Fleet UI or run fleetctl --version) main: 4fdf640

Operating system: (e.g. macOS 11.2.3) MacOS

Web browser: (e.g. Chrome 88.0.4324) Chrome


🧑‍💻  Expected behavior

I expect to receive a 400 Bad Request

💥  Actual behavior

I receive a 500 Server Error with the following payload:

{
  "message": "DEP auth error: 400 Bad Request: oauth_problem_adviceBad Request",
  "errors": [
    {
      "name": "base",
      "reason": "DEP auth error: 400 Bad Request: oauth_problem_adviceBad Request"
    }
  ]
}

👣 Reproduction steps

  1. Add an invalid ABM key/cert via environment variables.
  2. Start the server and navigate to Settings > Integrations
  3. Check network tab for failed request to /fleet/mdm/apple_bm

More info

roperzh commented 1 year ago

@lukeheath I think this one might require extra thoughts/specs, looking at the docs:

An HTTP 400 Bad Request error indicates one of the following:

  • Unsupported OAuth parameters
  • Unsupported signature method
  • Missing required authorization parameter
  • Duplicated OAuth protocol parameter

An HTTP 401 Unauthorized error indicates one of the following:

  • Invalid consumer key
  • Invalid or expired token
  • Invalid signature
  • Invalid or already-used nonce

An HTTP 403 Forbidden error indicates one of the following:

  • The MDM server, or the MDM server's consumer key/token does not have access to perform the specific request. In this case, the request body contains ACCESS_DENIED.
  • The organization has not accepted the latest terms and conditions of the program. In this case, the request body contains T_C_NOT_SIGNED.

In light of that:

lukeheath commented 1 year ago

@roperzh Thanks for the info! Yes, we'll need to determine how to surface this to the user since this API interaction is driven by an environment variable.

@noahtalerman would you please take a look and let us know how we should notify the user in this case? Thanks!

noahtalerman commented 1 year ago

Hey @lukeheath I think I would expect the Fleet server to fail to start and show an error if the ABM key/cert is invalid.

cc @mna

lukeheath commented 1 year ago

Good idea! @mna Let's go with validating the key/certs on start and failing to start the server if they are invalid.

mna commented 1 year ago

@noahtalerman @lukeheath We had discussed doing it at startup when we first implemented it, but there were some concerns (in particular, extra time required at startup before the Fleet instances report as "healthy", and the possibility of the Apple API being down, preventing startup). The discussion at that time: https://github.com/fleetdm/fleet/issues/8725#issuecomment-1332294940

A few options I can think of:

Even if we do run the validation at startup, there's still the risk that Apple's API could be down at that moment and that could prevent starting the instance (and there's the off-chance that Apple invalidates a cert at any point for some reason). Of course this is not something that should be super common, but even at 5 nines availability (which is unlikely to be that high), that's about a minute every 2 months. We could differentiate between an "invalid cert" API response and an unreachable/Apple's server error, and keep going if Apple is unreachable (instead of preventing startup), but we could then find out later on that the cert is actually invalid.

Being able to handle that dynamically (i.e. at runtime, not at startup) is more robust and flexible, would handle all curveballs that a third party API could throw at us, but is probably a bit more involved (though not that much, as mentioned we have similar logic for the expiration). A step further would be to design a "notification center" for all such "background failures" that should bubble up to the user, but that's of course a whole new feature and quite a bit more work.

mna commented 1 year ago

That being said, for the api/latest/fleet/mdm/apple_bm endpoint itself, we can definitely handle an error from the call to Apple's API and return a 4xx instead of 500 when we can detect that it's due to the cert configuration.

mna commented 1 year ago

@roperzh @lukeheath

For the cases where is the user's fault, should we display a banner like we do for 403s? otherwise without an alarm raising (a 500 error) and an user message, the issue will fly under the radar.

Wouldn't the frontend display the usual "request failed" error with the details from the response's payload if the server endpoint returned a 400? If not would it be a big change to make it do that? My understanding is that at the moment, the error is not displayed to the user (and only visible via the browser's developer tools) due to being a 500.

mna commented 1 year ago

@noahtalerman @lukeheath A recap on this ticket and the remaining decisions to take now that I created the PR to change the status code of that API endpoint to a 400 if the ABM certificate/token is invalid:

  1. I think at the very least we'd want the UI to display a flash message in the Settings->Integrations->Automatic enrollment if the API endpoint returns a 400 error, in this page:

Screenshot from 2023-05-24 09-46-56

Something like that for the error message: "The Apple Business Manager certificate or server token is invalid. Restart Fleet with a valid certificate and token. See https://fleetdm.com/docs/using-fleet/mdm-setup#apple-business-manager-abm for help."

  1. In addition to step 1, keeping in mind the concerns about delaying startup of Fleet, we could validate the cert/token with Apple's API. Note that we already validate that the token can be decrypted with the certificate, and that it is not expired, so it is unlikely that the provided cert/token are invalid! The most likely reason that it would be invalid is if the user downloaded a new ABM token but provided the old one (Apple invalidates the old one when a new one is downloaded). Another possibility is that the token expired since startup. Both of those cases can happen after startup and as such, validating this at startup is not sufficient.

  2. In addition to step 1 and independently of whether we implement step 2, we could also display the banner whenever we detect that the certificate has become invalid. This adds the benefit that the user doesn't have to navigate to the "Settings->Integrations->Automatic enrollment" page to see that the cert/token is invalid.

The PR currently implements 1. only (and still requires frontend changes and adding the user-friendly message).

lukeheath commented 1 year ago

@mna Thanks for the update!

@georgekarrv @noahtalerman Would y'all please sync up with Martin on this today and make sure we're aligned on how to implement a fix? Thanks!

mna commented 1 year ago

We discussed this during standup, the decision is that:

Once https://github.com/fleetdm/fleet/pull/11899 gets merged this ticket can be closed (after QA of course).

xpkoala commented 1 year ago

@mna I believe I'm sending invalid files used for authentication in order to test this fix, but I'm still receiving a 200 status result.

In both cases I'm receiving a 200 response. It seems as if I am modifying the wrong file or I need to trigger an update to Apple BM with the invalid keys?

mna commented 1 year ago

@xpkoala this is a bit more involved because if you just provide invalid cert/key or token files, it will be validated at startup as invalid (e.g. cannot be parsed as a valid cert/key or cannot decrypt the token) and Fleet should not even start. This is what I get when I modify my certificate by changing random chars:

Failed to start: validate Apple BM token, certificate and key: Apple BM configuration: parse key pair: x509: malformed signature

and if I change the token with invalid chars:

Failed to start: validate Apple BM token, certificate and key: Apple BM configuration: decrypt token: ber2der: BER tag length too long

So this technique cannot be used to verify that the endpoint does return 400 (you shouldn't have been able to even start Fleet with those invalid files - at least I'm not).

The only way I can think of, as the files have to be otherwise valid to decrypt the token and the token must be valid json too, is to start fleet with valid files, then go to Apple's Business Manager website and download a new token file:

  1. go to business.apple.com, login and click on your username in the lower left corner, select "Preferences"
  2. in the list of "Your MDM Servers", select yours (assuming you have the certificate+private key associated with your MDM server defined here)
  3. at the top of the rightmost screen section, you should see "Download token" - doing this will download a new Apple BM encrypted token, and will disable the old one (make it invalid).
  4. make sure you store your new token so you can run with valid Apple BM settings later, but for now start Fleet with the old/now invalid apple_bm token

Making a request to the apple_bm endpoint should result in this (I tested by running fleetctl get mdm-apple-bm which calls this endpoint):

could not get Apple BM information: GET /api/latest/fleet/mdm/apple_bm received status 400 Bad request: Get "https://mdmenrollment.apple.com/account": DEP auth error: 400 Bad Request: oauth_problem_adviceBad Request
mna commented 1 year ago

@xpkoala let me know if you need any help/clarification for that. Happy to jump on a call too.

fleet-release commented 1 year ago

Invalid key, error, Nature's calm teaches right path, Fleet finds solutions.