fleet icon indicating copy to clipboard operation
fleet copied to clipboard

Store fleetd's TLS certificate and key in Keychain Access

Open zhumo opened this issue 1 year ago • 25 comments

Goal

User story
As an endpoint operator admin,
I want to fleetd to store its TLS certificate and key in Keychain Access for macOS hosts
so that I can obscure the contents of the cert/key.

Changes

This issue's estimation includes completing:

  • [ ] fleetd changes:
    • fleetd at startup looks for this environment variable: FLEET_TLS_CLIENT_CERTIFICATE_NAME
    • Based on the names, it looks up the certificate/key in the Keychain or Windows Credential Manager.
    • fleetd shows a helpful error if it can't extract the certificate and/or key
    • fleetd shows a helpful error if the certificate and/or key don't exist
  • [ ] osquery changes: Add a new env variable to osquery to accept the cert/key
  • [ ] Outdated documentation changes:
    • Update TLS docs to explain how to store fleetd's TLS certificate and key in Keychain Access (macOS) or Credential Manager (Windows): users need to do the following:
      • Add the certificate and key
      • Mark the cert as extractable and not sensitive in the keychain (so that the private key can be extracted)
      • Update the fleetd env variables to point fleetd to the cert/key

Engineering

  • [ ] REST API changes: TODO
  • [ ] Database schema migrations: TODO
  • [ ] Documentation changes complete

Engineering scope

  • TLS 1.3 only? (do not support TLS 1.2)
  • Which signature algorithms to support? We can limit the scope by supporting only one initially, like rsa_pss_rsae_sha256 https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.3
  • Support certificate chains (intermediate certificates)? We should, since this is a standard feature of certificates.

QA

Risk assessment

  • Requires load testing: TODO
  • Risk level: Low / High TODO
  • Risk description: TODO

Manual testing steps

Testing notes

Confirmation

  1. [ ] Engineer (@____): Added comment to user story confirming succesful completion of QA.
  2. [ ] QA (@____): Added comment to user story confirming succesful completion of QA.

zhumo avatar May 03 '23 03:05 zhumo

Hey @zayhanlon @ksatter. We're deprioritizing this issue as we won't be able to deliver it in the next 6 weeks. Please bring this back to the PFR call if it surfaces again so we can re-prioritize

zayhanlon avatar Jun 02 '23 18:06 zayhanlon

FF

Noah: How painful is this right now? Has it gotten more painful compared to 4 months ago?

Noah: WONT

noahtalerman avatar Oct 12 '23 20:10 noahtalerman

Hey @zayhanlon this wasn't prioritized at FF. Please bring it back if you think we should reconsider.

noahtalerman avatar Oct 12 '23 20:10 noahtalerman

Zay: Medium priority

noahtalerman avatar Oct 20 '23 14:10 noahtalerman

Luke: Might be stored on the disk somewhere today.

Luke: If we did this for mTLS we would be able reuse this work for the the enroll secret storage and other credentials in the future.

noahtalerman avatar Dec 14 '23 20:12 noahtalerman

From : https://github.com/keybase/go-keychain I took some example code and created this simple app to store and read to/from keychain. It works for me. feel free to use it.

Import: go get github.com/keybase/go-keychain

package main

import (
	"fmt"

	"github.com/keybase/go-keychain"
)

func writeToKeychain() {
	item := keychain.NewItem()
	item.SetSecClass(keychain.SecClassGenericPassword)
	item.SetService("MyService")
	item.SetAccount("gabriel")
	item.SetLabel("A label")
	item.SetAccessGroup("A123456789.group.com.mycorp")
	item.SetData([]byte("toomanysecrets"))
	item.SetSynchronizable(keychain.SynchronizableNo)
	item.SetAccessible(keychain.AccessibleWhenUnlocked)
	err := keychain.AddItem(item)

	if err == keychain.ErrorDuplicateItem {
		// Duplicate
	}
}

func readFromKeychain() string {
	query := keychain.NewItem()
	query.SetSecClass(keychain.SecClassGenericPassword)
	query.SetService("MyService")
	query.SetAccount("gabriel")
	query.SetAccessGroup("A123456789.group.com.mycorp")
	query.SetMatchLimit(keychain.MatchLimitOne)
	query.SetReturnData(true)
	results, err := keychain.QueryItem(query)
	if err != nil {
		// Error
	} else if len(results) != 1 {
		return "not found"
	} else {
		return string(results[0].Data)
	}
	return "not found"
}

func main() {
	// Write to Keychain
	writeToKeychain()
	pass := readFromKeychain()

	fmt.Println("Read password from Keychain:", pass)
}

result: > Read password from Keychain: toomanysecrets

sharon-fdm avatar Jan 04 '24 15:01 sharon-fdm

@getvictor can you please help us put together a diagram of the expected workflows for (1) the customer and (2) everyone else?

This way, we can show the customer on a call and get feedback.

noahtalerman avatar Jan 09 '24 16:01 noahtalerman

Hey @sharon-fdm heads up, this story needs more drafting based on learnings from customer and changes to osquery.

I removed the release label, added product label, and assigned myself.

@lukeheath heads up, because this story needs changes from osquery (likely won't be completed in 4.44 release timeline), I removed the 4.44 milestone.

noahtalerman avatar Jan 10 '24 21:01 noahtalerman

@noahtalerman #13832 is also about storing stuff in the keychain (Also in Windows Credential manager) Should @getvictor stop working on it? Or maybe only do the Windows part?

sharon-fdm avatar Jan 10 '24 21:01 sharon-fdm

Proposal:

fleetd at startup looks for environment variables:

FLEETD_CLIENT_TLS_CERTIFICATE_NAME
FLEETD_CLIENT_TLS_KEY_NAME

Based on those names, it looks up the certificate/key in the Keychain or Windows Credential Manager.

getvictor avatar Jan 10 '24 21:01 getvictor

@noahtalerman https://github.com/fleetdm/fleet/issues/13832 is also about storing stuff in the keychain (Also in Windows Credential manager) Should @getvictor stop working on it? Or maybe only do the Windows part?

@sharon-fdm good question. I think no. We continue working on #13832.

We learned that this story (#11494) has some added complexity because the TLS certificate/key is used by other tools (not just Fleet) and has a unique name per device (defined by the customer).

For #13832, the enroll secret is only used by Fleet and it's set at package time so I don't think there are other edge cases that we have to consider.

@getvictor please correct me if I'm wrong.

noahtalerman avatar Jan 10 '24 21:01 noahtalerman

Update after 1/16 osquery office hours.

The best practice is to create a TLS signature without extracting the private key from the macOS keychain. However, osquery will likely not have this support in the next 5.12 release.

As a stopgap, fleetd can extract the cert/key from the keychain and pass it to osquery as an env variable.

  • requires small osquery change to accept env variable (for next 5.12 release)
  • requires small fleetd change to extract the cert/key
  • customers would need to mark the cert as extractable and not sensitive in the keychain (so that the private key can be extracted)

getvictor avatar Jan 19 '24 14:01 getvictor

  • customers would need to mark the cert as extractable and not sensitive in the keychain (so that the private key can be extracted)

Does this mean setting the trust settings or would it be viewable in plain text? Only viewable to an admin user?

Would deploying a Fleet.app bundle make any of this better? more secure?

nonpunctual avatar Jan 19 '24 14:01 nonpunctual

I believe most people use extractable and non-sensitive identities in the keychain -- that appears to be the default. Such an item on the System keychain is still subject to Access Control and can only be extracted by admin.

For reference, macOS security CLI tool can import identity as non-extractable by passing -x option. The IsSensitive attribute doesn't appear to have wide usage. Keys are non-sensitive by default.

getvictor avatar Jan 19 '24 19:01 getvictor

I guess I assume the key is sensitive though? Like, it should only be extractable by the service, not by any user (even admin) because it's a persistent API key that never gets rotated if I understand correctly. I'm certainly not any kind of decider :) but I would think this should be locked as tight as possible?

nonpunctual avatar Jan 19 '24 22:01 nonpunctual

Hey @getvictor what do you think about this naming for the env variables instead?

FLEET_TLS_CLIENT_CERTIFICATE_NAME
FLEET_TLS_CLIENT_KEY_NAME

We already have --fleet-tls-client-certificate and --fleet-tls-client-key flags for fleetctl package that function very similarly (accept a path instead of a name).

I think no need for a FLEETD_ or ORBIT_ prefix because these env variables touch both orbit and osquery functionality. I think for env variables that touch many components of fleet we can remove the prefix.

What do you think?

Also I updated the "Changes" section in the story to reflect the plan here.

Let me know what you think.

noahtalerman avatar Jan 23 '24 14:01 noahtalerman

The best practice is to create a TLS signature without extracting the private key from the macOS keychain. However, osquery will likely not have this support in the next 5.12 release.

@getvictor when osquery supports this, what do you suggest we do? Will our solution (passing cert/key to osquery) break? If not, should we adopt this best practice solution? Does it depend on how the customer is using the Keychain?

noahtalerman avatar Jan 23 '24 14:01 noahtalerman

Learned from customer-starkchik that their TLS key is non extractable.

So, to support them, fleetd and osquery will need to be able to use cert/key when key is non-extractable.

Most of their tools seems to support using a non-extractable key:

  • Munki
  • Others?

The next step is to look into these tools to learn how they're doing it. Maybe we can point the folks making the osquery changes in the same direction?

cc @getvictor

noahtalerman avatar Jan 23 '24 19:01 noahtalerman

The next step is to look into these tools to learn how they're doing it. Maybe we can point the folks making the osquery changes in the same direction?

@sharon-fdm can you please file a separate timebox story that we can bring into the next sprint?

noahtalerman avatar Jan 23 '24 22:01 noahtalerman

https://www.linkedin.com/posts/microsoft-entra_join-brian-melton-grace-olga-dalton-and-activity-7155690107008675840-avOZ?utm_source=share&utm_medium=member_desktop

nonpunctual avatar Jan 24 '24 05:01 nonpunctual

Victor:

I'm ready to write the code for the Orbit piece. We could ship the change where Orbit uses the cert from Keychain while osquery still uses the cert in the filesystem.

Then, when osquery adds a config for receiving the cert Orbit will pass osquery the cert and the customer can remove the cert from the filesystem.

Noah:

TODO Schedule a follow up call w/ Victor to chat about this.

noahtalerman avatar Feb 15 '24 19:02 noahtalerman

Munki docs are somewhat hard to parse through. Munki uses the Apple API to do URL requests. This means that, just like when using Safari, the user gets a pop-up to select the client certificate for the mTLS connection.

getvictor avatar Feb 15 '24 19:02 getvictor

Can you point me to that spot in the munki docs? Is it possible that an admin deployiung munki can deploy a cert or a PPPC profile to prevent this popup?

The munki code for fetching an identity from the keychain is here.

There is no profile you can deploy to prevent a popup. The admin needs to ensure the calling app has an ACL entry to use the private key in the keychain. If the calling app isn't in the ACL list, then the OS will surface the keychain prompt to allow the user to deny or approve the request.

Setting of the ACLs is best handled at key creation and import time. For example, using the security import tool from Apple, you can specify the -A flag to allow any app to use the key (insecure) or you can pass -T along with a path to an app that can use the key (you can pass -T /path/to/app multiple times).

bfreezy avatar Mar 06 '24 15:03 bfreezy

Hey @getvictor and @Patagonia121, heads up, we didn't get to this one in the last design sprint.

Bringing it back to feature fest.

noahtalerman avatar Mar 28 '24 18:03 noahtalerman

Hey @Patagonia121, heads up, we reviewed this during feature fest.

We don't have the space to take this one in the upcoming design sprint (4.49).

Removing this from the feature fest board.

noahtalerman avatar Mar 29 '24 21:03 noahtalerman

We have been waiting for osquery to add support for mTLS certificates stored in the Windows store and macOS keychain since February 2024. Perhaps we need to take over that work.

We already have a proof of concept mTLS Go client using macOS keychain

getvictor avatar Jun 05 '24 14:06 getvictor

If we want to speed up the osquery portion of this feature, we could consider paying the osquery contributor working on it.

getvictor avatar Jun 10 '24 19:06 getvictor

Brock: Jamf stores all it's certs in Keychain Access

noahtalerman avatar Jun 20 '24 20:06 noahtalerman