daemon: refactoring to enable applications running in-library self-contained daemon logic (a.k.a standalone mode)

[rolbk]

Summary

This PR allows applications to run without a SCION daemon by communicating with the control service directly and running daemon functionality in-process.

Main Changes

1. Split daemon server: Separated the existing daemon gRPC server into gRPC (DaemonServer) and core logic (DaemonEngine) components. The engine contains the business logic, independent of transport.

2. Move engine to pkg: Moved the DaemonEngine into pkg/daemon/private/engine to make it accessible for in-process usage.

3. Implement standalone connector: Added NewStandaloneConnector() to create a Connector with in-process pathdb, trust verification, revocation cache, etc.

4. Add DefaultConnector helper: Implemented NewDefaultConnector() that selects between standalone and remote daemon based on availability:

   • Priority 1: Use supplied daemon address (gRPC)
   • Priority 2: Load from topology file if exists (standalone)
   • Priority 3: Connect to default daemon if reachable (gRPC)
   • Can be extended in the future with bootstrapping functionality.

5. Update scion-cli: Adapted the CLI to use the new connector system via --sciond flag. If supplied, a remote daemon is used, by default the standalone is used.

Testing

Added daemon_vs_standalone acceptance test that runs e2e integration test with both modes. The test are already integrated in the CI/CD AT: daemon_vs_standalone_daemon and AT: daemon_vs_standalone_standalone.

One can alternatively run both test using the Bazel framework provided that the local development is installed (https://docs.scion.org/en/latest/dev/setup.html#setup). The commands are respectively:

bazel test --test_output=streamed //acceptance/daemon_vs_standalone:test_ping
bazel test --test_output=streamed //acceptance/daemon_vs_standalone:test_standalone

Testing scion utility in your SCION environment

If not having or not wanting to use the development setup, one can also directly try out the standalone feature by compiling the scion utility. The scion utility is an endhost CLI tool that can ping or retrieve SCION path when run in an endhost connected to a SCION network (production network, local setup, container network, etc.).

To compile the tool, check out to this PR and:

CGO_ENABLED=0 go build -o bin/ ./scion/cmd/...

For example, if you want to try out the feature in the production network, you can try to fetch paths to the ETHZ AS, as follows.

bin/scion sp 64-2:0:9

The binary needs a valid topology.json file. By default it will try to find it under /etc/scion/topology.json if defined elsewhere please, specify the parent directory with the --config flag.

Follow-up Tasks (not considered in this PR)

• Bootstrapping functonality: The NewAutoConnector() function should implement options to automatically bootstrap SCION connectivity from bootstrap servers / DNS records similar to JPAN.
• DRKey in Standalone Mode: The NewStandaloneConnector() should set up and manage a DRKeyEngine appropriately configured for standalone mode. Currently DRKey is explicitly disabled by setting the engine to nil.

Open Discussion Points

Naming/API

• Rename old Service? (JordiSubira): Should daemon.NewService() be renamed to daemon.NewRemoteService() for symmetry with NewStandaloneConnector(), breaking the API?
• LocalASInfo naming: Is the name LocalASInfo (formerly CPInfo and Topology) now ok?
• AcceptAllVerifier naming (marcfrei): Should verifier.AcceptAll be renamed to AcceptAllVerifier, TrustAllVerifier, or NoOpVerifier?
• SuppliedOption naming (marcfrei): Should SuppliedOption be renamed? Options: DefaultOption, connectorOption. Should it be exported? Should it match the pattern of standaloneOption?

Logic/Design

• DefaultConnector logic: Is the priority order correct? (1. supplied daemon addr -> 2. topology file -> 3. default daemon)

Code Organization

• API types location (marcfrei): Should pkg/daemon/private/types move to pkg/daemon/types, breaking the API?
• drkey grpc location (marcfrei): Should pkg/daemon/private/drkey move to private/drkey?
• Public API surface: Is the current pkg/daemon API minimal and appropriate?

Documentation

• README: Create a README for pkg/daemon documenting the public API and usage patterns (once other points resolved)

[JordiSubira]

Tackling some questions in the description:

RevNotification handling: RevNotification should of course trigger NotifyInterfaceDown, but should NotifyInterfaceDown also trigger RevNotification, or should this be handled differently?

I do not completely undestand. So far, applications used RevNotification() "reactively" after receiving a "PathDown" SCMP message. The API factory internally used the daemon gRPC endpoint NotifyInterfaceDown() to inform the daemon service about it. Then the server added this information on the RevCache. If we keep the same definition of daemon.Connector, how can it be triggered a NotificationDown without a RevNotification() ?

In addition to running the Acceptance Tests, I spun up a fresh Kathara setup with the new binaries and no caches to see if I could ping a neighbor, which works without issues.

If I am not wrong, you have not changed the acceptance test setup, right? This means that the acceptance/integration test keep running with daemons for all cases right? I would at least create a new acceptance test that runs an end2end_integration test with standalone client, or even better, making this the default behavior and having one test with "non-standalone" clients and the rest runs without daemons. At the end, we are moving to this deployment model in most scenarios.

[rolbk]

RevNotification handling: RevNotification should of course trigger NotifyInterfaceDown, but should NotifyInterfaceDown also trigger RevNotification, or should this be handled differently?

After putting the mentioned functions under a bit more scrutiny, I think that the splitting (the first commit of this branch) was performed correctly for this functionality.
However, there is one semantic difference, of which I'm not sure of the consequences: The path_mgmt.RevInfo that is inserted into the RevCache might have its LinkType set as LinkType_unset instead of LinkType_core, due to the following:

The existing daemon grpc stub contstructs a NotifyInterfaceDownRequestfrom the path_mgmt.RevInfo, which only contains IfID and IsdAs:

	_, err := client.NotifyInterfaceDown(ctx, &sdpb.NotifyInterfaceDownRequest{
		Id:    uint64(revInfo.IfID),
		IsdAs: uint64(revInfo.RawIsdas),
	})

The daemon then reconstructs the path_mgmt.RevInfo and just statically sets the LinkType to LinkType_core:

	revInfo := &path_mgmt.RevInfo{
		RawIsdas:     addr.IA(req.IsdAs),
		IfID:         iface.ID(req.Id),
		LinkType:     proto.LinkType_core,
		RawTTL:       10,
		RawTimestamp: util.TimeToSecs(time.Now()),
	}
	_, err := s.RevCache.Insert(ctx, revInfo)

However, if the new backend is plugged in as the connector direcly, this replacement never happens, and the path_mgmt.RevInfo constructed by the App (here the SCMP handler in `scmp.go) is inserted directly:

		// no LinkType is set here!
		return h.handleSCMPRev(typeCode, &path_mgmt.RevInfo{
			IfID:         iface.ID(msg.Interface),
			RawIsdas:     msg.IA,
			RawTimestamp: util.TimeToSecs(time.Now()),
			RawTTL:       10,
		})

Im not sure of the consequences of this and would love some additional thoughts on this.

[lukedirtwalker]

Btw force pushing is really not great if we use github review... It loses all context in that case.

[marcfrei]

The diff is currently unnecessary large since it's not up to date with master and because of many unnecessary format changes. Is there an easy way to revert those or to split them out into a separate PR?

[rolbk]

The diff is currently unnecessary large since it's not up to date with master

I will of course rebase before merging.

and because of many unnecessary format changes.

What changes do you mean concretely? I always try to not change anything unnecessary. For the few whitespace changes, these were mostly needed due to lines reaching the 100 char mark after renaming types, failing the linter test.

[marcfrei]

I will of course rebase before merging.

Continuously rebasing, not just before merging, would help with reviewing the PR.

What changes do you mean concretely?

Looking for example at scion/cmd/scion/ping.go, we have many changes like from

				opts = append(opts, path.WithProbing(&path.ProbeConfig{
					LocalIA: topo.LocalIA,
					LocalIP: localIP,
				}))

to

				opts = append(
					opts, path.WithProbing(
						&path.ProbeConfig{
							LocalIA: topo.LocalIA,
							LocalIP: localIP,
						},
					),
				)

[rolbk]

Happy New Year! 🎉

After further discussing the old split of the gRPC and backend/DaemonEngine, we came to the conclusion that it's actually a bad idea to have the engine conform to Connector. So I reset the gRPC server to master and re-split it into gRPC and engine - and I have to say you were right, it works out quite a bit nicer without trying to force it into Connector.

The StandaloneDaemon now serves a similar purpose as the gRPC server: being an adapter on top of the engine and implementing the Connector interface. It also handles closing all the closable resources.

TODO: Fixing/closing the topology.Loader is still pending. Changed it to only load once

[JordiSubira]

RevNotification handling: RevNotification should of course trigger NotifyInterfaceDown, but should NotifyInterfaceDown also trigger RevNotification, or should this be handled differently?

After putting the mentioned functions under a bit more scrutiny, I think that the splitting (the first commit of this branch) was performed correctly for this functionality. However, there is one semantic difference, of which I'm not sure of the consequences: The path_mgmt.RevInfo that is inserted into the RevCache might have its LinkType set as LinkType_unset instead of LinkType_core, due to the following:

The existing daemon grpc stub contstructs a NotifyInterfaceDownRequestfrom the path_mgmt.RevInfo, which only contains IfID and IsdAs:

	_, err := client.NotifyInterfaceDown(ctx, &sdpb.NotifyInterfaceDownRequest{
		Id:    uint64(revInfo.IfID),
		IsdAs: uint64(revInfo.RawIsdas),
	})

The daemon then reconstructs the path_mgmt.RevInfo and just statically sets the LinkType to LinkType_core:

	revInfo := &path_mgmt.RevInfo{
		RawIsdas:     addr.IA(req.IsdAs),
		IfID:         iface.ID(req.Id),
		LinkType:     proto.LinkType_core,
		RawTTL:       10,
		RawTimestamp: util.TimeToSecs(time.Now()),
	}
	_, err := s.RevCache.Insert(ctx, revInfo)

However, if the new backend is plugged in as the connector direcly, this replacement never happens, and the path_mgmt.RevInfo constructed by the App (here the SCMP handler in `scmp.go) is inserted directly:

		// no LinkType is set here!
		return h.handleSCMPRev(typeCode, &path_mgmt.RevInfo{
			IfID:         iface.ID(msg.Interface),
			RawIsdas:     msg.IA,
			RawTimestamp: util.TimeToSecs(time.Now()),
			RawTTL:       10,
		})

Im not sure of the consequences of this and would love some additional thoughts on this.

I guess after refactoring to only use the Connector as the Client Abstraction the behavior is consistent with what it used to be, right?

[rolbk]

@marcfrei

The diff is currently unnecessary large since it's not up to date with master and because of many unnecessary format changes. Is there an easy way to revert those or to split them out into a separate PR?

Found the culprit! While troubleshooting the CI linter failures, I accidentally enabled an auto-formatting option in my IDE that went rogue without me noticing.

I've reverted all the unnecessary changes I could find. Let me know if you spot any others I missed.

Apologies for the noise.

[romshark]

LGTM, aside from the few nitpicks.
my only concern was #4853 (comment)

P.S. I'm not too familiar with the daemon at this point.

[jeltevanbommel]

Suggested change

	{Src: toWildCard(dst), Dst: dst, SegType: Core},
	{Src: toWildCard(src), Dst: dst, SegType: Core},

Seems this is wrong.

This seg request is used when the source is not a core AS, and the destination IS a core AS, we then need Up segments to our local ISD's core AS and a core segments from the source ISD to the destination AS.
The current code will get core segments from the source ISD to the desination ISD. These segments will likely anyways contain the destination AS, as it is a core AS, but technically does not have to, so I think that request should be made explicit.

Requesting core segments from the destination ISD's core ASes to the destination core AS does not make a lot of sense.

[rolbk]

does this still apply on the current HEAD?

	if s.Inspector == nil { // In case inspector is not set, fall back to basic splitting
		if srcCore {
			return Requests{
				{SegType: Down, Src: src, Dst: dst},
				{SegType: Core, Src: src, Dst: dst},
				{SegType: Core, Src: src, Dst: toWildCard(dst)},
				{SegType: Down, Src: toWildCard(dst), Dst: dst},
			}, nil
		}
		reqs := Requests{
			{SegType: Up, Src: src, Dst: toWildCard(src)},
			{SegType: Core, Src: toWildCard(src), Dst: toWildCard(dst)},
			{SegType: Core, Src: toWildCard(dst), Dst: dst},
			{SegType: Down, Src: toWildCard(dst), Dst: dst},
		}
		if src.ISD() == dst.ISD() && dst.IsWildcard() {
			reqs = append(reqs, Request{SegType: Up, Src: src, Dst: dst})
		}
		return reqs, nil
	}

EDIT:
does this fix it?

	if s.Inspector == nil { // In case inspector is not set, fall back to basic splitting
		if srcCore {
			return Requests{
				{SegType: Down, Src: src, Dst: dst},
				{SegType: Core, Src: src, Dst: dst},
				{SegType: Core, Src: src, Dst: toWildCard(dst)},
				{SegType: Down, Src: toWildCard(dst), Dst: dst},
			}, nil
		}
		reqs := Requests{
			{SegType: Up, Src: src, Dst: toWildCard(src)},
			{SegType: Core, Src: toWildCard(src), Dst: toWildCard(dst)},
			{SegType: Core, Src: toWildCard(src), Dst: dst},
			{SegType: Down, Src: toWildCard(dst), Dst: dst},
		}
		if src.ISD() == dst.ISD() && dst.IsWildcard() {
			reqs = append(reqs, Request{SegType: Up, Src: src, Dst: dst})
		}
		return reqs, nil
	}

committed

[JordiSubira]

Can you (@jeltevanbommel ) also confirm that:

{SegType: Up, Src: src, Dst: toWildCard(src)},

and

if src.ISD() == dst.ISD() && dst.IsWildcard() {
			reqs = append(reqs, Request{SegType: Up, Src: src, Dst: dst})
}

are redundant? If affirmative, we can remove the latter. This would also simplify the readability (related to Roman's comment)

[jeltevanbommel]

Yes, they should be redundant.

[tzaeschke]

@jeltevanbommel The SCION book suggests using at most three queries, one UP, one CORE and one DOWN.
Should we add some documentation here why four (or more?) requests are made?

I suspect that it has to do with a query returning a maximum of 20 segments, so the CORE(wildcard(src), dst) query is necessary because CORE(wildcard(src), wildcard(dst)) may not return any segments that end at dst if dst is a core?

[jeltevanbommel]

In the regular Daemon implementation for path lookup we rely on having the TRCs to determine which ASes are core and which are not. We can then optimize the query to the path server as then we'll know whether we need up or down segments.

In the daemonless case, we do not want to rely on the TRCs, as it's an extra storage requirement (or requires fetching before we can do the path lookup) and hence we look up as if the destination is both a core AS and not a core AS, meaning you have to do an extra requests.

Example: Assuming we are not a core AS. If the destination is not a core AS, you'll need a core path from src ISD to dst ISD, and then a down path from dst ISD wildcard to dst AS => {CORE, wildcard(src), wildcard(dst)}, {DOWN, wildcard(dst), dst}
If the destination is a core AS, you'll need a core path from src ISD to dst ISD,AS. So you'll request {CORE, wildcard(src), dst}.
We cannot assume anything, so we'll request both cases, and let the combinator figure it out:
{CORE, wildcard(src), wildcard(dst)}, {DOWN, wildcard(dst), dst}, {CORE, wildcard(src), dst}.

Maybe adding this as some docs is good indeed.

[tzaeschke]

We cannot assume anything, so we'll request both cases, and let the combinator figure it out:
{CORE, wildcard(src), wildcard(dst)}, {DOWN, wildcard(dst), dst}, {CORE, wildcard(src), dst}.

But if we didn't have the 20 segment limit, then the CORE(wildcard, wildcard) query would always contain the correct CORE segment, no need to ask twice? Also, if our dst-AS is one of the endpoints in the CORE segments, then it must be a core AS, no need to ask the TRC?

[jeltevanbommel]

Yes, if the path server would return all path segments then we would not need to ask twice. And yes we can, based on the replies we get, also determine whether we would need any subsequent requests (i.e. not have to request the DOWN segment if we see the AS as a CORE AS). However, that is not how the current system is implemented and changing that would require a lot more changes. At that point I think it'd be better to rethink this in general, and for instance consider a mode at the path server where one can just provide src,dst and the path server handles this splitting.

[tzaeschke]

At that point I think it'd be better to rethink this in general, and for instance consider a mode at the path server where one can just provide src,dst and the path server handles this splitting.

That would be the new endhost API :-)

[rolbk]

So, given that there are no more open points, can I now rebase it into a clean git history? Or do you want to keep it as it is?