diff --git a/crates/jmap/src/inbuxa/protocol_policy.rs b/crates/jmap/src/inbuxa/protocol_policy.rs index e382dbd..e60129f 100644 --- a/crates/jmap/src/inbuxa/protocol_policy.rs +++ b/crates/jmap/src/inbuxa/protocol_policy.rs @@ -13,7 +13,12 @@ //! [`Server::set_protocol_policy`], which applies the locks (LP-21), removes //! or restores the listener objects (LP-1, LP-5) and closes or opens their //! sockets (LP-2). What comes back is what actually happened. +//! +//! [`validate_listener`] is the registry's side of it: while the switch is +//! off, no listener it would close may be created, or made by an update +//! (LP-4). +use crate::registry::mapping::{ObjectResponse, RegistrySetResponse, ValidationResult}; use common::{Server, auth::AccessToken, network::legacy::PolicyChange}; use inbuxa_features::security::{ listeners, @@ -31,6 +36,7 @@ use jmap_proto::{ request::IntoValid, }; use jmap_tools::{Key, Map, Value}; +use registry::schema::{prelude::Property, structs::NetworkListener}; use types::id::Id; type PValue = Value<'static, P, ProtocolPolicyValue>; @@ -308,3 +314,119 @@ pub async fn set( } Ok(response) } + +/// LP-4: while legacy protocols are off, a listener the switch would close +/// may not be created, nor may an update make one. Otherwise a listener could +/// quietly reopen a port the switch is meant to keep closed. +/// +/// The rule is the switch's own ([`listeners::closes`]), so a locked protocol +/// or the inbound port is never refused here, and what the switch would close +/// is exactly what can't be added. Putting saved listeners back (LP-5) goes +/// through the registry directly, not through `/set`, so it isn't affected. +pub(crate) async fn validate_listener( + set: &RegistrySetResponse<'_>, + listener: &NetworkListener, +) -> ValidationResult { + let policy = set.server.protocol_policy().await?; + Ok(match listener_refusal(&policy, listener) { + Some((property, why)) => Err(SetError::invalid_properties() + .with_property(property) + .with_description(why)), + None => Ok(ObjectResponse::default()), + }) +} + +/// Why this listener can't exist under this policy, naming the policy and the +/// property to change, or `None` when it can. +fn listener_refusal(policy: &Policy, listener: &NetworkListener) -> Option<(Property, String)> { + if !listeners::closes(policy, listener) { + return None; + } + let protocol = listeners::protocol_name(listener.protocol); + // A submission listener closes because of its port, not its protocol + // (LP-3), so the port is what would have to change. + let property = if protocol == "smtp" { + Property::Bind + } else { + Property::Protocol + }; + Some(( + property, + format!( + "Legacy mail protocols are off (inbuxa:ProtocolPolicy), and this {protocol} \ + listener would reopen a port the switch keeps closed. Turn legacy protocols \ + back on first." + ), + )) +} + +#[cfg(test)] +mod tests { + use super::*; + use registry::{ + schema::{enums::NetworkListenerProtocol, prelude::SocketAddr}, + types::map::Map, + }; + use std::str::FromStr; + + fn listener(protocol: NetworkListenerProtocol, bind: &str) -> NetworkListener { + NetworkListener { + name: "new".to_string(), + protocol, + bind: Map::new(vec![SocketAddr::from_str(bind).unwrap()]), + ..Default::default() + } + } + + fn off() -> Policy { + let mut policy = Policy { + legacy_protocols: LegacyProtocols::Disabled, + ..Default::default() + }; + policy.apply_locks(); + policy + } + + #[test] + fn on_refuses_nothing() { + let on = Policy::default(); + for protocol in [ + NetworkListenerProtocol::Imap, + NetworkListenerProtocol::Pop3, + NetworkListenerProtocol::ManageSieve, + ] { + assert!(listener_refusal(&on, &listener(protocol, "[::]:1993")).is_none()); + } + } + + #[test] + fn off_refuses_every_legacy_protocol_naming_the_policy() { + for protocol in [ + NetworkListenerProtocol::Imap, + NetworkListenerProtocol::Pop3, + NetworkListenerProtocol::ManageSieve, + ] { + let (property, why) = + listener_refusal(&off(), &listener(protocol, "[::]:1993")).expect("refused"); + assert_eq!(property, Property::Protocol); + assert!(why.contains("inbuxa:ProtocolPolicy"), "{why}"); + } + } + + #[test] + fn off_still_allows_what_the_switch_never_closes() { + // Locked (LP-21) and inbound (LP-3): the switch doesn't close them, + // so there is nothing for a new one to reopen. + for (protocol, bind) in [ + (NetworkListenerProtocol::Smtp, "[::]:25"), + (NetworkListenerProtocol::Smtp, "[::]:587"), + (NetworkListenerProtocol::Http, "[::]:443"), + (NetworkListenerProtocol::Lmtp, "[::]:24"), + ] { + assert!( + listener_refusal(&off(), &listener(protocol, bind)).is_none(), + "{protocol:?} on {bind}" + ); + } + } +} diff --git a/crates/jmap/src/registry/set.rs b/crates/jmap/src/registry/set.rs index 8029695..a7f59ab 100644 --- a/crates/jmap/src/registry/set.rs +++ b/crates/jmap/src/registry/set.rs @@ -502,6 +502,11 @@ impl RegistrySet for Server { ) .await? } + // inbuxa: legacy-protocols LP-4 + ObjectInner::NetworkListener(listener) => { + crate::inbuxa::protocol_policy::validate_listener(&set, listener) + .await? + } // inbuxa: ME-12 to ME-17 ObjectInner::MaskedEmail(mask) => { let old = match &modification { diff --git a/tests/e2e/legacy_protocols.py b/tests/e2e/legacy_protocols.py index a7ed06d..7831291 100755 --- a/tests/e2e/legacy_protocols.py +++ b/tests/e2e/legacy_protocols.py @@ -13,10 +13,11 @@ a running server (LP-2), and whether a listener put back actually binds again (LP-5). Acceptance tests 15, 17 and 18. It also checks the second lock (LP-6): while the switch is off, sign-in over -submission -- locked open -- and over an IMAP listener that exists by mistake -is refused with the spec's words, with the right password and with a wrong -one, and refusals never add up to a disconnect (LP-11). And that a normal -IMAP sign-in works with the switch on, before and after. +submission -- locked open -- is refused with the spec's words, with the right +password and with a wrong one, and refusals never add up to a disconnect +(LP-11). And that a normal IMAP sign-in works with the switch on, before and +after. And that while it is off, no listener the switch would close can be +created, or made by an update (LP-4, test 4). Passwords are generated into files under target/e2e and never printed. Everything is removed afterwards unless KEEP=1. @@ -31,10 +32,8 @@ HTTP = "http://127.0.0.1:18080" # makes the host side accept connections whether or not anything is listening # inside the container, so a bare connect proves nothing: each port has to be # made to speak. -PORTS = {"imap": 18993, "pop3": 18995, "submissions": 18465, "smtp": 18025, "mistake": 18994} -TLS_PORTS = {18993, 18995, 18465, 18994} -IMAP_REFUSAL = ("NO [ALERT] This server allows only INBUXA webmail and JMAP apps. " - "This mail app can't sign in.") +PORTS = {"imap": 18993, "pop3": 18995, "submissions": 18465, "smtp": 18025} +TLS_PORTS = {18993, 18995, 18465} SMTP_REFUSAL = ("535 5.7.0 This server allows only INBUXA webmail and JMAP apps. " "This mail app can't send.") INBUXA = "urn:inbuxa:jmap" @@ -70,8 +69,7 @@ def start(env_file=None): "-p", f"127.0.0.1:{PORTS['submissions']}:465", "-p", f"127.0.0.1:{PORTS['imap']}:993", "-p", f"127.0.0.1:{PORTS['pop3']}:995", - "-p", f"127.0.0.1:{PORTS['smtp']}:25", - "-p", f"127.0.0.1:{PORTS['mistake']}:1993"] + "-p", f"127.0.0.1:{PORTS['smtp']}:25"] if env_file: args += ["--env-file", env_file] args += ["stalwartlabs/stalwart:v0.16.22", "--config", "/etc/inbuxa/config.json"] @@ -297,13 +295,32 @@ def main(): if not all(r == SMTP_REFUSAL for r in replies): print(" replies:", replies) - # An IMAP listener that exists by mistake: created while the switch is off - # (LP-4 will refuse this later), and live after the restart below. + # No listener the switch would close can be added while it is off (LP-4, + # test 4), and the refusal names the policy. res = one(admin, admin_pw, "x:NetworkListener/set", {"create": {"m": { - "name": "imap-mistake", "protocol": "imap", "bind": {"0.0.0.0:1993": True}, + "name": "imap-new", "protocol": "imap", "bind": {"0.0.0.0:1993": True}, "tlsImplicit": True}}}) - mistake = (res[1].get("created") or {}).get("m", {}).get("id") - check(mistake is not None, "an IMAP listener can still be created by mistake") + refused = (res[1].get("notCreated") or {}).get("m") or {} + check(refused.get("type") == "invalidProperties" + and "protocol" in (refused.get("properties") or []) + and "inbuxa:ProtocolPolicy" in (refused.get("description") or ""), + "creating an IMAP listener is refused, naming the policy (LP-4)") + if not refused: + print(" reply:", json.dumps(res[1])[:300]) + + # What the switch never closes can still be added; turning it into a + # listener the switch would close is refused like creating one. + res = one(admin, admin_pw, "x:NetworkListener/set", {"create": {"s": { + "name": "submission-extra", "protocol": "smtp", "bind": {"0.0.0.0:2587": True}}}}) + extra = (res[1].get("created") or {}).get("s", {}).get("id") + check(extra is not None, "an SMTP listener can still be created, being locked (LP-4, LP-21)") + if extra: + res = one(admin, admin_pw, "x:NetworkListener/set", + {"update": {extra: {"protocol": "imap"}}}) + refused = (res[1].get("notUpdated") or {}).get(extra) or {} + check(refused.get("type") == "invalidProperties", + "turning it into an IMAP listener is refused (LP-4)") + one(admin, admin_pw, "x:NetworkListener/set", {"destroy": [extra]}) # A restart must not reopen them: the objects are gone, not just the sockets. stop() @@ -311,16 +328,6 @@ def main(): check(settle(PORTS["imap"], False), "IMAP still closed after a restart") check(accepts(PORTS["smtp"]), "inbound SMTP still accepts after a restart") - if mistake: - check(settle(PORTS["mistake"], True), "the mistaken IMAP listener is up") - check(imap_login(PORTS["mistake"], admin, admin_pw) == IMAP_REFUSAL, - "the mistaken listener refuses the right password (LP-6)") - check(imap_login(PORTS["mistake"], admin, "wrong") == IMAP_REFUSAL, - "and a wrong one, the same way (LP-11)") - check(imap_login(PORTS["mistake"], "nobody@legacy.test", "x") == IMAP_REFUSAL, - "and an account that doesn't exist (LP-11)") - one(admin, admin_pw, "x:NetworkListener/set", {"destroy": [mistake]}) - # Turn it back on: the listeners come back and bind again (LP-5). res = one(admin, admin_pw, "inbuxa:ProtocolPolicy/set", policy_set({"legacyProtocols": "enabled"}))