Multi tcb info fields validation with derived context #269

hpya93 · 2023-11-25T19:50:59Z

Hi @jhand2
This commit has :

MultiTcb information has field-wise validation (apart from checking structure)
Verify by deriving more child contexts in non-simulation mode and simulation mode.
- Check VendorInfo by validating against target locality
- Check Type by passing Tci type to derived child context
- Check Fwids[n].hashAlg by validating against expected hash alg OID SHA384/ SHA256 based on DPE profile
- Check Fwid[0].Digest by passing a TCI value to child context and checking if reflected in TCI_CURRENT of child TCB info
- Check Fwid[1].Digest by calculating cumulative TCI and cross verification
Aside from above checks, have moved helper functions that operate of certificate and cert extensions to certs.go for better readabilty of certifyKey.go. The helper functions will just return errors and caller must handle/log errors using t.Error()
Check if multitcb info fwidarray length is 1 when extendtci is not supported by DPE profile.
Specification contents

{
	Vendor: "",              // Optional and VendorDefined
	Model: "",               // Optional and VendorDefined
	Version: "",             // Omitted
	SVN: 0,                  // Omitted
	Layer: 0,                // Omitted
	Index: 0,                // Omitted
	Fwids: [ 
		{{Digest : [], HashAlg:OID},   // TCI_CURRENT 
                {Digest : [], HashAlg:OID}],   // TCI_CUMULATIVE  omitted when extend_tci is not supported
	]
	Flags: NotConfigured (0), 	// Omitted
	VendorInfo: [0,0,0,0], 		// TARGET_LOCALITY parameter
	Type: [0,0,0,0]}	       // Vendor defined / input (tci_type) provided by a client to DeriveChild
}

verification/getCertificateChain.go

verification/extendTCI.go

hpya93 · 2023-11-25T20:07:04Z

verification/certs.go

@@ -3,11 +3,116 @@
 package verification


Functions that validate various fields are moved here from certifyKey.go. This is an effort to keep helper functions that are independent of DPEClient instance in certs.go
This was done to reduce the bloating in CertifyKey.go

hpya93 · 2023-11-25T20:08:25Z

verification/certifyKey.go

@@ -455,6 +54,7 @@ func testCertifyKey(d TestDPEInstance, c DPEClient, t *testing.T, simulation boo

 	certifyKeyParams := []CertifyKeyParams{
 		{Label: make([]byte, digestLen), Flags: CertifyKeyFlags(0)},
+		{Label: make([]byte, digestLen), Flags: CertifyKeyFlags(CertifyAddIsCA)},


A new test case to check if IS_CA is set.

hpya93 · 2023-11-25T20:12:24Z

verification/certifyKey.go

+		checkWithDerivedChildContextSimulation(d, c, t, handle)
+	} else {
+		checkWithDerivedChildContext(d, c, t, handle)
+	}


Recommended to have two separate methods to test derived child contexts as clubbing together lead to confusing if-elses.
The simulation mode varies in locality, Derivechild flags, not needing restoration of parent handle etc...

hpya93 · 2023-11-25T20:14:36Z

verification/certifyKey.go

+//   - the "fwid" field must contain cumulative TCI measurement.
+func checkWithDerivedChildContext(d TestDPEInstance, c DPEClient, t *testing.T, handle *ContextHandle) {
+	profile, err := GetTransportProfile(d)
+	if err != nil {


New method after adding DeriveContext support.
Derives child contexzt, preserves & rotates back default handle, focusses on multi tcb info fields validation.

hpya93 · 2023-11-25T20:15:40Z

verification/certifyKey.go

-	keyUsageNames := []string{}
+// Checks Multi Tcb Info for context derived from simulation mode
+func checkWithDerivedChildContextSimulation(d TestDPEInstance, c DPEClient, t *testing.T, handle *ContextHandle) {
+	profile, err := GetTransportProfile(d)


New method after adding DeriveContext support.
Derives child context from simulated context, there is no need to preserve & rotate back parent handle, focusses on multi tcb info fields validation.

hpya93 · 2023-11-25T20:17:30Z

verification/certs.go

+// FWID at index 1 has the TCI_CUMULATIVE as digest
+// The length of FWID array in each DICE TCB information block is 2.
+func validateDiceTcbFwids(leafCertBytes []byte, expectedCurrentTci []byte) error {
+	var leafCert *x509.Certificate


This function cycles through all multi TCB info blocks and validates cumulative measurement, it gets the current TCI values from caller - the most recend node's current TCI value will be added first in this input list.

verification/certs.go

hpya93 · 2023-11-25T20:21:44Z

verification/certs.go

@@ -18,12 +123,12 @@ func getMultiTcbInfo(c *x509.Certificate) (TcgMultiTcbInfo, error) {
 	for _, ext := range c.Extensions {
 		if ext.Id.Equal(OidExtensionTcgDiceMultiTcbInfo) { // OID for Tcg Dice MultiTcbInfo
 			if !ext.Critical {
-				return multiTcbInfo, fmt.Errorf("[ERROR]: TCG DICE MultiTcbInfo extension is not marked as CRITICAL")
+				return multiTcbInfo, fmt.Errorf("multiTcbInfo extension is not marked as CRITICAL")


Changed to smallcase as IDE kept complaining that error message should not start with Upper case, have new lines and have punctuation at the end.

hpya93 · 2023-11-25T20:22:11Z

verification/certs.go

+)
+
+type TcgMultiTcbInfo = []DiceTcbInfo
+


Some required structs and constants

hpya93 · 2023-11-27T06:43:21Z

verification/certifyKey.go

-	binary.Write(flagsBuf, binary.LittleEndian, flags)
+// Checks whether FWID array omits index-1 when extend TCI is not supported in DPE profile.
+func TestCertifyKeyWithoutExtendTciSupport(d TestDPEInstance, c DPEClient, t *testing.T) {
+	simulation := false


New test case to check if FWID[1] is omitted when no extendTCI support in DPE profile

jhand2 · 2023-11-28T01:36:54Z

verification/certs.go

+	for i, tcbinfo := range multiTcbInfo {
+		currentTci := tcbinfo.Fwids[0].Digest
+		cumulativeTci := tcbinfo.Fwids[1].Digest
+		hasher.Reset()
+		hasher.Write(lastCumulativeTCI)
+		hasher.Write(currentTci)
+		expectedCumulativeValue := hasher.Sum(nil)
+		if !bytes.Equal(cumulativeTci, expectedCumulativeValue) {
+			return fmt.Errorf("cumulative TCI value for TCB block-%d, want %v but got %v", i, expectedCumulativeValue, cumulativeTci)
+		}
+		lastCumulativeTCI = cumulativeTci
+	}


TCI_CUMULATIVE isn't a hash of all the currents from TcbInfos in the cert. TCI_CUMULATIVE gets extended whenever the ExtendTCI command is called on a node so for a given TCI Node X, TCI_CUMULATIVE is calculated using all the values input for each call to ExtendTCI(handle=X, input_data=input).

Rectified the issue

hpya93 · 2023-11-28T10:46:38Z

verification/certifyKey.go

+	if !d.GetSupport().ExtendTci {
+		t.Errorf("ExtendTCI is unsupported by profile, unable to run tests to verify TCI_CUMULATIVE measurement")
+		return
+	}


This is because extendTCI when not supported cumulative hash will be omitted from FWID array

hpya93 · 2023-11-28T10:47:26Z

verification/certifyKey.go

+	for i := range childTCI2 {
+		childTCI2[i] = byte(i + 2)
+	}
+


Just to try TCB validation for multiple TCI nodes with different TCI inputs

hpya93 · 2023-11-28T10:47:54Z

verification/certifyKey.go

-	return opts
+	childHandle = &certifiedKey.Handle
+	leafCertBytes := certifiedKey.Certificate
+


Update child handle for cleanup.

hpya93 · 2023-11-28T10:48:24Z

verification/certifyKey.go

+
+	// Build list of tci_current for validation and use it for validating TCI measurements
+	currentTCIs := [][]byte{childTCI2, childTCI1}
+	if err = validateDiceTcbFwids(leafCertBytes, currentTCIs, digestLen); err != nil {


Rectified the issue pointed out.

hpya93 · 2023-11-28T10:51:58Z

verification/certs.go

+	// Check cumulative, current TCI of other indices if any
+	lastCumulativeTCI := defaultTci
+	multiTcbInfo = multiTcbInfo[:lastIndex]
+


Separate validation for the last index as the TCI_CURRENT = TCI_CUMULATIVE = Default TCI.

hpya93 · 2023-11-28T10:52:48Z

verification/certs.go

+		wantCurrentTci := currentTcis[i]
+		verifyDiceTcbDigest(tcbinfo, wantCurrentTci, lastCumulativeTCI)
+	}
+	return err


cross verify the TCI_CUMULATIVE with TCI_CURRENT value passed.

hpya93 · 2023-12-19T17:57:13Z

verification/simulator.go

+			"CertifyKey_TcbValidation",
+			getTestTarget([]string{"AutoInit", "Simulation", "X509", "Csr", "IsCA", "RotateContext", "ExtendTci"}),
+			[]TestCase{DiceTcbValidationTestCase, DiceTcbValidationSimulationTestCase},
+		},


Have added this as separate test target.
As per a comment in #PR250, it was mentioned to move tests that modify DPE tree out of basic testcases.
I addressed it in this PR because it is relevant to CertifyKey command.
If i had to include these tests in "AllTestCases" list, I had to use RetainParent, ChangeLocality(for Sim mode), so that the parent context wouyld be available for further tests.

hpya93 · 2023-12-19T17:58:43Z

verification/verification.go

+	"CheckDiceTcbInfo", TestDiceTcbInfo, []string{"AutoInit", "X509", "IsCA", "RotateContext", "ExtendTci"},
+}
+var DiceTcbValidationSimulationTestCase = TestCase{
+	"CheckDiceTcbInfoInSimulationMode", TestDiceTcbInfoSimulation, []string{"AutoInit", "Simulation", "X509", "IsCA", "RotateContext", "ExtendTci"},


Having DiceTcb Validation as separate test from CertifyKey as it uses DeriveChild command and due to aforementioned reason.

hpya93 · 2023-12-19T18:02:53Z

verification/certifyKey.go

 )

-type TcgMultiTcbInfo = []DiceTcbInfo
-


Moved to certs.go

hpya93 · 2023-12-19T18:03:34Z

verification/certifyKey.go

+func TestDiceTcbInfoSimulation(d TestDPEInstance, c DPEClient, t *testing.T) {
+	testDiceTcbInfo(d, c, t, true)
+}
+


Separate test case to validate DiceTcb with derived contexts in normal mode and Sim mode.

hpya93 · 2023-12-19T18:04:12Z

verification/certifyKey.go

-// unknownExtnMap collects extensions unknown to both x509 and the DICE certificate profiles spec.
-// positive case expects the unknownExtnMap to be empty.
-func removeTcgDiceCriticalExtensions(t *testing.T, certs []*x509.Certificate) {
-	t.Helper()


Moved the utility function to certs.go

hpya93 · 2023-12-19T18:04:20Z

verification/certifyKey.go

-// unknownKeyUsagesMap collects keyusages unknown to both x509 and the DICE certificate profiles spec.
-// positive case expects the unknownKeyUsagesMap to be empty.
-func removeTcgDiceExtendedKeyUsages(t *testing.T, certs []*x509.Certificate) {
-	t.Helper()


Moved the utility function to certs.go