Skip to content

Commit

Permalink
refactor(coap): Use demt-or-log instead of explicitly passed string w…
Browse files Browse the repository at this point in the history 
…riter
  • Loading branch information
@chrysn
chrysn committed Oct 29, 2024
1 parent f897afe commit ec6d0b4
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 47 deletions.
25 changes: 25 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions src/lib/coapcore/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,10 @@ liboscore-msgbackend = { git = "https://gitlab.com/oscore/liboscore/", features
], rev = "447e34fc5af427de7513f96a756babfe2f0b2c5c" }
minicbor = "0.23.0"
heapless = "0.8.0"
defmt-or-log = { version = "0.2.1", default-features = false }
defmt = { workspace = true, optional = true }
log = { version = "0.4", optional = true }

[features]
defmt = ["defmt-or-log/defmt", "dep:defmt"]
log = ["defmt-or-log/log", "dep:log"]
17 changes: 17 additions & 0 deletions src/lib/coapcore/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,23 @@
//! the CoAP server itself, but merely a middleware. (Providing the full CoAP will be a requirement
//! for at least as long as the OSCORE component is tightly coupled to a particular implementation
//! of [`coap-message`]).
//!
//! ## Logging
//!
//! Extensive logging is available in this crate through [`defmt_or_log`], depending on features
//! enabled.
//!
//! Errors from CoAP are currently logged through its [`Debug2Format`](defmt_or_log::Debug2Format)
//! facility, representing a compromise between development and runtime complexity. Should
//! benchmarks show this to be a significant factor in code size in applications that need error
//! handling, more fine grained control can be implemented (eg. offering an option to make
//! Debug2Format merely print the type name or even make it empty).
//!
//! See the book for [how defmt is configured in
//! RIOT-rs](https://future-proof-iot.github.io/RIOT-rs/dev/docs/book/tooling/defmt.html).
//!
//! **Warning**: At the Debug level, this module may show cryptographic key material. This will be
//! revised once all components .
#![no_std]

// Might warrant a standalone crate at some point
Expand Down
93 changes: 55 additions & 38 deletions src/lib/coapcore/src/seccontext.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use coap_message::{
};
use coap_message_utils::{Error as CoAPError, OptionsExt as _};
use core::fmt::Write;
use defmt_or_log::{debug, error, info, warn, Debug2Format};

// If this exceeds 47, COwn will need to be extended.
const MAX_CONTEXTS: usize = 4;
Expand All @@ -21,6 +22,7 @@ pub type SecContextPool<Crypto> =
/// This type represents any of the 48 efficient identifiers that use CBOR one-byte integer
/// encodings (see RFC9528 Section 3.3.2), or equivalently the 1-byte long OSCORE identifiers
// FIXME Could even limit to positive values if MAX_CONTEXTS < 24
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
#[derive(Copy, Clone, PartialEq, Debug)]
struct COwn(u8);

Expand Down Expand Up @@ -82,6 +84,7 @@ impl Into<lakers::ConnId> for COwn {
/// FIXME: At the moment, this always represents an authorization that allows everything, and only
/// has runtime information about whether or not stdout is allowed. On the long run, this will
/// likely be a CBOR item with pre-verified structure.
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
#[derive(Debug)]
struct AifStaticRest {
may_use_stdout: bool,
Expand Down Expand Up @@ -111,6 +114,7 @@ impl AifStaticRest {
}
}

#[cfg_attr(feature = "defmt", derive(defmt::Format))]
#[derive(Debug)]
struct SecContextState<Crypto: lakers::Crypto> {
// FIXME: Should also include timeout. How do? Store expiry, do raytime in not-even-RTC mode,
Expand Down Expand Up @@ -163,6 +167,26 @@ enum SecContextStage<Crypto: lakers::Crypto> {
Oscore(liboscore::PrimitiveContext),
}

#[cfg(feature = "defmt")]
impl<Crypto: lakers::Crypto> defmt::Format for SecContextStage<Crypto> {
fn format(&self, f: defmt::Formatter) {
match self {
SecContextStage::Empty => defmt::write!(f, "Empty"),
SecContextStage::EdhocResponderProcessedM1 { c_r, .. } => {
defmt::write!(f, "EdhocResponderProcessedM1 {{ c_r: {:?}, ... }}", c_r)
}
SecContextStage::EdhocResponderSentM2 { c_r, .. } => {
defmt::write!(f, "EdhocResponderSentM2 {{ c_r: {:?}, ... }}", c_r)
}
SecContextStage::Oscore(primitive_context) => defmt::write!(
f,
"Oscore(with recipient_id {:?})",
primitive_context.recipient_id()
),
}
}
}

impl<Crypto: lakers::Crypto> core::fmt::Display for SecContextState<Crypto> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
use SecContextStage::*;
Expand Down Expand Up @@ -234,7 +258,7 @@ impl<Crypto: lakers::Crypto> SecContextState<Crypto> {
/// While the EDHOC part could be implemented as a handler that is to be added into the tree, the
/// OSCORE part needs to wrap the inner handler anyway, and EDHOC and OSCORE are intertwined rather
/// strongly in processing the EDHOC option.
pub struct OscoreEdhocHandler<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> {
pub struct OscoreEdhocHandler<'a, H: coap_handler::Handler, Crypto: lakers::Crypto> {
// It'd be tempted to have sharing among multiple handlers for multiple CoAP stacks, but
// locks for such sharing could still be acquired in a factory (at which point it may make
// sense to make this a &mut).
Expand All @@ -254,27 +278,21 @@ pub struct OscoreEdhocHandler<'a, H: coap_handler::Handler, L: Write, Crypto: la
// called, or an AuthorizationChecked::Allowed is around.
inner: H,

log: L,

crypto_factory: fn() -> Crypto,
}

impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto>
OscoreEdhocHandler<'a, H, L, Crypto>
{
impl<'a, H: coap_handler::Handler, Crypto: lakers::Crypto> OscoreEdhocHandler<'a, H, Crypto> {
// FIXME: Apart from an own identity, this will also need a function to convert ID_CRED_I into
// a (CRED_I, AifStaticRest) pair.
pub fn new(
own_identity: (&'a lakers::CredentialRPK, &'static [u8]),
inner: H,
log: L,
crypto_factory: fn() -> Crypto,
) -> Self {
Self {
pool: Default::default(),
own_identity,
inner,
log,
crypto_factory,
}
}
Expand Down Expand Up @@ -368,8 +386,8 @@ impl<O: RenderableOnMinimal, I: RenderableOnMinimal> RenderableOnMinimal for OrI
}
}

impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> coap_handler::Handler
for OscoreEdhocHandler<'a, H, L, Crypto>
impl<'a, H: coap_handler::Handler, Crypto: lakers::Crypto> coap_handler::Handler
for OscoreEdhocHandler<'a, H, Crypto>
{
type RequestData = OrInner<
EdhocResponse<Result<H::RequestData, H::ExtractRequestError>>,
Expand Down Expand Up @@ -469,6 +487,7 @@ impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> coap_handle
let starts_with_true = first_byte == &0xf5;

if starts_with_true {
info!("Processing incoming EDHOC message 1");
let message_1 =
&lakers::EdhocMessageBuffer::new_from_slice(&request.payload()[1..])
.map_err(too_small)?;
Expand Down Expand Up @@ -502,15 +521,14 @@ impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> coap_handle
),
);

writeln!(self.log, "Entries in pool:").unwrap();
debug!("Entries in pool:");
for (i, e) in self.pool.entries.iter().enumerate() {
writeln!(self.log, "{i}. {e}").unwrap();
debug!("{}. {}", i, e);
}
write!(self.log, "Sequence: ").unwrap();
debug!("Sequence:");
for index in self.pool.sorted.iter() {
write!(self.log, "{index},").unwrap();
debug!("* {}", index);
}
writeln!(self.log, "").unwrap();
let evicted = self.pool.force_insert(SecContextState {
protocol_stage: SecContextStage::EdhocResponderProcessedM1 {
c_r,
Expand All @@ -520,9 +538,9 @@ impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> coap_handle
authorization: self.unauthenticated_edhoc_user_authorization(),
});
if let Some(evicted) = evicted {
writeln!(self.log, "To insert new EDHOC, evicted {}", evicted).unwrap();
warn!("To insert new EDHOC, evicted {}", evicted);
} else {
writeln!(self.log, "To insert new EDHOC, evicted none").unwrap();
debug!("To insert new EDHOC, evicted none");
}

Ok(Own(EdhocResponse::OkSend2(c_r)))
Expand Down Expand Up @@ -589,11 +607,7 @@ impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> coap_handle
if id_cred_i.reference_only() {
match id_cred_i.kid {
43 => {
writeln!(
self.log,
"Peer indicates use of the one preconfigured key"
)
.unwrap();
info!("Peer indicates use of the one preconfigured key");

use hexlit::hex;
const CRED_I: &[u8] = &hex!("A2027734322D35302D33312D46462D45462D33372D33322D333908A101A5010202412B2001215820AC75E9ECE3E50BFC8ED60399889522405C47BF16DF96660A41298CB4307F7EB62258206E5DE611388A4B8A8211334AC7D37ECB52A387D257E6DB3C2A93DF21FF3AFFC8");
Expand All @@ -614,12 +628,10 @@ impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> coap_handle
}
}
} else {
writeln!(
self.log,
info!(
"Got credential by value: {:?}..",
&id_cred_i.value.get_slice(0, 5)
)
.unwrap();
);

cred_i = lakers::CredentialRPK::new(id_cred_i.value)
// FIXME What kind of error do we send here?
Expand All @@ -642,8 +654,8 @@ impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> coap_handle
let oscore_salt = responder.edhoc_exporter(1u8, &[], 8); // label is 1
let oscore_secret = &oscore_secret[..16];
let oscore_salt = &oscore_salt[..8];
writeln!(self.log, "OSCORE secret: {:?}...", &oscore_secret[..5]).unwrap();
writeln!(self.log, "OSCORE salt: {:?}", &oscore_salt).unwrap();
debug!("OSCORE secret: {:?}...", &oscore_secret[..5]);
debug!("OSCORE salt: {:?}", &oscore_salt);

let sender_id = c_i.as_slice();
let recipient_id = kid.0;
Expand Down Expand Up @@ -679,6 +691,11 @@ impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> coap_handle
// processing.
}

info!(
"Processed {} bytes at start of message into new EDHOC context",
cutoff
);

cutoff
} else {
0
Expand Down Expand Up @@ -753,7 +770,7 @@ impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> coap_handle

let Ok((correlation, extracted)) = decrypted else {
// FIXME is that the right code?
writeln!(self.log, "Decryption failure").unwrap();
error!("Decryption failure");
return Err(Own(CoAPError::unauthorized()));
};

Expand Down Expand Up @@ -884,35 +901,35 @@ impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> coap_handle
// One attempt to render rendering errors
// FIXME rewind message
Err(e) => {
write!(self.log, "Rendering successful extraction failed with {e:?}, ").unwrap();
error!("Rendering successful extraction failed with {:?}", Debug2Format(&e));
match e.render(response) {
Ok(()) => {
writeln!(self.log, "error rendered").unwrap();
error!("Error rendered.");
},
Err(e2) => {
writeln!(self.log, "error could not be rendered: {e2:?}").unwrap();
error!("Error could not be rendered: {:?}.", Debug2Format(&e2));
// FIXME rewind message
response.set_code(coap_numbers::code::INTERNAL_SERVER_ERROR);
}
};
},
},
AuthorizationChecked::Allowed(Err(inner_request_error)) => {
write!(self.log, "Extraction failed with {inner_request_error:?}, ").unwrap();
error!("Extraction failed with {:?}.", Debug2Format(&inner_request_error));
match inner_request_error.render(response) {
Ok(()) => {
writeln!(self.log, "rendered successfully").unwrap();
error!("Original error rendered successfully.");
},
Err(e) => {
write!(self.log, "could not be rendered due to {e:?}, ").unwrap();
error!("Original error could not be rendered due to {:?}:", Debug2Format(&e));
// Two attempts to render extraction errors
// FIXME rewind message
match e.render(response) {
Ok(()) => {
writeln!(self.log, "which was rendered fine").unwrap();
error!("Error was rendered fine.");
},
Err(e2) => {
writeln!(self.log, "rendering which caused {e2:?}").unwrap();
error!("Rendering error caused {:?}.", Debug2Format(&e2));
// FIXME rewind message
response.set_code(
coap_numbers::code::INTERNAL_SERVER_ERROR,
Expand All @@ -931,7 +948,7 @@ impl<'a, H: coap_handler::Handler, L: Write, Crypto: lakers::Crypto> coap_handle
)
.is_err()
{
writeln!(self.log, "Oups, responding with weird state").unwrap();
error!("Oups, responding with weird state");
// todo!("Thanks to the protect API we've lost access to our response");
}
});
Expand Down
3 changes: 3 additions & 0 deletions src/riot-rs-coap/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ workspace = true
## if no features are configured at all.
doc = ["embassy-net/proto-ipv6", "embassy-net/medium-ip"]

## Enables defmt logging of coapcore
defmt = ["coapcore/log"]

# Only around while we carry the embedded-nal-async impl
proto-ipv4 = ["embassy-net/proto-ipv4"]
proto-ipv6 = ["embassy-net/proto-ipv6"]
10 changes: 1 addition & 9 deletions src/riot-rs-coap/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,6 @@ use static_cell::StaticCell;

const CONCURRENT_REQUESTS: usize = 3;

struct DevNull;

impl core::fmt::Write for DevNull {
fn write_str(&mut self, _: &str) -> core::fmt::Result {
Ok(())
}
}

// FIXME: log_stdout is not something we want to have here
// FIXME: I'd rather have the client_out available anywhere, but at least the way CoAPRuntimeClient
// is set up right now, server and client have to run in the same thread.
Expand Down Expand Up @@ -75,7 +67,7 @@ pub async fn coap_run(
// FIXME: Should we allow users to override that? After all, this is just convenience and may
// be limiting in special applications.
let handler = handler.with_wkc();
let mut handler = seccontext::OscoreEdhocHandler::new(own_identity, handler, DevNull, || {
let mut handler = seccontext::OscoreEdhocHandler::new(own_identity, handler, || {
lakers_crypto_rustcrypto::Crypto::new(riot_rs_random::crypto_rng())
});

Expand Down
1 change: 1 addition & 0 deletions src/riot-rs/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ wifi-esp = ["riot-rs-embassy/wifi-esp"]
debug-console = ["riot-rs-rt/debug-console"]
## Enables logging support through `defmt`.
defmt = [
"riot-rs-coap?/defmt",
"riot-rs-debug/defmt",
"riot-rs-embassy/defmt",
"riot-rs-threads?/defmt",
Expand Down

0 comments on commit ec6d0b4

Please sign in to comment.