From 233ccaa36af0c932631b7f91cc7c0bb9923cad52 Mon Sep 17 00:00:00 2001 From: chrysn Date: Thu, 12 Dec 2024 14:58:33 +0100 Subject: [PATCH] refactor(coap): Implement dedicated type for the previously default arbitrary policy This completes one part of the transition away from hard-coded settings. --- src/lib/coapcore/src/authorization_server.rs | 40 ++++++++++++-------- src/lib/coapcore/src/scope.rs | 10 +---- src/lib/coapcore/src/seccontext.rs | 22 ++++------- 3 files changed, 34 insertions(+), 38 deletions(-) diff --git a/src/lib/coapcore/src/authorization_server.rs b/src/lib/coapcore/src/authorization_server.rs index 0853e4e47..adb28ec85 100644 --- a/src/lib/coapcore/src/authorization_server.rs +++ b/src/lib/coapcore/src/authorization_server.rs @@ -74,7 +74,7 @@ pub trait AsDescription { /// Type list of authorization servers. Any operation is first tried on the first item, then on the /// second. /// -/// It's convention to have a single A1 and then another chain in A2 or an [`Empty`], but that's +/// It's convention to have a single A1 and then another chain in A2 or an [`DenyAll`], but that's /// mainly becuse that version is easiy to construct pub struct AsChain { a1: A1, @@ -154,31 +154,41 @@ where } /// The empty set of authorization servers. -pub struct Empty; +pub struct DenyAll; -impl AsDescription for Empty { +impl AsDescription for DenyAll { const IS_EMPTY: bool = true; type Scope = core::convert::Infallible; type ScopeGenerator = core::convert::Infallible; } -/// A transition helper -#[derive(Default)] -pub struct GenerateDefault(core::marker::PhantomData); - -impl AsDescription for GenerateDefault { - const IS_EMPTY: bool = true; - - type Scope = Scope; - type ScopeGenerator = Self; +/// A ScopeGenerator that can be used on [`AsDescription`] types that don't process tokens +pub enum NullGenerator { + _Phantom(core::convert::Infallible, core::marker::PhantomData), } -impl crate::scope::ScopeGenerator for GenerateDefault { +impl crate::scope::ScopeGenerator for NullGenerator { type Scope = Scope; fn from_token_scope(self, bytes: &[u8]) -> Result { - Ok(Default::default()) + match self { + NullGenerator::_Phantom(infallible, _) => match infallible {}, + } + } +} + +/// An AS representing unconditionally allowed access, including unencrypted. +pub struct AllowAll; + +impl AsDescription for AllowAll { + const IS_EMPTY: bool = true; + + type Scope = crate::scope::AllowAll; + type ScopeGenerator = NullGenerator; + + fn nosec_authorization(&self) -> Option { + Some(crate::scope::AllowAll) } } @@ -188,7 +198,7 @@ impl AsDescription for GenerateArbitrary { const IS_EMPTY: bool = true; type Scope = crate::scope::AifValue; - type ScopeGenerator = GenerateDefault; + type ScopeGenerator = NullGenerator; fn nosec_authorization(&self) -> Option { use cbor_macro::cbor; diff --git a/src/lib/coapcore/src/scope.rs b/src/lib/coapcore/src/scope.rs index 87c8e012f..010939120 100644 --- a/src/lib/coapcore/src/scope.rs +++ b/src/lib/coapcore/src/scope.rs @@ -41,8 +41,7 @@ impl ScopeGenerator for core::convert::Infallible { #[derive(Debug, Copy, Clone)] pub struct InvalidScope; -// FIXME: Default just needed while GenerateDefault is a thing -#[derive(Debug, defmt::Format, Default)] +#[derive(Debug, defmt::Format)] pub struct AllowAll; impl Scope for AllowAll { @@ -93,13 +92,6 @@ impl TryFrom<&[u8]> for AifValue { } } -// FIXME: Default just needed while GenerateDefault is a thing -impl Default for AifValue { - fn default() -> Self { - AifValue([0; AIF_SCOPE_MAX_LEN]) - } -} - impl Scope for AifValue { fn request_is_allowed(&self, request: &M) -> bool { let code: u8 = request.code().into(); diff --git a/src/lib/coapcore/src/seccontext.rs b/src/lib/coapcore/src/seccontext.rs index 89759265f..9cd965f24 100644 --- a/src/lib/coapcore/src/seccontext.rs +++ b/src/lib/coapcore/src/seccontext.rs @@ -7,7 +7,7 @@ use defmt_or_log::{debug, error, info, Debug2Format}; use crate::authorization_server::AsDescription; -use crate::scope::{AifValue, AllowAll, DenyAll, Scope}; +use crate::scope::Scope; // If this exceeds 47, COwn will need to be extended. const MAX_CONTEXTS: usize = 4; @@ -235,7 +235,7 @@ impl< Crypto: lakers::Crypto, CryptoFactory: Fn() -> Crypto, RNG: rand_core::RngCore + rand_core::CryptoRng, - > OscoreEdhocHandler<'a, H, Crypto, CryptoFactory, crate::authorization_server::Empty, RNG> + > OscoreEdhocHandler<'a, H, Crypto, CryptoFactory, crate::authorization_server::DenyAll, RNG> { /// Create a new CoAP server implementation (a [Handler][coap_handler::Handler]). /// @@ -250,14 +250,14 @@ impl< inner: H, crypto_factory: CryptoFactory, rng: RNG, - ) -> OscoreEdhocHandler<'a, H, Crypto, CryptoFactory, crate::authorization_server::Empty, RNG> + ) -> OscoreEdhocHandler<'a, H, Crypto, CryptoFactory, crate::authorization_server::DenyAll, RNG> { Self { pool: Default::default(), own_identity, inner, crypto_factory, - authorities: crate::authorization_server::Empty, + authorities: crate::authorization_server::DenyAll, rng, } } @@ -269,25 +269,19 @@ impl< Crypto: lakers::Crypto, CryptoFactory: Fn() -> Crypto, RNG: rand_core::RngCore + rand_core::CryptoRng, - > OscoreEdhocHandler<'a, H, Crypto, CryptoFactory, crate::authorization_server::Empty, RNG> + > OscoreEdhocHandler<'a, H, Crypto, CryptoFactory, crate::authorization_server::DenyAll, RNG> { /// Builds a CoAP server that accepts any request without any authentication. pub fn allow_all( self, - ) -> OscoreEdhocHandler< - 'a, - H, - Crypto, - CryptoFactory, - crate::authorization_server::GenerateDefault, - RNG, - > { + ) -> OscoreEdhocHandler<'a, H, Crypto, CryptoFactory, crate::authorization_server::AllowAll, RNG> + { OscoreEdhocHandler { // Starting from DenyAll allows us to diregard any old connections as they couldn't do // anything pool: Default::default(), own_identity: self.own_identity, - authorities: Default::default(), + authorities: crate::authorization_server::AllowAll, inner: self.inner, crypto_factory: self.crypto_factory, rng: self.rng,