Author: Kevin Schoon [me@kevinschoon.com]
Hash: ccda01bbe79dc5781cc619003785e6c47f180b14
Timestamp: Sun, 04 Aug 2024 11:16:10 +0000 (2 months ago)

+81 -23 +/-2 browse
ensure client initializes session before processing commands
1diff --git a/maitred/src/server.rs b/maitred/src/server.rs
2index 9ef55f4..d5441d3 100644
3--- a/maitred/src/server.rs
4+++ b/maitred/src/server.rs
5 @@ -39,6 +39,7 @@ const DEFAULT_MAXIMUM_SIZE: u64 = 5_000_000;
6 const DEFAULT_CAPABILITIES: u32 =
7 smtp_proto::EXT_SIZE | smtp_proto::EXT_ENHANCED_STATUS_CODES | smtp_proto::EXT_PIPELINING;
8
9+ /// Apply pipelining if running in extended mode and configured to support it
10 struct ConditionalPipeline<'a> {
11 pub opts: &'a SessionOptions,
12 pub session: &'a mut Session,
13 @@ -48,10 +49,10 @@ struct ConditionalPipeline<'a> {
14 impl ConditionalPipeline<'_> {
15 pub fn apply(&mut self, req: &Request<String>, data: Option<&Bytes>) -> Vec<Response<String>> {
16 let response = self.session.process(self.opts, req, data);
17- if self.opts.capabilities & smtp_proto::EXT_PIPELINING != 0 {
18+ if self.opts.capabilities & smtp_proto::EXT_PIPELINING != 0 && self.session.is_extended() {
19 self.pipeline.process(req, &response)
20 } else {
21- match self.session.process(self.opts, req, data) {
22+ match response {
23 Ok(response) => {
24 tracing::debug!("Client response: {:?}", response);
25 vec![response]
26 @@ -143,11 +144,7 @@ impl Server {
27 self
28 }
29
30- async fn process<T>(
31- &self,
32- mut framed: Framed<T, Transport>,
33- opts: crate::session::Options,
34- ) -> Result<Session, Error>
35+ async fn process<T>(&self, mut framed: Framed<T, Transport>) -> Result<Session, Error>
36 where
37 T: tokio::io::AsyncRead + tokio::io::AsyncWrite + std::marker::Unpin,
38 {
39 @@ -217,7 +214,7 @@ impl Server {
40 let addr = socket.local_addr()?;
41 tracing::info!("Accepted connection on: {:?}", addr);
42 let framed = Framed::new(socket, Transport::default());
43- if let Err(err) = self.process(framed, self.config.session_opts()).await {
44+ if let Err(err) = self.process(framed).await {
45 tracing::warn!("Client encountered an error: {:?}", err);
46 }
47 }
48 @@ -298,16 +295,7 @@ mod test {
49 };
50 let server = Server::new("example.org");
51 let framed = Framed::new(stream, Transport::default());
52- let session = server
53- .process(
54- framed,
55- crate::session::Options {
56- hostname: "localhost".to_string(),
57- ..Default::default()
58- },
59- )
60- .await
61- .unwrap();
62+ let session = server.process(framed).await.unwrap();
63 assert!(session
64 .mail_from
65 .is_some_and(|mail_from| mail_from.get_email() == "fuu@bar.com"));
66 diff --git a/maitred/src/session.rs b/maitred/src/session.rs
67index 59f0c4e..9bc01da 100644
68--- a/maitred/src/session.rs
69+++ b/maitred/src/session.rs
70 @@ -13,6 +13,11 @@ use crate::{smtp_err, smtp_ok, smtp_response};
71 /// level error that will be returned to the client.
72 pub type Result = StdResult<Response<String>, Response<String>>;
73
74+ enum Mode {
75+ Legacy,
76+ Extended,
77+ }
78+
79 enum DataTransfer {
80 Data,
81 Bdat,
82 @@ -29,6 +34,12 @@ pub fn timeout(message: &str) -> Response<String> {
83 smtp_response!(421, 4, 4, 2, format!("Timeout exceeded: {}", message))
84 }
85
86+ /// Sent when the client attempts communication before initializing the
87+ /// session with HELO/ELHO
88+ pub fn not_initialized() -> Result {
89+ smtp_err!(500, 0, 0, 0, "It's polite to say EHLO first")
90+ }
91+
92 /// Runtime options that influence server behavior
93 #[derive(Default)]
94 pub(crate) struct Options {
95 @@ -52,6 +63,7 @@ pub(crate) struct Session {
96 pub hostname: Option<Host>,
97 /// If an active data transfer is taking place
98 data_transfer: Option<DataTransfer>,
99+ initialized: Option<Mode>,
100 }
101
102 impl Session {
103 @@ -64,6 +76,26 @@ impl Session {
104 self.data_transfer = None;
105 }
106
107+ /// If the session is in extended mode i.e. EHLO was sent
108+ pub fn is_extended(&self) -> bool {
109+ self.initialized
110+ .as_ref()
111+ .is_some_and(|mode| matches!(mode, Mode::Extended))
112+ }
113+
114+ fn check_initialized(&self) -> StdResult<(), Response<String>> {
115+ if self.initialized.is_none() {
116+ return Err(smtp_response!(
117+ 500,
118+ 0,
119+ 0,
120+ 0,
121+ "It's polite to say EHLO first"
122+ ));
123+ }
124+ Ok(())
125+ }
126+
127 /// Statefully process the SMTP command with optional data payload, any
128 /// error returned is passed back to the caller.
129 /// NOTE:
130 @@ -91,15 +123,18 @@ impl Session {
131 Request::Lhlo { host } => {
132 self.hostname =
133 Some(Host::parse(host).map_err(|e| smtp_response!(500, 0, 0, 0, e))?);
134+ self.initialized = Some(Mode::Legacy);
135 smtp_ok!(250, 0, 0, 0, format!("Hello {}", host))
136 }
137 Request::Helo { host } => {
138 self.hostname = Some(
139 Host::parse(host).map_err(|e| smtp_response!(500, 0, 0, 0, e.to_string()))?,
140 );
141+ self.initialized = Some(Mode::Legacy);
142 smtp_ok!(250, 0, 0, 0, format!("Hello {}", host))
143 }
144 Request::Mail { from } => {
145+ self.check_initialized()?;
146 let mail_from = Address::try_from(from.address.as_str()).map_err(|e| {
147 smtp_response!(
148 500,
149 @@ -113,6 +148,7 @@ impl Session {
150 smtp_ok!(250, 0, 0, 0, "OK")
151 }
152 Request::Rcpt { to } => {
153+ self.check_initialized()?;
154 let rcpt_to = Address::try_from(to.address.as_str()).map_err(|e| {
155 smtp_response!(500, 0, 0, 0, format!("Cannot parse: {} {}", to.address, e))
156 })?;
157 @@ -127,14 +163,17 @@ impl Session {
158 chunk_size: _,
159 is_last: _,
160 } => {
161+ self.check_initialized()?;
162 if let Some(transfer_mode) = &self.data_transfer {
163 match transfer_mode {
164 DataTransfer::Data => {
165 panic!("Transfer mode changed from DATA to BDAT")
166 }
167 DataTransfer::Bdat => {
168- let message_payload =
169- data.as_ref().expect("data returned without a payload").to_vec();
170+ let message_payload = data
171+ .as_ref()
172+ .expect("data returned without a payload")
173+ .to_vec();
174 let parser = MessageParser::new();
175 let response = match parser.parse(&message_payload) {
176 Some(_) => smtp_ok!(250, 0, 0, 0, "OK"),
177 @@ -161,10 +200,14 @@ impl Session {
178 mechanism,
179 initial_response,
180 } => todo!(),
181- Request::Noop { value: _ } => smtp_ok!(250, 0, 0, 0, "OK".to_string()),
182+ Request::Noop { value: _ } => {
183+ self.check_initialized()?;
184+ smtp_ok!(250, 0, 0, 0, "OK".to_string())
185+ }
186 Request::Vrfy { value } => todo!(),
187 Request::Expn { value } => todo!(),
188 Request::Help { value } => {
189+ self.check_initialized()?;
190 if value.is_empty() {
191 smtp_ok!(250, 0, 0, 0, opts.help_banner.to_string())
192 } else {
193 @@ -182,14 +225,17 @@ impl Session {
194 Request::Burl { uri, is_last } => todo!(),
195 Request::StartTls => todo!(),
196 Request::Data => {
197+ self.check_initialized()?;
198 if let Some(transfer_mode) = &self.data_transfer {
199 match transfer_mode {
200 DataTransfer::Bdat => {
201 panic!("Transfer mode changed from BDAT to DATA")
202 }
203 DataTransfer::Data => {
204- let message_payload =
205- data.as_ref().expect("data returned without a payload").to_vec();
206+ let message_payload = data
207+ .as_ref()
208+ .expect("data returned without a payload")
209+ .to_vec();
210 let parser = MessageParser::new();
211 let response = match parser.parse(&message_payload) {
212 Some(_) => smtp_ok!(250, 0, 0, 0, "OK".to_string()),
213 @@ -219,6 +265,7 @@ impl Session {
214 }
215 }
216 Request::Rset => {
217+ self.check_initialized()?;
218 self.reset();
219 smtp_ok!(200, 0, 0, 0, "".to_string())
220 }
221 @@ -317,6 +364,29 @@ mod test {
222 }
223
224 #[test]
225+ fn test_command_with_no_hello() {
226+ let requests = &[TestCase {
227+ request: Request::Mail {
228+ from: MailFrom {
229+ address: String::from("fuu@example.org"),
230+ ..Default::default()
231+ },
232+ },
233+ payload: None,
234+ expected: smtp_err!(500, 0, 0, 0, String::from("It's polite to say EHLO first")),
235+ }];
236+ let mut session = Session::default();
237+ process_all(
238+ &mut session,
239+ &Options {
240+ hostname: EXAMPLE_HOSTNAME.to_string(),
241+ ..Default::default()
242+ },
243+ requests,
244+ );
245+ }
246+
247+ #[test]
248 fn test_email_with_body() {
249 let requests = &[
250 TestCase {