ACME External Account Binding

[hslatman]

Description

This PR adds External Account Binding support to the ACME provisioner.

The implementation currently supports key material creation by using a new admin (beta) endpoint at /admin/acme/eab. Changes to the CLI can be found here.

Still need to iron out some details and have to add tests, but the basics work with acmez already 🙂

[dopey]

certificates/ca/ca.go

Lines 154 to 171 in 0dd6564

	// ACME Router
	prefix := "acme"
	var acmeDB acme.DB
	if config.DB == nil {
	acmeDB = nil
	} else {
	acmeDB, err = acmeNoSQL.New(auth.GetDatabase().(nosql.DB))
	if err != nil {
	return nil, errors.Wrap(err, "error configuring ACME DB interface")
	}
	}
	acmeHandler := acmeAPI.NewHandler(acmeAPI.HandlerOptions{
	Backdate: *config.AuthorityConfig.Backdate,
	DB: acmeDB,
	DNS: dns,
	Prefix: prefix,
	CA: auth,
	})

-- In this section we create the acmeDB, and then directly below we instantiate the admin API handler. We can just pass the acmeDB to the admin Handler as another argument.

Or we can just add another attribute to authority (here

certificates/authority/authority.go

Lines 35 to 73 in 0dd6564

	type Authority struct {
	config *config.Config
	keyManager kms.KeyManager
	provisioners *provisioner.Collection
	admins *administrator.Collection
	db db.AuthDB
	adminDB admin.DB
	templates *templates.Templates
	// X509 CA
	x509CAService cas.CertificateAuthorityService
	rootX509Certs []*x509.Certificate
	rootX509CertPool *x509.CertPool
	federatedX509Certs []*x509.Certificate
	certificates *sync.Map
	// SCEP CA
	scepService *scep.Service
	// SSH CA
	sshCAUserCertSignKey ssh.Signer
	sshCAHostCertSignKey ssh.Signer
	sshCAUserCerts []ssh.PublicKey
	sshCAHostCerts []ssh.PublicKey
	sshCAUserFederatedCerts []ssh.PublicKey
	sshCAHostFederatedCerts []ssh.PublicKey
	// Do not re-initialize
	initOnce bool
	startTime time.Time
	// Custom functions
	sshBastionFunc func(ctx context.Context, user, hostname string) (*config.Bastion, error)
	sshCheckHostFunc func(ctx context.Context, principal string, tok string, roots []*x509.Certificate) (bool, error)
	sshGetHostsFunc func(ctx context.Context, cert *x509.Certificate) ([]config.Host, error)
	getIdentityFunc provisioner.GetIdentityFunc
	adminMutex sync.RWMutex
	}

) like acmeDB and just store the acmeDB there when you create it (using a setter method since you won't have access to the private variable from ca/ca.go). The admin handler already takes a *authority as its only argument. Then you'd just add a method like GetACMEDatabase (here

certificates/authority/authority.go

Lines 500 to 509 in 0dd6564

	// GetDatabase returns the authority database. If the configuration does not
	// define a database, GetDatabase will return a db.SimpleDB instance.
	func (a *Authority) GetDatabase() db.AuthDB {
	return a.db
	}
	// GetAdminDatabase returns the admin database, if one exists.
	func (a *Authority) GetAdminDatabase() admin.DB {
	return a.adminDB
	}

).

I'm fine with either method. The second may look a bit cleaner.

[hslatman]

I'm fine with either method. The second may look a bit cleaner.

I've chosen the first method, so that the acmeDB dependency is injected directly. While your second suggestion would result in a similar setup as the adminDB and does indeed look cleaner from that perspective, I think that making the Authority keep a reference to the acmeDB is not its responsibility.

[hslatman]

This PR requires the changes from smallstep/linkedca#5 to be merged.

[hslatman]

@dopey: this PR now also includes tests for the Admin API (Admin, Provisioner, EAB). Thought it would be good to add those, so that new stuff can be added on top of the existing functionality.

[dopey]

See comments.

[dopey]

Why are we using mutexes? And if they are necessary will this be (yet another) a problem when running multiple open source CAs that read/write from the same DB?

[hslatman]

See explanation about mutexes in the other comment.

Regarding running multiple open source CAs: using Badger will already stop you from doing that, as only a single process can work with the DB at a single time. When using MySQL, the mutexes are not going to save our day, because they're not distributed. We'd need a distributed locking mechanism built on top of Raft or Paxos, for example, to do so (using the current database interface). I guess the same "guarantees" apply as for HA ACME: it might just work. A (fully) distributed CA would be super interesting from an engineering perspective, though 😉

The above would probably be quite different if the DB layer provided full control over transactions (like explicit row locking) and a consistent view of the data within that transaction, for example through MVCC.

[dopey]

I can see that we might want the mutex here, but I want to understand why we need them for the other reads/writes/updates. Like if we're just doing a lookup by an ID. We don't generally use mutexes in other places for this (maybe we should be ?).

[hslatman]

Added them in these other places out of an abundance of caution. For the ones in eakEADID and removeEAKID I have followed your pattern, because I thought they would certainly make sense there. For the others, I thought it wouldn't harm to add them there too.

When retrieving ACME EAB keys, of which there can be many, it can take a while before they're loaded and sent to the client. Considering that adding EAB keys may be scripted, it's likely that there will be multiple calls to adding new keys, perhaps mixed with retrievals. I wanted to have a strong guarantee that the results would always be consistent.

The various concrete nosql implementations do (or at least seem to) offer transactions and provide means for a consistent view of the data (based on views), but since these implementations currently don't allow us to explicitly open and commit transactions for multiple types of entities in a single transaction, I try to mimic that using (rw) mutexes. Doing it at the DB API level may not be the best way to do it, but so far it worked quite well 😄. Ideally I would've liked direct control over when a DB transaction was started, the total unit of work to operate on, and then explicitly committed all at once.

[dopey]

it's an external_account_keyID by reference, right? You use keyID in the table and variable name below, but not in this one.

[hslatman]

Fixed in bf21319.

[hslatman]

In bf21319 I also fixed a bug with persistence. I must've missed this case in other manual tests due to using an existing ACME provisioner. It now seems to work correct doing multiple cycles of adding and removing keys with an entirely new provisioner.

[dopey]

lgtm