certificates icon indicating copy to clipboard operation
certificates copied to clipboard

Enable SQL Backend

Open TheSecMaven opened this issue 4 years ago • 4 comments
trafficstars

Description

This resolves https://github.com/smallstep/certificates/issues/688 and enables users to perform a normal upgrade of smallstep, and the new schema will be migrated to. At which point users can begin to query for certificates the application has signed on a go forward basis.

Note, some tests fail at present because GetCertificate changed to use Context in some places. Figured we could discuss before changing that, or handle however your team prefers.

TheSecMaven avatar Sep 21 '21 15:09 TheSecMaven

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Sep 21 '21 15:09 CLAassistant

You will see that some methods haven't changed to a SQL backend fully, and implement the old NoSQL methods under the sql.DB. I think there is an opportunity to slow roll and migrate to a SQL schema as time passes. This at least enables SQL for storing certificates, which is a great start and can be built on top of.

TheSecMaven avatar Sep 21 '21 15:09 TheSecMaven

Hey @mkkeffeler the way that we're beginning this implementation isn't quite what we're looking for.

What we want (and would consider merging) is a SQL implementation of the following three interfaces:

  • Auth DB: https://github.com/smallstep/certificates/blob/8df9f629b173182eb9c73768c87b377cb4f56d53/db/db.go#L44-L57
  • ACME DB: https://github.com/smallstep/certificates/blob/8df9f629b173182eb9c73768c87b377cb4f56d53/acme/db.go#L15-L40
  • Admin DB: https://github.com/smallstep/certificates/blob/2d5bfd34857eaebd682a91016a4106b03bb332cc/authority/admin/db.go#L57-L70

Here's an example of one method on the Auth DB interface implemented with SQL:

func (d *DB) GetCertificate(serialNumber string) (*x509.Certificate, error) {
	var raw []byte

	if err := d.db.QueryRow("SELECT certificate FROM certificates WHERE serial = $1", serialNumber).Scan(&raw); err != nil {
		return nil, errors.Wrap(err, "error retrieving certificate")
	}

	cert, err := x509.ParseCertificate(raw)
	if err != nil {
		return nil, errors.Wrap(err, "error parsing certificate")
	}

	return cert, nil
}

There's ~40 methods across all 3 interfaces that will need to be implemented (with unit tests). Let me know if that does / doesn't make sense.

Note: this is a big code change and it's possible we won't have time to prioritize and thoroughly review it until next year.

dopey avatar Sep 22 '21 19:09 dopey

There's more to what @dopey said. That works for a custom implementation of the start of the ca (ca/ca.go), the rest of the code stays the same, but to propper implement the functionality, there are some architectural changes that we need to make. For example to be able to register and load these interfaces in the right place.

maraino avatar Sep 22 '21 19:09 maraino