TrustStore: Inserter handles chains

[oncilla]

This PR adds the capability to handle certificate inserts to the inserter.

Fixes #3472

---

This change is 

[oncilla]

Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)

---

go/lib/infra/modules/trust/v2/main_test.go, line 121 at r2 (raw file):

}

func loadChain(t *testing.T, desc ChainDesc) decoded.Chain {

add help message to all require statements

[scrye]

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

---

go/lib/infra/modules/trust/v2/inserter.go, line 147 at r2 (raw file):

	if found, err := ins.db.TRCExists(ctx, decTRC); err != nil || found {
		return false, err

I think this changes the semantics of shouldInsertTRC.

Previously, we had:

true, nil -> return false, nil
true, error -> return false, error
false, nil -> continue
false, error -> return true, error

Now, we have:

true, nil -> return false, nil
true, error -> return false, error
false, nil -> continue
false, error -> return false (!), error

Is it intentional?

---

go/lib/infra/modules/trust/v2/inserter.go, line 199 at r2 (raw file):

		return false, serrors.WrapStr("error validating the certificate chain", err)
	}
	t, err := trcProvider(ctx, chain.Issuer.Subject.I, chain.Issuer.Issuer.TRCVersion)

That Issuer.Issuer does not read nicely. Can one of the fields be renamed to something more appropriate?

[lukedirtwalker]

Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on @oncilla and @scrye)

---

go/lib/infra/modules/trust/v2/inserter.go, line 147 at r2 (raw file):

Previously, scrye (Sergiu Costea) wrote…

I think this changes the semantics of shouldInsertTRC.

Previously, we had:

true, nil -> return false, nil
true, error -> return false, error
false, nil -> continue
false, error -> return true, error

Now, we have:

true, nil -> return false, nil
true, error -> return false, error
false, nil -> continue
false, error -> return false (!), error

Is it intentional?

I think it makes sense to always return default values on errors. Therefore I changed it.

---

go/lib/infra/modules/trust/v2/inserter.go, line 199 at r2 (raw file):

Previously, scrye (Sergiu Costea) wrote…

That Issuer.Issuer does not read nicely. Can one of the fields be renamed to something more appropriate?

hm that is kind of hard wired into design already.
https://github.com/scionproto/scion/blob/master/doc/ControlPlanePKI.md#certificate-section-issuer
I think if we want to change it certainly not in this PR.

---

go/lib/infra/modules/trust/v2/main_test.go, line 121 at r2 (raw file):

Previously, Oncilla wrote…

add help message to all require statements

Done.

[oncilla]

Reviewed 2 of 3 files at r4.
Reviewable status: 3 of 4 files reviewed, 2 unresolved discussions (waiting on @scrye)

---

go/lib/infra/modules/trust/v2/inserter.go, line 199 at r2 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

hm that is kind of hard wired into design already.
https://github.com/scionproto/scion/blob/master/doc/ControlPlanePKI.md#certificate-section-issuer
I think if we want to change it certainly not in this PR.

Agreed, it's not nice to read.
But in the end it is the issuer of an issuer certificate, so 🤷‍♂️

[oncilla]

Reviewable status: 3 of 4 files reviewed, 2 unresolved discussions (waiting on @scrye)

[scrye]

Reviewed 1 of 3 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved

[scrye]

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

[oncilla]

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