Skip to content

Commit 04ff1c7

Browse files
committed
Address PR #570 review feedback for OCSP responder examples
- Add SO_RCVTIMEO (5s) on accepted client sockets to prevent indefinite blocking from incomplete requests - Move 64KB httpBuf/respBuf from stack to static globals - Fix SendAll infinite loop when send() returns 0 (check n <= 0) - Ignore SIGPIPE to prevent crash on client disconnect during writes - Use case-insensitive Content-Length header matching per RFC 7230 - Track error state and return nonzero from main on fatal errors - Reset ret after wolfSSL_CertManagerLoadCABuffer to avoid leaking WOLFSSL_SUCCESS (1) into error paths in ocsp-request-response.c - Add -Wextra to Makefile CFLAGS
1 parent 1f6aa9c commit 04ff1c7

3 files changed

Lines changed: 45 additions & 10 deletions

File tree

ocsp/responder/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
CC = gcc
88
WOLFSSL_INSTALL_DIR = /usr/local
9-
CFLAGS = -Wall -g -I$(WOLFSSL_INSTALL_DIR)/include
9+
CFLAGS = -Wall -Wextra -g -I$(WOLFSSL_INSTALL_DIR)/include
1010
LIBS = -L$(WOLFSSL_INSTALL_DIR)/lib -lwolfssl
1111

1212
# Uncomment for static linking:

ocsp/responder/ocsp-request-response.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ int main(int argc, char** argv)
172172
wolfSSL_CertManagerFree(tmpCm);
173173
goto cleanup;
174174
}
175+
ret = 0; /* Reset from WOLFSSL_SUCCESS (1) to 0 for error paths */
175176
wc_InitDecodedCert(&serverCert, serverCertDer,
176177
(word32)serverCertDerSz, NULL);
177178
serverCertInit = 1;
@@ -311,6 +312,7 @@ int main(int argc, char** argv)
311312
printf("Error loading CA into CertManager: %d\n", ret);
312313
goto cleanup;
313314
}
315+
ret = 0; /* Reset from WOLFSSL_SUCCESS (1) to 0 for error paths */
314316

315317
ocsp = wc_NewOCSP(cm);
316318
if (ocsp == NULL) {

ocsp/responder/ocsp-responder-http.c

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@
5252

5353
#include "ocsp-load-certs.h"
5454

55+
#include <strings.h>
56+
5557
#include <sys/socket.h>
5658
#include <netinet/in.h>
5759
#include <unistd.h>
@@ -61,6 +63,22 @@
6163

6264
static volatile int running = 1;
6365

66+
/* Case-insensitive substring search (for HTTP headers per RFC 7230) */
67+
static char* FindHeaderCI(const char* haystack, const char* needle)
68+
{
69+
size_t nLen = strlen(needle);
70+
while (*haystack) {
71+
if (strncasecmp(haystack, needle, nLen) == 0)
72+
return (char*)haystack;
73+
haystack++;
74+
}
75+
return NULL;
76+
}
77+
78+
/* Large buffers as static globals to avoid 128KB on the stack each iteration */
79+
static byte httpBuf[BUF_SZ];
80+
static byte respBuf[BUF_SZ];
81+
6482
static void sigHandler(int sig)
6583
{
6684
(void)sig;
@@ -118,8 +136,7 @@ static int RecvHttp(int fd, byte* buf, int bufSz)
118136
if (hdrEnd) {
119137
char* cl;
120138
headerEnd = (int)(hdrEnd - (char*)buf) + 4;
121-
cl = strstr((char*)buf, "Content-Length:");
122-
if (!cl) cl = strstr((char*)buf, "content-length:");
139+
cl = FindHeaderCI((char*)buf, "Content-Length:");
123140
if (cl) {
124141
long val = strtol(cl + 15, NULL, 10);
125142
if (val > 0 && val < bufSz)
@@ -152,8 +169,7 @@ static int ParsePost(const byte* http, int httpSz,
152169
if (!end) return -1;
153170
offset = (int)(end - hdr) + 4;
154171

155-
cl = strstr(hdr, "Content-Length:");
156-
if (!cl) cl = strstr(hdr, "content-length:");
172+
cl = FindHeaderCI(hdr, "Content-Length:");
157173
if (cl) {
158174
long val = strtol(cl + 15, NULL, 10);
159175
if (val <= 0 || val > httpSz - offset)
@@ -177,7 +193,7 @@ static int SendAll(int fd, const void* data, int sz)
177193
int remaining = sz;
178194
while (remaining > 0) {
179195
int n = (int)send(fd, p, (size_t)remaining, 0);
180-
if (n < 0) return -1;
196+
if (n <= 0) return -1;
181197
p += n;
182198
remaining -= n;
183199
}
@@ -219,7 +235,7 @@ int main(int argc, char** argv)
219235
int caCertInit = 0;
220236
char caSubject[256];
221237
word32 caSubjectSz = sizeof(caSubject);
222-
int sockfd = -1, clientfd, opt = 1, i;
238+
int sockfd = -1, clientfd, opt = 1, i, ret = 0;
223239
struct sockaddr_in addr;
224240

225241
if (argc < 4) {
@@ -244,37 +260,46 @@ int main(int argc, char** argv)
244260
sigemptyset(&sa.sa_mask);
245261
sigaction(SIGINT, &sa, NULL);
246262
sigaction(SIGTERM, &sa, NULL);
263+
264+
/* Ignore SIGPIPE so client disconnections during writes don't crash */
265+
sa.sa_handler = SIG_IGN;
266+
sigaction(SIGPIPE, &sa, NULL);
247267
}
248268

249269
caCertDer = LoadCertDer(certFile, &caCertDerSz);
250270
caKeyDer = LoadKeyDer(keyFile, &caKeyDerSz);
251271
if (!caCertDer || !caKeyDer) {
252272
fprintf(stderr, "Error loading cert/key\n");
273+
ret = -1;
253274
goto cleanup;
254275
}
255276

256277
wc_InitDecodedCert(&caCert, caCertDer, (word32)caCertDerSz, NULL);
257278
caCertInit = 1;
258279
if (wc_ParseCert(&caCert, CERT_TYPE, 0, NULL) != 0) {
259280
fprintf(stderr, "Error parsing CA cert\n");
281+
ret = -1;
260282
goto cleanup;
261283
}
262284

263285
if (wc_GetDecodedCertSubject(&caCert, caSubject, &caSubjectSz) != 0) {
264286
fprintf(stderr, "Error getting CA subject\n");
287+
ret = -1;
265288
goto cleanup;
266289
}
267290

268291
responder = wc_OcspResponder_new(NULL, 1);
269292
if (!responder) {
270293
fprintf(stderr, "Error creating responder\n");
294+
ret = -1;
271295
goto cleanup;
272296
}
273297

274298
if (wc_OcspResponder_AddSigner(responder, caCertDer, (word32)caCertDerSz,
275299
caKeyDer, (word32)caKeyDerSz,
276300
NULL, 0) != 0) {
277301
fprintf(stderr, "Error adding signer\n");
302+
ret = -1;
278303
goto cleanup;
279304
}
280305

@@ -289,10 +314,12 @@ int main(int argc, char** argv)
289314
sockfd = socket(AF_INET, SOCK_STREAM, 0);
290315
if (sockfd < 0) {
291316
perror("socket");
317+
ret = -1;
292318
goto cleanup;
293319
}
294320
if (setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)) < 0) {
295321
perror("setsockopt");
322+
ret = -1;
296323
goto cleanup;
297324
}
298325
memset(&addr, 0, sizeof(addr));
@@ -302,24 +329,30 @@ int main(int argc, char** argv)
302329

303330
if (bind(sockfd, (struct sockaddr*)&addr, sizeof(addr)) < 0) {
304331
perror("bind");
332+
ret = -1;
305333
goto cleanup;
306334
}
307335
if (listen(sockfd, 5) < 0) {
308336
perror("listen");
337+
ret = -1;
309338
goto cleanup;
310339
}
311340
printf("OCSP responder listening on port %d\n", port);
312341

313342
while (running) {
314-
byte httpBuf[BUF_SZ];
315-
byte respBuf[BUF_SZ];
316343
word32 respSz;
317344
const byte* ocspReq;
318345
int ocspReqSz, recvLen;
346+
struct timeval tv;
319347

320348
clientfd = accept(sockfd, NULL, NULL);
321349
if (clientfd < 0) continue;
322350

351+
/* Set receive timeout so incomplete requests don't block forever */
352+
tv.tv_sec = 5;
353+
tv.tv_usec = 0;
354+
setsockopt(clientfd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
355+
323356
recvLen = RecvHttp(clientfd, httpBuf, BUF_SZ);
324357
if (recvLen <= 0 ||
325358
ParsePost(httpBuf, recvLen, &ocspReq, &ocspReqSz) != 0) {
@@ -349,7 +382,7 @@ int main(int argc, char** argv)
349382
free(caCertDer);
350383
free(caKeyDer);
351384
wolfSSL_Cleanup();
352-
return 0;
385+
return ret;
353386
}
354387

355388
#else

0 commit comments

Comments
 (0)