From 44c2f6766c74bc6e4df797952b68128004fba574 Mon Sep 17 00:00:00 2001 From: Matthias Frei Date: Thu, 12 Oct 2023 14:39:07 +0200 Subject: [PATCH] router: feature toggle for experimental SCMP authentication (#4418) The experimental implementation of DRKey-based authentication for SCMP messages is incomplete and, in the current form, not practically useful. Add a feature flag to explicitly opt-in to the experimental SCMP authentication in the router. --- acceptance/router_multi/conf/br.toml | 3 +++ doc/manuals/router.rst | 19 +++++++++++++++---- private/env/features.go | 9 +++++++++ router/cmd/router/main.go | 3 ++- router/dataplane.go | 21 +++++++++++++-------- 5 files changed, 42 insertions(+), 13 deletions(-) diff --git a/acceptance/router_multi/conf/br.toml b/acceptance/router_multi/conf/br.toml index 8c997390e9..5fa757f1c4 100644 --- a/acceptance/router_multi/conf/br.toml +++ b/acceptance/router_multi/conf/br.toml @@ -2,6 +2,9 @@ id = "brA" config_dir = "/share/conf" +[features] + experimental_scmp_authentication = true + [log] [log.console] level = "debug" diff --git a/doc/manuals/router.rst b/doc/manuals/router.rst index 494012ec59..c68768754a 100644 --- a/doc/manuals/router.rst +++ b/doc/manuals/router.rst @@ -100,8 +100,7 @@ Environment Variables .. object:: SCION_TESTING_DRKEY_EPOCH_DURATION For **testing only**. - This option relates to :ref:`DRKey-based authentication of SCMPs ` in the - router, which is **experimental** and currently **incomplete**. + This option relates :option:`features.experimental_scmp_authentication `. Override the global duration for :doc:`/cryptography/drkey` epochs. @@ -113,8 +112,7 @@ Environment Variables .. envvar:: SCION_TESTING_ACCEPTANCE_WINDOW For **testing only**. - This option relates to :ref:`DRKey-based authentication of SCMPs ` in the - router, which is **experimental** and currently **incomplete**. + This option relates :option:`features.experimental_scmp_authentication `. Defines the length of the window around the current time for which SCMP authentication timestamps are accepted. See :ref:`SPAO specification `. @@ -158,6 +156,19 @@ considers the following options. If this is a relative path, it is interpreted as relative to the current working directory of the program (i.e. **not** relative to the location of this .toml configuration file). +.. object:: features + + Features is a container for generic, boolean feature flags (usually for experimental or + transitional features). + + .. option:: features.experimental_scmp_authentication = (Default: false) + + Enable the :ref:`DRKey-based authentication of SCMPs ` in the + router, which is **experimental** and currently **incomplete**. + + When enabled, the router inserts the :ref:`authenticator-option` for SCMP messages. + For now, the MAC is computed based on a dummy key, and consequently is not practically useful. + .. object:: router .. option:: router.receive_buffer_size = (Default: 0) diff --git a/private/env/features.go b/private/env/features.go index cd62fced89..c004ec21d6 100644 --- a/private/env/features.go +++ b/private/env/features.go @@ -36,6 +36,15 @@ type Features struct { // Example: // DanceAtMidnight bool `toml:"dance_at_midnight,omitempty"` + + // ExperimentalSCMPAuthentication enables experimental, DRKey-based + // authentication of SCMP messages. + // + // When enabled, the router inserts the SPAO authenticator for SCMP error messages, + // generated with a dummy key! + // + // Experimental: This field is experimental and will be subject to change. + ExperimentalSCMPAuthentication bool `toml:"experimental_scmp_authentication"` } func (cfg *Features) Sample(dst io.Writer, path config.Path, ctx config.CtxMap) { diff --git a/router/cmd/router/main.go b/router/cmd/router/main.go index c2ecdbdbf6..f458ab2547 100644 --- a/router/cmd/router/main.go +++ b/router/cmd/router/main.go @@ -58,7 +58,8 @@ func realMain(ctx context.Context) error { metrics := router.NewMetrics() dp := &router.Connector{ DataPlane: router.DataPlane{ - Metrics: metrics, + Metrics: metrics, + ExperimentalSCMPAuthentication: globalCfg.Features.ExperimentalSCMPAuthentication, }, ReceiveBufferSize: globalCfg.Router.ReceiveBufferSize, SendBufferSize: globalCfg.Router.SendBufferSize, diff --git a/router/dataplane.go b/router/dataplane.go index 57b533da4a..362c298c34 100644 --- a/router/dataplane.go +++ b/router/dataplane.go @@ -112,6 +112,8 @@ type DataPlane struct { Metrics *Metrics forwardingMetrics map[uint16]forwardingMetrics + ExperimentalSCMPAuthentication bool + // The pool that stores all the packet buffers as described in the design document. See // https://github.com/scionproto/scion/blob/master/doc/dev/design/BorderRouter.rst packetPool chan []byte @@ -2153,14 +2155,17 @@ func (p *slowPathPacketProcessor) prepareSCMP( scmpH := slayers.SCMP{TypeCode: typeCode} scmpH.SetNetworkLayerForChecksum(&scionL) - // Error messages must be authenticated. - // Traceroute are OPTIONALLY authenticated ONLY IF the request - // was authenticated. - // TODO(JordiSubira): Reuse the key computed in p.hasValidAuth - // if SCMPTypeTracerouteReply to create the response. - needsAuth := cause != nil || - (scmpH.TypeCode.Type() == slayers.SCMPTypeTracerouteReply && - p.hasValidAuth(time.Now())) + needsAuth := false + if p.d.ExperimentalSCMPAuthentication { + // Error messages must be authenticated. + // Traceroute are OPTIONALLY authenticated ONLY IF the request + // was authenticated. + // TODO(JordiSubira): Reuse the key computed in p.hasValidAuth + // if SCMPTypeTracerouteReply to create the response. + needsAuth = cause != nil || + (scmpH.TypeCode.Type() == slayers.SCMPTypeTracerouteReply && + p.hasValidAuth(time.Now())) + } var quote []byte if cause != nil {