Commit | Line | Data |
---|---|---|
01e60269 AM |
1 | From 0a5441fcd93ae4145c07b3ed138dfe0e107174e0 Mon Sep 17 00:00:00 2001 |
2 | From: Jeremy Harris <jgh146exb@wizmail.org> | |
3 | Date: Mon, 27 May 2019 23:44:31 +0100 | |
4 | Subject: [PATCH 1/2] Fix smtp response timeout | |
5 | ||
6 | --- | |
7 | doc/ChangeLog | 6 ++++++ | |
8 | src/functions.h | 4 ++-- | |
9 | src/ip.c | 16 +++++++--------- | |
10 | src/malware.c | 26 +++++++++++++------------- | |
11 | src/routers/iplookup.c | 2 +- | |
12 | src/smtp_out.c | 9 +++++---- | |
13 | src/spam.c | 2 +- | |
14 | src/transports/smtp_socks.c | 6 +++--- | |
15 | src/verify.c | 2 +- | |
16 | 9 files changed, 39 insertions(+), 34 deletions(-) | |
17 | ||
18 | --- a/doc/ChangeLog | |
19 | +++ b/doc/ChangeLog | |
20 | @@ -50,6 +50,13 @@ JH/27 Bug 2404: Use the main-section con | |
21 | success-DSN messages. Previously the From: header was always the default | |
22 | one for these; the option was ignored. | |
23 | ||
24 | +JH/28 Fix the timeout on smtp response to apply to the whole response. | |
25 | + Previously it was reset for every read, so a teergrubing peer sending | |
26 | + single bytes within the time limit could extend the connection for a | |
27 | + long time. Credit to Qualsys Security Advisory Team for the discovery. | |
28 | +[from GIT master] | |
29 | + | |
30 | + | |
31 | ||
32 | Exim version 4.92 | |
33 | ----------------- | |
34 | --- a/src/functions.h | |
35 | +++ b/src/functions.h | |
36 | @@ -225,7 +225,7 @@ extern uschar *expand_string_copy(const | |
37 | extern int_eximarith_t expand_string_integer(uschar *, BOOL); | |
38 | extern void modify_variable(uschar *, void *); | |
39 | ||
40 | -extern BOOL fd_ready(int, int); | |
41 | +extern BOOL fd_ready(int, time_t); | |
42 | ||
43 | extern int filter_interpret(uschar *, int, address_item **, uschar **); | |
44 | extern BOOL filter_personal(string_item *, BOOL); | |
45 | @@ -271,7 +271,7 @@ extern int ip_connectedsocket(int, c | |
46 | int, host_item *, uschar **, const blob *); | |
47 | extern int ip_get_address_family(int); | |
48 | extern void ip_keepalive(int, const uschar *, BOOL); | |
49 | -extern int ip_recv(client_conn_ctx *, uschar *, int, int); | |
50 | +extern int ip_recv(client_conn_ctx *, uschar *, int, time_t); | |
51 | extern int ip_socket(int, int); | |
52 | ||
53 | extern int ip_tcpsocket(const uschar *, uschar **, int); | |
54 | --- a/src/ip.c | |
55 | +++ b/src/ip.c | |
56 | @@ -566,16 +566,15 @@ if (setsockopt(sock, SOL_SOCKET, SO_KEEP | |
57 | /* | |
58 | Arguments: | |
59 | fd the file descriptor | |
60 | - timeout the timeout, seconds | |
61 | + timelimit the timeout endpoint, seconds-since-epoch | |
62 | Returns: TRUE => ready for i/o | |
63 | FALSE => timed out, or other error | |
64 | */ | |
65 | BOOL | |
66 | -fd_ready(int fd, int timeout) | |
67 | +fd_ready(int fd, time_t timelimit) | |
68 | { | |
69 | fd_set select_inset; | |
70 | -time_t start_recv = time(NULL); | |
71 | -int time_left = timeout; | |
72 | +int time_left = timelimit - time(NULL); | |
73 | int rc; | |
74 | ||
75 | if (time_left <= 0) | |
76 | @@ -609,8 +608,7 @@ do | |
77 | DEBUG(D_transport) debug_printf("EINTR while waiting for socket data\n"); | |
78 | ||
79 | /* Watch out, 'continue' jumps to the condition, not to the loops top */ | |
80 | - time_left = timeout - (time(NULL) - start_recv); | |
81 | - if (time_left > 0) continue; | |
82 | + if ((time_left = timelimit - time(NULL)) > 0) continue; | |
83 | } | |
84 | ||
85 | if (rc <= 0) | |
86 | @@ -634,18 +632,18 @@ Arguments: | |
87 | cctx the connection context (socket fd, possibly TLS context) | |
88 | buffer to read into | |
89 | bufsize the buffer size | |
90 | - timeout the timeout | |
91 | + timelimit the timeout endpoint, seconds-since-epoch | |
92 | ||
93 | Returns: > 0 => that much data read | |
94 | <= 0 on error or EOF; errno set - zero for EOF | |
95 | */ | |
96 | ||
97 | int | |
98 | -ip_recv(client_conn_ctx * cctx, uschar * buffer, int buffsize, int timeout) | |
99 | +ip_recv(client_conn_ctx * cctx, uschar * buffer, int buffsize, time_t timelimit) | |
100 | { | |
101 | int rc; | |
102 | ||
103 | -if (!fd_ready(cctx->sock, timeout)) | |
104 | +if (!fd_ready(cctx->sock, timelimit)) | |
105 | return -1; | |
106 | ||
107 | /* The socket is ready, read from it (via TLS if it's active). On EOF (i.e. | |
108 | --- a/src/malware.c | |
109 | +++ b/src/malware.c | |
110 | @@ -349,13 +349,13 @@ return cre; | |
111 | -2 on timeout or error | |
112 | */ | |
113 | static int | |
114 | -recv_line(int fd, uschar * buffer, int bsize, int tmo) | |
115 | +recv_line(int fd, uschar * buffer, int bsize, time_t tmo) | |
116 | { | |
117 | uschar * p = buffer; | |
118 | ssize_t rcv; | |
119 | BOOL ok = FALSE; | |
120 | ||
121 | -if (!fd_ready(fd, tmo-time(NULL))) | |
122 | +if (!fd_ready(fd, tmo)) | |
123 | return -2; | |
124 | ||
125 | /*XXX tmo handling assumes we always get a whole line */ | |
126 | @@ -382,9 +382,9 @@ return p - buffer; | |
127 | ||
128 | /* return TRUE iff size as requested */ | |
129 | static BOOL | |
130 | -recv_len(int sock, void * buf, int size, int tmo) | |
131 | +recv_len(int sock, void * buf, int size, time_t tmo) | |
132 | { | |
133 | -return fd_ready(sock, tmo-time(NULL)) | |
134 | +return fd_ready(sock, tmo) | |
135 | ? recv(sock, buf, size, 0) == size | |
136 | : FALSE; | |
137 | } | |
138 | @@ -430,7 +430,7 @@ for (;;) | |
139 | } | |
140 | ||
141 | static inline int | |
142 | -mksd_read_lines (int sock, uschar *av_buffer, int av_buffer_size, int tmo) | |
143 | +mksd_read_lines (int sock, uschar *av_buffer, int av_buffer_size, time_t tmo) | |
144 | { | |
145 | client_conn_ctx cctx = {.sock = sock}; | |
146 | int offset = 0; | |
147 | @@ -438,7 +438,7 @@ int i; | |
148 | ||
149 | do | |
150 | { | |
151 | - i = ip_recv(&cctx, av_buffer+offset, av_buffer_size-offset, tmo-time(NULL)); | |
152 | + i = ip_recv(&cctx, av_buffer+offset, av_buffer_size-offset, tmo); | |
153 | if (i <= 0) | |
154 | { | |
155 | (void) malware_panic_defer(US"unable to read from mksd UNIX socket (/var/run/mksd/socket)"); | |
156 | @@ -497,7 +497,7 @@ switch (*line) | |
157 | ||
158 | static int | |
159 | mksd_scan_packed(struct scan * scanent, int sock, const uschar * scan_filename, | |
160 | - int tmo) | |
161 | + time_t tmo) | |
162 | { | |
163 | struct iovec iov[3]; | |
164 | const char *cmd = "MSQ\n"; | |
165 | @@ -746,7 +746,7 @@ if (!malware_ok) | |
166 | if (m_sock_send(malware_daemon_ctx.sock, scanrequest, Ustrlen(scanrequest), &errstr) < 0) | |
167 | return m_panic_defer(scanent, CUS callout_address, errstr); | |
168 | ||
169 | - bread = ip_recv(&malware_daemon_ctx, av_buffer, sizeof(av_buffer), tmo-time(NULL)); | |
170 | + bread = ip_recv(&malware_daemon_ctx, av_buffer, sizeof(av_buffer), tmo); | |
171 | ||
172 | if (bread <= 0) | |
173 | return m_panic_defer_3(scanent, CUS callout_address, | |
174 | @@ -1064,7 +1064,7 @@ badseek: err = errno; | |
175 | if (m_sock_send(malware_daemon_ctx.sock, cmdopt[i], Ustrlen(cmdopt[i]), &errstr) < 0) | |
176 | return m_panic_defer(scanent, CUS callout_address, errstr); | |
177 | ||
178 | - bread = ip_recv(&malware_daemon_ctx, av_buffer, sizeof(av_buffer), tmo-time(NULL)); | |
179 | + bread = ip_recv(&malware_daemon_ctx, av_buffer, sizeof(av_buffer), tmo); | |
180 | if (bread > 0) av_buffer[bread]='\0'; | |
181 | if (bread < 0) | |
182 | return m_panic_defer_3(scanent, CUS callout_address, | |
183 | @@ -1096,7 +1096,7 @@ badseek: err = errno; | |
184 | { | |
185 | errno = ETIMEDOUT; | |
186 | i = av_buffer+sizeof(av_buffer)-p; | |
187 | - if ((bread= ip_recv(&malware_daemon_ctx, p, i-1, tmo-time(NULL))) < 0) | |
188 | + if ((bread= ip_recv(&malware_daemon_ctx, p, i-1, tmo)) < 0) | |
189 | return m_panic_defer_3(scanent, CUS callout_address, | |
190 | string_sprintf("unable to read result (%s)", strerror(errno)), | |
191 | malware_daemon_ctx.sock); | |
192 | @@ -1401,7 +1401,7 @@ badseek: err = errno; | |
193 | ||
194 | /* wait for result */ | |
195 | memset(av_buffer, 0, sizeof(av_buffer)); | |
196 | - if ((bread = ip_recv(&malware_daemon_ctx, av_buffer, sizeof(av_buffer), tmo-time(NULL))) <= 0) | |
197 | + if ((bread = ip_recv(&malware_daemon_ctx, av_buffer, sizeof(av_buffer), tmo)) <= 0) | |
198 | return m_panic_defer_3(scanent, CUS callout_address, | |
199 | string_sprintf("unable to read from UNIX socket (%s)", scanner_options), | |
200 | malware_daemon_ctx.sock); | |
201 | @@ -1737,7 +1737,7 @@ b_seek: err = errno; | |
202 | ||
203 | /* Read the result */ | |
204 | memset(av_buffer, 0, sizeof(av_buffer)); | |
205 | - bread = ip_recv(&malware_daemon_ctx, av_buffer, sizeof(av_buffer), tmo-time(NULL)); | |
206 | + bread = ip_recv(&malware_daemon_ctx, av_buffer, sizeof(av_buffer), tmo); | |
207 | (void)close(malware_daemon_ctx.sock); | |
208 | malware_daemon_ctx.sock = -1; | |
209 | malware_daemon_ctx.tls_ctx = NULL; | |
210 | @@ -1895,7 +1895,7 @@ b_seek: err = errno; | |
211 | return m_panic_defer(scanent, CUS callout_address, errstr); | |
212 | ||
213 | /* Read the result */ | |
214 | - bread = ip_recv(&malware_daemon_ctx, av_buffer, sizeof(av_buffer), tmo-time(NULL)); | |
215 | + bread = ip_recv(&malware_daemon_ctx, av_buffer, sizeof(av_buffer), tmo); | |
216 | ||
217 | if (bread <= 0) | |
218 | return m_panic_defer_3(scanent, CUS callout_address, | |
219 | --- a/src/routers/iplookup.c | |
220 | +++ b/src/routers/iplookup.c | |
221 | @@ -279,7 +279,7 @@ while ((hostname = string_nextinlist(&li | |
222 | /* Read the response and close the socket. If the read fails, try the | |
223 | next IP address. */ | |
224 | ||
225 | - count = ip_recv(&query_cctx, reply, sizeof(reply) - 1, ob->timeout); | |
226 | + count = ip_recv(&query_cctx, reply, sizeof(reply) - 1, time(NULL) + ob->timeout); | |
227 | (void)close(query_cctx.sock); | |
228 | if (count <= 0) | |
229 | { | |
230 | --- a/src/smtp_out.c | |
231 | +++ b/src/smtp_out.c | |
232 | @@ -587,14 +587,14 @@ Arguments: | |
233 | inblock the SMTP input block (contains holding buffer, socket, etc.) | |
234 | buffer where to put the line | |
235 | size space available for the line | |
236 | - timeout the timeout to use when reading a packet | |
237 | + timelimit deadline for reading the lime, seconds past epoch | |
238 | ||
239 | Returns: length of a line that has been put in the buffer | |
240 | -1 otherwise, with errno set | |
241 | */ | |
242 | ||
243 | static int | |
244 | -read_response_line(smtp_inblock *inblock, uschar *buffer, int size, int timeout) | |
245 | +read_response_line(smtp_inblock *inblock, uschar *buffer, int size, time_t timelimit) | |
246 | { | |
247 | uschar *p = buffer; | |
248 | uschar *ptr = inblock->ptr; | |
249 | @@ -637,7 +637,7 @@ for (;;) | |
250 | ||
251 | /* Need to read a new input packet. */ | |
252 | ||
253 | - if((rc = ip_recv(cctx, inblock->buffer, inblock->buffersize, timeout)) <= 0) | |
254 | + if((rc = ip_recv(cctx, inblock->buffer, inblock->buffersize, timelimit)) <= 0) | |
255 | { | |
256 | DEBUG(D_deliver|D_transport|D_acl) | |
257 | debug_printf_indent(errno ? " SMTP(%s)<<\n" : " SMTP(closed)<<\n", | |
258 | @@ -694,6 +694,7 @@ smtp_read_response(void * sx0, uschar * | |
259 | smtp_context * sx = sx0; | |
260 | uschar * ptr = buffer; | |
261 | int count = 0, rc; | |
262 | +time_t timelimit = time(NULL) + timeout; | |
263 | ||
264 | errno = 0; /* Ensure errno starts out zero */ | |
265 | ||
266 | @@ -713,7 +714,7 @@ response. */ | |
267 | ||
268 | for (;;) | |
269 | { | |
270 | - if ((count = read_response_line(&sx->inblock, ptr, size, timeout)) < 0) | |
271 | + if ((count = read_response_line(&sx->inblock, ptr, size, timelimit)) < 0) | |
272 | return FALSE; | |
273 | ||
274 | HDEBUG(D_transport|D_acl|D_v) | |
275 | --- a/src/spam.c | |
276 | +++ b/src/spam.c | |
277 | @@ -503,7 +503,7 @@ offset = 0; | |
278 | while ((i = ip_recv(&spamd_cctx, | |
279 | spamd_buffer + offset, | |
280 | sizeof(spamd_buffer) - offset - 1, | |
281 | - sd->timeout - time(NULL) + start)) > 0) | |
282 | + sd->timeout + start)) > 0) | |
283 | offset += i; | |
284 | spamd_buffer[offset] = '\0'; /* guard byte */ | |
285 | ||
286 | --- a/src/transports/smtp_socks.c | |
287 | +++ b/src/transports/smtp_socks.c | |
288 | @@ -129,7 +129,7 @@ switch(method) | |
289 | #ifdef TCP_QUICKACK | |
290 | (void) setsockopt(fd, IPPROTO_TCP, TCP_QUICKACK, US &off, sizeof(off)); | |
291 | #endif | |
292 | - if (!fd_ready(fd, tmo-time(NULL)) || read(fd, s, 2) != 2) | |
293 | + if (!fd_ready(fd, tmo) || read(fd, s, 2) != 2) | |
294 | return FAIL; | |
295 | HDEBUG(D_transport|D_acl|D_v) | |
296 | debug_printf_indent(" SOCKS<< %02x %02x\n", s[0], s[1]); | |
297 | @@ -320,7 +320,7 @@ HDEBUG(D_transport|D_acl|D_v) debug_prin | |
298 | (void) setsockopt(fd, IPPROTO_TCP, TCP_QUICKACK, US &off, sizeof(off)); | |
299 | #endif | |
300 | ||
301 | -if ( !fd_ready(fd, tmo-time(NULL)) | |
302 | +if ( !fd_ready(fd, tmo) | |
303 | || read(fd, buf, 2) != 2 | |
304 | ) | |
305 | goto rcv_err; | |
306 | @@ -370,7 +370,7 @@ if (send(fd, buf, size, 0) < 0) | |
307 | /* expect conn-reply (success, local(ipver, addr, port)) | |
308 | of same length as conn-request, or non-success fail code */ | |
309 | ||
310 | -if ( !fd_ready(fd, tmo-time(NULL)) | |
311 | +if ( !fd_ready(fd, tmo) | |
312 | || (size = read(fd, buf, size)) < 2 | |
313 | ) | |
314 | goto rcv_err; | |
315 | --- a/src/verify.c | |
316 | +++ b/src/verify.c | |
317 | @@ -2770,7 +2770,7 @@ for (;;) | |
318 | int size = sizeof(buffer) - (p - buffer); | |
319 | ||
320 | if (size <= 0) goto END_OFF; /* Buffer filled without seeing \n. */ | |
321 | - count = ip_recv(&ident_conn_ctx, p, size, rfc1413_query_timeout); | |
322 | + count = ip_recv(&ident_conn_ctx, p, size, time(NULL) + rfc1413_query_timeout); | |
323 | if (count <= 0) goto END_OFF; /* Read error or EOF */ | |
324 | ||
325 | /* Scan what we just read, to see if we have reached the terminating \r\n. Be |