introduce inter-domain #167

tianyuan129 · 2026-01-05T00:44:43Z

WIP. Descriptions come later.

Copilot

Pull request overview

This pull request introduces inter-domain trust configuration support through CertList validation mechanisms. The changes enable trust anchors to be dynamically established via certificate lists signed by trusted authorities, allowing cross-domain certificate validation without requiring all trust anchors to be pre-configured.

Key changes:

Added CertList encoding/decoding and validation logic for inter-domain trust establishment
Refactored trust validation to support self-signed certificates as potential trust anchors
Split test suite into separate intra-domain and inter-domain test scenarios

Reviewed changes

Copilot reviewed 7 out of 10 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
std/security/trust_config_test_lvs_intra.tlv	New LVS schema test data for intra-domain trust validation scenarios
std/security/trust_config_test_lvs_inter.tlv	New LVS schema test data for inter-domain trust validation scenarios
std/security/trust_config_test.go	Refactored test infrastructure with enhanced fetch function supporting CanBePrefix, added comprehensive inter-domain test suite, and updated fetch count assertions
std/security/trust_config.go	Added CertList cache, implemented self-signed certificate handling via CertList validation, and added PromoteAnchor functionality for dynamic trust anchor establishment
std/security/name_convention.go	Added helper functions for CertList naming conventions and key name extraction from various certificate name formats
std/security/name_convention_test.go	Added comprehensive test coverage for CertList naming and key name extraction functionality
std/security/certificate.go	Implemented CertList encoding/decoding functions for serializing certificate name lists
std/security/certificate_test.go	Added test coverage for CertList encode/decode operations
std/security/cert_cache.go	Implemented CertListCache for storing validated certificate lists with version preference
.gitignore	Added .gocache to ignore list

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-05T05:06:05Z

std/security/trust_config.go

+		return
+	}
+
+	// If already an trust anchor


Grammar error: "an trust anchor" should be "a trust anchor" since "trust" starts with a consonant sound.

Suggested change

// If already an trust anchor

// If already a trust anchor

Copilot · 2026-01-05T05:06:05Z

std/security/trust_config_test.go

 		signer: mAliceSigner,
 	}))
-	require.Equal(t, 4, tcTestFetchCount) // invalid cert not in store
+	require.Equal(t, 6, tcTestFetchCount) // invalid cert not in store (+ CertList)


The comment states "invalid cert not in store (+ CertList)" but the fetch count increases by 2 (from 3 to 6), suggesting 2 additional fetches. This should clarify whether it's 1 invalid cert + 1 CertList, or explain the actual fetch behavior.

Suggested change

require.Equal(t, 6, tcTestFetchCount) // invalid cert not in store (+ CertList)

require.Equal(t, 6, tcTestFetchCount) // invalid cert not in store: 3 additional fetches (including CertList)

Copilot · 2026-01-05T05:06:05Z

std/security/trust_config_test.go

 		signer: mallorySigner,
 	}))
-	require.Equal(t, 6, tcTestFetchCount) // don't bother fetching mallory root because of schema miss
+	require.Equal(t, 8, tcTestFetchCount) // don't bother fetching mallory root because of schema miss (+ CertList)


The comment states "don't bother fetching mallory root because of schema miss (+ CertList)" but the fetch count only increases by 1 (from 7 to 8). If a CertList is being fetched as indicated, the comment should clarify that only the CertList is fetched, or the count should be higher.

Suggested change

require.Equal(t, 8, tcTestFetchCount) // don't bother fetching mallory root because of schema miss (+ CertList)

require.Equal(t, 8, tcTestFetchCount) // only CertList is fetched; mallory root is skipped due to schema miss

Copilot · 2026-01-05T05:06:06Z

std/security/trust_config_test.go

 		signer: mallorySigner,
 	}))
-	require.Equal(t, 8, tcTestFetchCount) // schema hit, fetch 2x mallory certs
+	require.Equal(t, 11, tcTestFetchCount) // schema hit, fetch 2x mallory certs (+ CertList)


The comment states "schema hit, fetch 2x mallory certs (+ CertList)" but the fetch count increases by 3 (from 8 to 11). This suggests 2 certs + 1 CertList = 3 fetches, but the comment should be clearer about this arithmetic.

Suggested change

require.Equal(t, 11, tcTestFetchCount) // schema hit, fetch 2x mallory certs (+ CertList)

require.Equal(t, 11, tcTestFetchCount) // schema hit, 3 fetches: 2x mallory certs + 1x CertList

Copilot · 2026-01-05T05:06:06Z

std/security/trust_config.go

+
+func (tc *TrustConfig) tryListedCerts(args certListArgs, names []enc.Name, idx int) {
+	if idx >= len(names) {
+		args.args.Callback(false, fmt.Errorf("no chain to trusted anchor"))


The error message "no chain to trusted anchor" could be more informative by including details about what was attempted. Consider including the anchor key name or certificate name to help with debugging.

Suggested change

args.args.Callback(false, fmt.Errorf("no chain to trusted anchor"))

args.args.Callback(false, fmt.Errorf("no chain to trusted anchor %s (tried %d certs from CertList)", args.anchorKey, len(names)))

Copilot · 2026-01-05T05:06:07Z

std/security/trust_config_test.go

 		signer: malloryRootSigner,
 	}))
-	require.Equal(t, 5, tcTestFetchCount) // fetch 1x mallory cert
+	require.Equal(t, 7, tcTestFetchCount) // fetch 1x mallory cert (+ CertList)


The comment states "fetch 1x mallory cert (+ CertList)" but the fetch count only increases by 1 (from 6 to 7). This is inconsistent with the pattern established in the previous comments where CertList fetches are counted separately.

Suggested change

require.Equal(t, 7, tcTestFetchCount) // fetch 1x mallory cert (+ CertList)

require.Equal(t, 7, tcTestFetchCount) // fetch 1x mallory cert

Copilot · 2026-01-05T05:06:07Z

std/security/trust_config.go

+	if cached, ok := tc.certListCache.Get(prefix); ok {
+		tc.processCertList(args, cached, nil)
+		return


In the exploreCertList function, if the CertList is found in the cache, processCertList is called with a nil listSigCov. However, processCertList at line 557 checks 'if listSigCov != nil' and only validates the signature if it's non-nil. This means cached CertLists skip signature validation on retrieval, which could be a security concern if the cache is somehow polluted. Consider whether cached items should have already been validated before insertion.

Copilot · 2026-01-05T05:06:07Z

std/security/name_convention.go

+	for i := 0; i+1 < len(name); i++ {
+		comp := name[i]
+		if comp.String() == "KEY" || comp.IsKeyword("KEY") {
+			return name[:i+2], nil


The function checks if a component's string representation equals "KEY" or if it's a keyword "KEY". However, the logic searches for any component that matches "KEY" without ensuring it's at the expected position in the name hierarchy. This could potentially extract an incorrect key name if "KEY" appears in an unexpected position. Consider adding validation that the KEY component is at a reasonable position.

Suggested change

return name[:i+2], nil

// Only accept KEY when it is followed by 1–3 components, which covers

// /<identity>/KEY/<keyid>,

// /<identity>/KEY/<keyid>/<issuer>, and

// /<identity>/KEY/<keyid>/<issuer>/<version>.

remaining := len(name) - i - 1

if remaining >= 1 && remaining <= 3 {

return name[:i+2], nil

}

std/security/cert_cache.go

std/security/certificate.go

zjkmxy · 2026-01-05T19:17:34Z

std/security/certificate.go

+	for _, n := range names {
+		length += len(n.Bytes())
+	}
+	buf := make([]byte, length)


Why make a single buffer? Is there anything wrong to return a Wire?

ret := enc.Wire{} for _, n := range names { ret = append(ret, n.Bytes()) } return ret, nil

zjkmxy · 2026-01-05T19:23:33Z

std/security/name_convention.go

+// that may be a key name, certificate name, or cert name without version.
+func KeyNameFromLocator(name enc.Name) (enc.Name, error) {
+	name = stripImplicitDigest(name)
+	for i := 0; i+1 < len(name); i++ {


Maybe start from the tail is better?

zjkmxy · 2026-01-06T18:18:07Z

std/security/certificate.go

 	Signer ndn.Signer
+	// SignerName sets the KeyLocator of the signature to a specific
+	// name. Optional; defaults to the signer’s key name.
+	SignerName enc.Name


Why don't just make ContextSigner implementing ndn.Signer interface?
The caller is responsible to make the signer contains correct KeyLocator.

Yeah good point

zjkmxy

Accept to unblock since I don't have time to keep monitoring all changes. Please merge when you think it is ready.

tianyuan129 requested a review from a-thieme January 5, 2026 00:45

introduce inter-domain

081aca6

tianyuan129 force-pushed the main branch from cda1b1b to 081aca6 Compare January 5, 2026 01:16

zjkmxy requested a review from Copilot January 5, 2026 05:00

Copilot started reviewing on behalf of zjkmxy January 5, 2026 05:01 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

zjkmxy reviewed Jan 5, 2026

View reviewed changes

optionally override keylocator in signcert

333d8b8

zjkmxy reviewed Jan 6, 2026

View reviewed changes

zjkmxy approved these changes Jan 7, 2026

View reviewed changes

trust_config persist certlist in store

609eda7

	require.Equal(t, 6, tcTestFetchCount) // invalid cert not in store (+ CertList)
	require.Equal(t, 6, tcTestFetchCount) // invalid cert not in store: 3 additional fetches (including CertList)

	require.Equal(t, 8, tcTestFetchCount) // don't bother fetching mallory root because of schema miss (+ CertList)
	require.Equal(t, 8, tcTestFetchCount) // only CertList is fetched; mallory root is skipped due to schema miss

	require.Equal(t, 11, tcTestFetchCount) // schema hit, fetch 2x mallory certs (+ CertList)
	require.Equal(t, 11, tcTestFetchCount) // schema hit, 3 fetches: 2x mallory certs + 1x CertList

	args.args.Callback(false, fmt.Errorf("no chain to trusted anchor"))
	args.args.Callback(false, fmt.Errorf("no chain to trusted anchor %s (tried %d certs from CertList)", args.anchorKey, len(names)))

	require.Equal(t, 7, tcTestFetchCount) // fetch 1x mallory cert (+ CertList)
	require.Equal(t, 7, tcTestFetchCount) // fetch 1x mallory cert

-			return name[:i+2], nil
+			// Only accept KEY when it is followed by 1–3 components, which covers
+			// /<identity>/KEY/<keyid>,
+			// /<identity>/KEY/<keyid>/<issuer>, and
+			// /<identity>/KEY/<keyid>/<issuer>/<version>.
+			remaining := len(name) - i - 1
+			if remaining >= 1 && remaining <= 3 {
+				return name[:i+2], nil
+			}

introduce inter-domain #167

Are you sure you want to change the base?

introduce inter-domain #167

Uh oh!

Conversation

tianyuan129 commented Jan 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zjkmxy Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

zjkmxy Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

zjkmxy Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

tianyuan129 Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

zjkmxy left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants