From d321e186ebbe16dfe835a4673470138c50364fec Mon Sep 17 00:00:00 2001 From: "Akhilesh Kr. Yadav" Date: Thu, 9 Jan 2025 16:25:25 +0530 Subject: [PATCH 1/2] merging-extra-lint Signed-off-by: Akhilesh Kr. Yadav --- Makefile | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index a22b3fd..28ec37d 100644 --- a/Makefile +++ b/Makefile @@ -12,16 +12,10 @@ GOPKG += github.com/veraison/corim/extensions GOLINT ?= golangci-lint -ifeq ($(MAKECMDGOALS),lint) -GOLINT_ARGS ?= run --timeout=3m -else - ifeq ($(MAKECMDGOALS),lint-extra) - GOLINT_ARGS ?= run --timeout=3m --issues-exit-code=0 -E dupl -E gocritic -E gosimple -E lll -E prealloc - endif -endif +GOLINT_ARGS ?= run --timeout=3m -E dupl -E gocritic -E gosimple -E lll -E prealloc -.PHONY: lint lint-extra -lint lint-extra: +.PHONY: lint +lint: $(GOLINT) $(GOLINT_ARGS) ifeq ($(MAKECMDGOALS),test) @@ -50,7 +44,7 @@ presubmit: @echo @echo ">>> Fix any lint error" @echo - $(MAKE) lint-extra + $(MAKE) lint .PHONY: licenses licenses: ; @./scripts/licenses.sh @@ -60,8 +54,7 @@ help: @echo "Available targets:" @echo " * test: run unit tests for $(GOPKG)" @echo " * test-cover: run unit tests and measure coverage for $(GOPKG)" - @echo " * lint: lint sources using default configuration" - @echo " * lint-extra: lint sources using default configuration and some extra checkers" + @echo " * lint: lint sources using default configuration and some extra checkers" @echo " * presubmit: check you are ready to push your local branch to remote" @echo " * help: print this menu" @echo " * licenses: check licenses of dependent packages" From 878bf439c1231fc0254ce7c9cff4c690f99bbfce Mon Sep 17 00:00:00 2001 From: "Akhilesh Kr. Yadav" Date: Mon, 13 Jan 2025 16:24:21 +0530 Subject: [PATCH 2/2] merging-extra-lint Signed-off-by: Akhilesh Kr. Yadav --- comid/instance_test.go | 2 +- comid/measurement.go | 24 +++++++++- comid/measurement_test.go | 97 ++++++++++++++++++++++++++++++++++++++ cots/cas_and_tas_test.go | 10 ++-- cots/cots.go | 2 +- cots/eat_cwtclaims_test.go | 12 ++--- encoding/cbor.go | 6 +-- extensions/README.md | 20 ++++---- 8 files changed, 145 insertions(+), 28 deletions(-) diff --git a/comid/instance_test.go b/comid/instance_test.go index 64ca885..75fc2e5 100644 --- a/comid/instance_test.go +++ b/comid/instance_test.go @@ -108,7 +108,7 @@ func Test_NewBytesInstance_OK(t *testing.T) { instance, err := NewBytesInstance(v) require.NoError(t, err) got := instance.Bytes() - assert.Equal(t, testBytes[:], got) + assert.Equal(t, testBytes, got) } } diff --git a/comid/measurement.go b/comid/measurement.go index 1977735..c64909f 100644 --- a/comid/measurement.go +++ b/comid/measurement.go @@ -424,6 +424,7 @@ func (o Mval) MarshalJSON() ([]byte, error) { } func (o Mval) Valid() error { + // Check if no measurement values are set if o.Ver == nil && o.SVN == nil && o.Digests == nil && @@ -439,28 +440,47 @@ func (o Mval) Valid() error { return fmt.Errorf("no measurement value set") } + // Validate Version if o.Ver != nil { if err := o.Ver.Valid(); err != nil { return err } } + // Validate Digests if o.Digests != nil { if err := o.Digests.Valid(); err != nil { return err } } + // Validate Flags if o.Flags != nil { if err := o.Flags.Valid(); err != nil { return err } } - // raw value and mask have no specific semantics + // Validate MAC Address + if o.MACAddr != nil { + macLen := len(*o.MACAddr) + if macLen != 6 && macLen != 8 { // MAC address must be either 6 or 8 bytes + return fmt.Errorf("invalid MAC address length: expected 6 or 8 bytes, got %d", macLen) + } + } + + // Validate IP Address + if o.IPAddr != nil { + ip := *o.IPAddr + // Must be valid IPv4 or IPv6 (i.e., .To4() != nil or .To16() != nil) + if ip.To4() == nil && ip.To16() == nil { + return fmt.Errorf("invalid IP address: %s", ip.String()) + } + } - // TODO(tho) MAC addr & friends (see https://github.com/veraison/corim/issues/18) + // raw value and raw-value-mask have no specific semantics here + // Validate extensions (custom logic implemented in validMval()) return o.Extensions.validMval(&o) } diff --git a/comid/measurement_test.go b/comid/measurement_test.go index 078c7ee..45505ea 100644 --- a/comid/measurement_test.go +++ b/comid/measurement_test.go @@ -6,6 +6,7 @@ package comid import ( "crypto" "fmt" + "net" "testing" "github.com/stretchr/testify/assert" @@ -601,3 +602,99 @@ func TestMkey_UintMkey(t *testing.T) { assert.NoError(t, err) assert.EqualValues(t, 7, ret) } + +func TestMval_Valid(t *testing.T) { + t.Run("No fields set", func(t *testing.T) { + mval := Mval{} + err := mval.Valid() + assert.EqualError(t, err, "no measurement value set") + }) + + t.Run("All fields nil except Ver, which is valid", func(t *testing.T) { + var scheme swid.VersionScheme + _ = scheme.SetCode(swid.VersionSchemeSemVer) + mval := Mval{ + Ver: &Version{ + Version: "1.0", + Scheme: scheme, + }, + } + err := mval.Valid() + assert.NoError(t, err) + }) + + // Test with valid 6-byte MAC + t.Run("MACAddr valid (6 bytes)", func(t *testing.T) { + mac := MACaddr([]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06}) // EUI-48 + mval := Mval{MACAddr: &mac} + err := mval.Valid() + assert.NoError(t, err, "6-byte MAC should be valid") + }) + + // Test with valid 8-byte MAC + t.Run("MACAddr valid (8 bytes)", func(t *testing.T) { + mac := MACaddr([]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}) // EUI-64 + mval := Mval{MACAddr: &mac} + err := mval.Valid() + assert.NoError(t, err, "8-byte MAC should be valid") + }) + + // Test with invalid MAC length + t.Run("MACAddr invalid (too many bytes)", func(t *testing.T) { + mac := MACaddr([]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}) // 7 bytes + mval := Mval{MACAddr: &mac} + err := mval.Valid() + assert.EqualError(t, err, "invalid MAC address length: expected 6 or 8 bytes, got 7") + }) + + // Test with invalid MAC length + t.Run("MACAddr invalid (too few bytes)", func(t *testing.T) { + mac := MACaddr([]byte{0x01, 0x02, 0x03, 0x04}) // 4 bytes + mval := Mval{MACAddr: &mac} + err := mval.Valid() + assert.EqualError(t, err, "invalid MAC address length: expected 6 or 8 bytes, got 4") + }) + + t.Run("MACAddr valid (6 bytes)", func(t *testing.T) { + mac := MACaddr([]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06}) + mval := Mval{MACAddr: &mac} + err := mval.Valid() + assert.NoError(t, err) + }) + + t.Run("IPAddr valid (IPv4)", func(t *testing.T) { + ip := net.IPv4(192, 168, 1, 100) + mval := Mval{IPAddr: &ip} + err := mval.Valid() + assert.NoError(t, err) + }) + + t.Run("IPAddr valid (IPv6)", func(t *testing.T) { + ip := net.ParseIP("2001:db8::1") + mval := Mval{IPAddr: &ip} + err := mval.Valid() + assert.NoError(t, err) + }) + + t.Run("Digests valid", func(t *testing.T) { + ds := NewDigests() + _ = ds.AddDigest(swid.Sha256, []byte{0xAA, 0xBB}) + mval := Mval{ + Digests: ds, + } + err := mval.Valid() + assert.NoError(t, err) + }) + + t.Run("Extensions valid", func(t *testing.T) { + // Suppose we have some extension data that is considered valid + ext := Extensions{} + mval := Mval{ + Extensions: ext, + // Must also set one non-empty field to pass "no measurement value set" + Ver: &Version{Version: "1.0"}, + } + err := mval.Valid() + assert.NoError(t, err) + }) +} diff --git a/cots/cas_and_tas_test.go b/cots/cas_and_tas_test.go index 46c8ad1..a8380df 100644 --- a/cots/cas_and_tas_test.go +++ b/cots/cas_and_tas_test.go @@ -145,7 +145,7 @@ func TestTrustAnchor_JSON_Roundtrip(t *testing.T) { err := tv2.FromJSON(j) assert.Nil(t, err) - assert.True(t, len(tv2.Data) == len(ta) && 0 == bytes.Compare(ta, tv2.Data), "Compare TA value") + assert.True(t, len(tv2.Data) == len(ta) && bytes.Equal(ta, tv2.Data), "Compare TA value") assert.True(t, TaFormatCertificate == tv2.Format, "Compare TA format") } @@ -166,7 +166,7 @@ func TestTrustAnchor_CBOR_Roundtrip(t *testing.T) { err := tv2.FromCBOR(c) assert.Nil(t, err) - assert.True(t, len(tv2.Data) == len(ta) && 0 == bytes.Compare(ta, tv2.Data), "Compare TA value") + assert.True(t, len(tv2.Data) == len(ta) && bytes.Equal(ta, tv2.Data), "Compare TA value") assert.True(t, TaFormatCertificate == tv2.Format, "Compare TA format") } @@ -203,9 +203,9 @@ func TestTasAndCas_JSON_full_Roundtrip(t *testing.T) { err := tv2.FromJSON(j) assert.Nil(t, err) - assert.True(t, len(tv2.Tas[0].Data) == len(ta) && 0 == bytes.Compare(ta, tv2.Tas[0].Data), "Compare TA value") + assert.True(t, len(tv2.Tas[0].Data) == len(ta) && bytes.Equal(ta, tv2.Tas[0].Data), "Compare TA value") assert.True(t, TaFormatCertificate == tv2.Tas[0].Format, "Compare TA format") - assert.True(t, len(tv2.Cas[0]) == len(ca) && 0 == bytes.Compare(ca, tv2.Cas[0]), "Compare CA") + assert.True(t, len(tv2.Cas[0]) == len(ca) && bytes.Equal(ca, tv2.Cas[0]), "Compare CA") } func TestTasAndCas_CBOR_full_Roundtrip(t *testing.T) { @@ -221,7 +221,7 @@ func TestTasAndCas_CBOR_full_Roundtrip(t *testing.T) { err := tv2.FromCBOR(c) assert.Nil(t, err) - assert.True(t, len(tv2.Tas[0].Data) == len(ta) && 0 == bytes.Compare(ta, tv2.Tas[0].Data), "Compare TA value") + assert.True(t, len(tv2.Tas[0].Data) == len(ta) && bytes.Equal(ta, tv2.Tas[0].Data), "Compare TA value") assert.True(t, TaFormatCertificate == tv2.Tas[0].Format, "Compare TA format") assert.True(t, TaFormatCertificate == tv2.Tas[0].Format, "Compare TA format") } diff --git a/cots/cots.go b/cots/cots.go index 412bc18..6aa0cee 100644 --- a/cots/cots.go +++ b/cots/cots.go @@ -34,7 +34,7 @@ func (o *ConciseTaStore) SetTagIdentity(tagID interface{}, tagIDVersion *uint) * } o.TagIdentity = &comid.TagIdentity{} o.TagIdentity.TagID = *id - if nil != tagIDVersion { + if tagIDVersion != nil { o.TagIdentity.TagVersion = *tagIDVersion } } diff --git a/cots/eat_cwtclaims_test.go b/cots/eat_cwtclaims_test.go index 6b4b6fc..151ee35 100644 --- a/cots/eat_cwtclaims_test.go +++ b/cots/eat_cwtclaims_test.go @@ -67,7 +67,7 @@ func getNonce() eat.Nonce { return nonce } -func cborRoundTripper(t *testing.T, tv EatCWTClaim, expected []byte) { +func cborRoundTripper(t *testing.T, tv *EatCWTClaim, expected []byte) { data, err := tv.ToCBOR() t.Logf("CBOR: %x", data) @@ -79,10 +79,10 @@ func cborRoundTripper(t *testing.T, tv EatCWTClaim, expected []byte) { err = actual.FromCBOR(data) assert.Nil(t, err) - assert.Equal(t, tv, actual) + assert.Equal(t, *tv, actual) } -func jsonRoundTripper(t *testing.T, tv EatCWTClaim, expected string) { +func jsonRoundTripper(t *testing.T, tv *EatCWTClaim, expected string) { data, err := tv.ToJSON() t.Logf("JSON: '%s'", string(data)) @@ -94,7 +94,7 @@ func jsonRoundTripper(t *testing.T, tv EatCWTClaim, expected string) { err = actual.FromJSON(data) assert.Nil(t, err) - assert.Equal(t, tv, actual) + assert.Equal(t, *tv, actual) } func TestEatCWTClaim_Full_RoundtripCBOR(t *testing.T) { @@ -116,7 +116,7 @@ func TestEatCWTClaim_Full_RoundtripCBOR(t *testing.T) { 0xff, 0xff, 0xff, 0xff, 0xff, } - cborRoundTripper(t, tv, expected) + cborRoundTripper(t, &tv, expected) } func TestEatCWTClaim_Full_RoundtripJSON(t *testing.T) { @@ -143,7 +143,7 @@ func TestEatCWTClaim_Full_RoundtripJSON(t *testing.T) { "iat": 0, "cti": "////////" }` - jsonRoundTripper(t, tv, expected) + jsonRoundTripper(t, &tv, expected) } func TestEatCWTClaims_Valid_empty_list(t *testing.T) { diff --git a/encoding/cbor.go b/encoding/cbor.go index 74656bd..afe76fe 100644 --- a/encoding/cbor.go +++ b/encoding/cbor.go @@ -261,13 +261,13 @@ func (o *structFieldsCBOR) ToCBOR(em cbor.EncMode) ([]byte, error) { header |= byte(25) out = append(out, header) out = binary.BigEndian.AppendUint16(out, uint16(mapLen)) - } else { + } else if mapLen <= math.MaxUint32 { header |= byte(26) out = append(out, header) out = binary.BigEndian.AppendUint32(out, uint32(mapLen)) + } else { + return nil, errors.New("mapLen cannot exceed math.MaxUint32") } - // Since len() returns an int, the value cannot exceed MaxUint32, so - // the 8-byte length variant cannot occur. for _, key := range o.Keys { marshalledKey, err := em.Marshal(key) diff --git a/extensions/README.md b/extensions/README.md index 10a4e82..8087725 100644 --- a/extensions/README.md +++ b/extensions/README.md @@ -50,16 +50,16 @@ effectively defining new fields for the corresponding structures. In the code base, these can be identified by the embedded `Extensions` struct. Each extensible type has a corresponding `extensions.Point`. These are: -| type | extension point | -| --------------------- | ------------------------------------------------------------- | -| `comid.Comid` | `comid.ExtComid` | -| `comid.Entity` | `comid.ExtEntity` | -| `comid.FlagsMap` | `comid.ExtReferenceValueFlags`, `comid.ExtEndorsedValueFlags` | -| `comid.Mval` | `comid.ExtReferenceValue`, `comid.ExtEndorsedValue` | -| `comid.Triples` | `comid.ExtTriples` | -| `corim.Entity` | `corim.ExtEntity` | -| `corim.Signer` | `corim.ExtSigner` | -| `corim.UnsignedCorim` | `corim.ExtUnsignedCorim` | +| Extended Type | Extension Point(s) | Parent Structure | Where to Call RegisterExtensions() | +|:-------------------:|:-------------------------------------------------------------------------:|:----------------------------------------------------:|:-------------------------------------------------------------------------:| +| comid.Comid | comid.ExtComid | comid.Comid (the top-level CoMID) | On a comid.Comid instance (e.g. myComid.RegisterExtensions(extMap)) | +| comid.Entity | comid.ExtEntity | comid.Entity | Usually indirect via myComid.RegisterExtensions(...) (the Comid sees it). | +| comid.Triples | comid.ExtTriples | comid.Triples | Typically indirect via myComid.RegisterExtensions(...). | +| comid.Mval | comid.ExtReferenceValue, comid.ExtEndorsedValue, comid.ExtMval | comid.Mval (measurement-value in reference/endorsed) | Usually indirect via myComid.RegisterExtensions(...). | +| comid.FlagsMap | comid.ExtReferenceValueFlags, comid.ExtEndorsedValueFlags, comid.ExtFlags | comid.FlagsMap | Typically indirect via myComid.RegisterExtensions(...). | +| corim.UnsignedCorim | corim.ExtUnsignedCorim | corim.UnsignedCorim (the top-level CoRIM) | On a corim.UnsignedCorim instance (e.g. myCorim.RegisterExtensions(...)) | +| corim.Entity | corim.ExtEntity | corim.Entity | Usually indirect via myCorim.RegisterExtensions(...). | +| corim.Signer | corim.ExtSigner | corim.Signer | Usually indirect via myCorim.RegisterExtensions(...). | Note that `comid.Mval` and `comid.FlagsMap` are used for both reference values and endorsed values, which may be extended separately. This is why there are