CS: Store customer keys in trust database

[lukedirtwalker]

Only use files to create the initial state and from then on work with the DB.

---

This change is 

[oncilla]

Reviewed 8 of 8 files at r1, 3 of 3 files at r2.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @lukedirtwalker)

---

go/cert_srv/main.go, line 79 at r1 (raw file):

	defer env.CleanupLog()
	defer env.LogAppStopped(common.CS, config.General.ID)
	defer log.LogPanicAndExit()

This change should be done in the fix for #2242.

---

go/cert_srv/internal/csconfig/customers.go, line 45 at r2 (raw file):

// Customers is a mapping from non-core ASes assigned to this core AS to their public
// verifying key.
type Customers struct {

Since the keys are now stored in TrustDB I don't think this struct is useful anymore.
The functions might as well be on State instead.

---

go/cert_srv/internal/csconfig/customers.go, line 117 at r2 (raw file):

// SetVerifyingKey sets the verifying key for a specified AS. The key is written to the file system.
func (c *Customers) SetVerifyingKey(ctx context.Context, ia addr.IA, ver uint64,

The point of checking against the old key was to simulate a "transaction".
(which is not completely clean)

Since the customer keys and certs are now in the same database, we could do this cleanly in one query/transaction.

Essentially the logic should be the following:

When issuing a new chain for the customer:

• The new verifying key must only be inserted if the chain is inserted as well (-> chain must not exist before)
• The new chain must only be inserted if the verifying key is inserted as well (-> verifying key must not exist before)
• The verifying key must only be inserted if the latest verifying key in the DB is the same as oldKey and the version is newVersion-1.

(The last condition could be simplified, I think)

---

go/cert_srv/internal/reiss/handler.go, line 188 at r2 (raw file):

	}
	// Set verifying key.
	err = h.State.Customers.SetVerifyingKey(ctx, c.Subject, c.Version, c.SubjectSignKey, vKey)

As mentioned above, this would be a good fit for a DB transaction.

---

go/lib/infra/modules/trust/trustdb/trustdb.go, line 65 at r2 (raw file):

	io.Closer
	// GetCustKey gets the customer signing key for the given AS in the latest version.
	GetCustKey(ctx context.Context, ia addr.IA) (common.RawBytes, error)

It would be useful to also return the version.

[lukedirtwalker]

Reviewable status: 5 of 10 files reviewed, 5 unresolved discussions (waiting on @oncilla)

---

go/cert_srv/internal/csconfig/customers.go, line 45 at r2 (raw file):

Previously, Oncilla wrote…

Since the keys are now stored in TrustDB I don't think this struct is useful anymore.
The functions might as well be on State instead.

Hm I left it here since I still think it is a nice split of functionality.

---

go/cert_srv/internal/csconfig/customers.go, line 117 at r2 (raw file):

Previously, Oncilla wrote…

The point of checking against the old key was to simulate a "transaction".
(which is not completely clean)

Since the customer keys and certs are now in the same database, we could do this cleanly in one query/transaction.

Essentially the logic should be the following:

When issuing a new chain for the customer:

• The new verifying key must only be inserted if the chain is inserted as well (-> chain must not exist before)
• The new chain must only be inserted if the verifying key is inserted as well (-> verifying key must not exist before)
• The verifying key must only be inserted if the latest verifying key in the DB is the same as oldKey and the version is newVersion-1.

(The last condition could be simplified, I think)

Done.

---

go/cert_srv/internal/reiss/handler.go, line 188 at r2 (raw file):

Previously, Oncilla wrote…

As mentioned above, this would be a good fit for a DB transaction.

Done.

---

go/lib/infra/modules/trust/trustdb/trustdb.go, line 65 at r2 (raw file):

Previously, Oncilla wrote…

It would be useful to also return the version.

Done.

---

go/cert_srv/main.go, line 79 at r1 (raw file):

Previously, Oncilla wrote…

This change should be done in the fix for #2242.

Done

[oncilla]

Reviewed 6 of 6 files at r3.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @lukedirtwalker)

---

go/cert_srv/internal/csconfig/customers.go, line 48 at r3 (raw file):

}

func NewCustomers(trustDB trustdb.TrustDB) *Customers {

I would drop NewCustomers and initialize the struct directly.

Or, if you want to support direct external use, export loadCustomers.

---

go/cert_srv/internal/reiss/handler.go, line 198 at r3 (raw file):

		return nil, err
	}
	if _, err = tx.InsertChain(ctx, chain); err != nil {

If there are 0 rows affected the chain already existed.
This should also throw an error and rollback the transaction.

---

go/cert_srv/internal/reiss/handler.go, line 203 at r3 (raw file):

		return nil, err
	}
	err = tx.Commit()

can be part of if statement.

---

go/lib/infra/modules/trust/trustdb/trustdb.go, line 69 at r3 (raw file):

	// GetAllTRCs fetches all TRCs from the database.
	GetAllTRCs(ctx context.Context) ([]*trc.TRC, error)
	// GetCustKey gets the customer signing key and the version

GetCustKey gets the latest signing key and version for the specified customer AS

---

go/lib/infra/modules/trust/trustdb/trustdbsqlite/db.go, line 96 at r3 (raw file):

	IssuerCertsTable = "IssuerCerts"
	LeafCertsTable   = "LeafCerts"
	CustKeysTable    = "CustKeys"

CustKeysLog missing

---

go/lib/infra/modules/trust/trustdb/trustdbsqlite/db.go, line 494 at r3 (raw file):

}

// GetCustKey gets the customer signing key for the given AS in the latest version.

I would sync the comment with the interface.

[lukedirtwalker]

Reviewable status: 5 of 10 files reviewed, 6 unresolved discussions (waiting on @oncilla)

---

go/cert_srv/internal/csconfig/customers.go, line 48 at r3 (raw file):

Previously, Oncilla wrote…

I would drop NewCustomers and initialize the struct directly.

Or, if you want to support direct external use, export loadCustomers.

Done.

---

go/cert_srv/internal/reiss/handler.go, line 198 at r3 (raw file):

Previously, Oncilla wrote…

If there are 0 rows affected the chain already existed.
This should also throw an error and rollback the transaction.

Done.

---

go/cert_srv/internal/reiss/handler.go, line 203 at r3 (raw file):

Previously, Oncilla wrote…

can be part of if statement.

Done.

---

go/lib/infra/modules/trust/trustdb/trustdb.go, line 69 at r3 (raw file):

Previously, Oncilla wrote…

GetCustKey gets the latest signing key and version for the specified customer AS

Done.

---

go/lib/infra/modules/trust/trustdb/trustdbsqlite/db.go, line 96 at r3 (raw file):

Previously, Oncilla wrote…

CustKeysLog missing

Done.

---

go/lib/infra/modules/trust/trustdb/trustdbsqlite/db.go, line 494 at r3 (raw file):

Previously, Oncilla wrote…

I would sync the comment with the interface.

Done.

[oncilla]

Reviewed 5 of 5 files at r4.
Reviewable status: 9 of 10 files reviewed, 1 unresolved discussion (waiting on @oncilla and @lukedirtwalker)

---

go/cert_srv/internal/reiss/handler.go, line 208 at r4 (raw file):

	if n == 0 {
		tx.Rollback()
		return nil, common.NewBasicError("Chain seems to already be in the DB", nil)

common.NewBasicError("Chain already in DB", nil, "chain", chain)

[lukedirtwalker]

Reviewable status: 9 of 10 files reviewed, 1 unresolved discussion (waiting on @oncilla)

---

go/cert_srv/internal/reiss/handler.go, line 208 at r4 (raw file):

Previously, Oncilla wrote…

common.NewBasicError("Chain already in DB", nil, "chain", chain)

Done.

[oncilla]

Reviewed 2 of 2 files at r5.
Reviewable status: complete! all files reviewed, all discussions resolved