From 15e16a99ba34fbac68867416b3a3d5353ebe022f Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Fri, 14 May 2021 22:11:24 +0200 Subject: [PATCH 01/20] mxshadowsrv: Refactor some automatics into statics Make some automatic variables, which will be needed by other threads, into statics. This avoids uneeded data structures to communicate this data into threads. SSL_CTX is reference counted and thread safe (if not modified). Keep a copy in an automatic variable to lose our reference via free_ssl_ctx()->SSL_CTX_free. --- mxshadowsrv.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/mxshadowsrv.c b/mxshadowsrv.c index 76b53c8..5c1eb94 100644 --- a/mxshadowsrv.c +++ b/mxshadowsrv.c @@ -61,6 +61,10 @@ static void unmap_shadow(char *shadow_buf, struct stat *statbuf) { if (sts == -1) { perror("munmap"); exit(1); } } +char *shadow_buf = NULL; +struct stat statbuf; +SSL_CTX *ssl_ctx; + static void validate_shadow(char **shadow_buf, char *filename, struct stat *statbuf) { if (*shadow_buf == NULL) *shadow_buf = map_shadow(filename, statbuf); @@ -169,8 +173,9 @@ int main(int argc, char **argv) { die_usage(argv[0]); char *filename = argv[optind++]; - SSL_CTX *ssl_ctx _cleanup_(free_ssl_ctx) = SSL_CTX_new(TLS_server_method()); - if (ssl_ctx == NULL) { psslerror("SSL_CTX_new"); return 1; } + SSL_CTX *_ssl_ctx _cleanup_(free_ssl_ctx) = SSL_CTX_new(TLS_server_method()); + if (_ssl_ctx == NULL) { psslerror("SSL_CTX_new"); return 1; } + ssl_ctx = _ssl_ctx; if (SSL_CTX_use_PrivateKey_file(ssl_ctx, key_file, SSL_FILETYPE_PEM) <= 0 ) { psslerror("SSL_CTX_use_PrivateKey_file"); return 1; } if (SSL_CTX_use_certificate_file(ssl_ctx, cert_file, SSL_FILETYPE_PEM) <= 0) { psslerror("SSL_CTX_use_certificate_file"); return 1; } @@ -192,8 +197,6 @@ int main(int argc, char **argv) { status = listen(listen_socket, 40); if (status == -1) { perror("listen"); exit(1); } - char *shadow_buf = NULL; - static struct stat statbuf; while (1) { int _cleanup_(free_fd) socket = accept4(listen_socket, NULL, NULL, SOCK_NONBLOCK); if (socket == -1 ) { perror("accept"); exit(1); } From 9cdc60d545d8da95639531de0a172d23eadbd9c6 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Fri, 14 May 2021 22:24:07 +0200 Subject: [PATCH 02/20] mxshadowsrv: Factor out processing of a single client We want to process each client in a individual thread. This requires a separate function per client. Factor that out. --- mxshadowsrv.c | 67 +++++++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/mxshadowsrv.c b/mxshadowsrv.c index 5c1eb94..e602c0f 100644 --- a/mxshadowsrv.c +++ b/mxshadowsrv.c @@ -134,6 +134,40 @@ static void die_usage(char *argv0) { exit(1); } +static void process_client(int socket) { + SSL *ssl _cleanup_(free_ssl) = SSL_new(ssl_ctx); + if (ssl == NULL) { psslerror("SSL_new"); return; } + SSL_set_fd(ssl, socket); + if (ssl_accept_with_timeout(ssl, socket, 1000) <= 0) { + perror("accept"); + return; + } + + char buf[64]; + int len = ssl_read_with_timeout(ssl, socket, buf, sizeof(buf), 1000); + if (len == 0) + return; + if (len < 0) { + perror("read"); + return; + } + if (len == sizeof(buf)) { + fprintf(stderr, "identifier to long\n"); + SSL_shutdown(ssl); + return; + } + + char *line; + int line_len; + line_len = find_in_shadow(buf, len, shadow_buf, statbuf.st_size, &line); + if (line_len) { + int status = ssl_write_with_timeout(ssl, socket, line, line_len, 1000); + if (status == -1) + perror("write"); + } + SSL_shutdown(ssl); +} + int main(int argc, char **argv) { char *key_file = NULL; @@ -201,38 +235,7 @@ int main(int argc, char **argv) { int _cleanup_(free_fd) socket = accept4(listen_socket, NULL, NULL, SOCK_NONBLOCK); if (socket == -1 ) { perror("accept"); exit(1); } validate_shadow(&shadow_buf, filename, &statbuf); - - SSL *ssl _cleanup_(free_ssl) = SSL_new(ssl_ctx); - if (ssl == NULL) { psslerror("SSL_new"); return 1; } - SSL_set_fd(ssl, socket); - if (ssl_accept_with_timeout(ssl, socket, 1000) <= 0) { - perror("accept"); - continue; - } - - char buf[64]; - int len = ssl_read_with_timeout(ssl, socket, buf, sizeof(buf), 1000); - if (len == 0) - continue; - if (len < 0) { - perror("read"); - continue; - } - if (len == sizeof(buf)) { - fprintf(stderr, "identifier to long\n"); - SSL_shutdown(ssl); - continue; - } - - char *line; - int line_len; - line_len = find_in_shadow(buf, len, shadow_buf, statbuf.st_size, &line); - if (line_len) { - int status = ssl_write_with_timeout(ssl, socket, line, line_len, 1000); - if (status == -1) - perror("write"); - } - SSL_shutdown(ssl); + process_client(socket); } } From 1925314442eeb9fa11c8d94194659bf57c087537 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Sat, 15 May 2021 12:31:18 +0200 Subject: [PATCH 03/20] mxshadowsrv: Protect shadow buffer with a mutex Use mutex to protect shadow_buf and statbuf. --- mxshadowsrv.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/mxshadowsrv.c b/mxshadowsrv.c index e602c0f..969432f 100644 --- a/mxshadowsrv.c +++ b/mxshadowsrv.c @@ -6,6 +6,7 @@ #include #include #include +#include #include "common.h" @@ -61,17 +62,22 @@ static void unmap_shadow(char *shadow_buf, struct stat *statbuf) { if (sts == -1) { perror("munmap"); exit(1); } } -char *shadow_buf = NULL; -struct stat statbuf; +pthread_mutex_t shadow_mutex = PTHREAD_MUTEX_INITIALIZER ; +char *shadow_buf = NULL; // protected by shadow_mutex +struct stat statbuf; // protected by shadow_mutex SSL_CTX *ssl_ctx; static void validate_shadow(char **shadow_buf, char *filename, struct stat *statbuf) { + int status = pthread_mutex_lock(&shadow_mutex); + if (status != 0) { errno = status; perror("pthread_mutex_lock"); exit(1);} if (*shadow_buf == NULL) *shadow_buf = map_shadow(filename, statbuf); while(shadow_is_changed(filename, statbuf)) { unmap_shadow(*shadow_buf, statbuf); *shadow_buf = map_shadow(filename, statbuf); } + status = pthread_mutex_unlock(&shadow_mutex); + if (status != 0) { errno = status; perror("pthread_mutex_unlock"); exit(1);} } static int find_in_shadow(char *username, int username_len, char *shadow_buf, size_t shadow_len, char **lineptr) { @@ -157,9 +163,16 @@ static void process_client(int socket) { return; } + int status = pthread_mutex_lock(&shadow_mutex); + if (status != 0) { errno = status; perror("pthread_mutex_lock"); exit(1);} + char *line; int line_len; line_len = find_in_shadow(buf, len, shadow_buf, statbuf.st_size, &line); + + status = pthread_mutex_unlock(&shadow_mutex); + if (status != 0) { errno = status; perror("pthread_mutex_unlock"); exit(1);} + if (line_len) { int status = ssl_write_with_timeout(ssl, socket, line, line_len, 1000); if (status == -1) From 37ce5b32c3f185bc264fbdc1dbfd1d27b33feff2 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Sat, 15 May 2021 12:51:58 +0200 Subject: [PATCH 04/20] mxshadowsrv: Process clients in threads --- mxshadowsrv.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/mxshadowsrv.c b/mxshadowsrv.c index 969432f..a09a739 100644 --- a/mxshadowsrv.c +++ b/mxshadowsrv.c @@ -7,6 +7,7 @@ #include #include #include +#include #include "common.h" @@ -62,10 +63,13 @@ static void unmap_shadow(char *shadow_buf, struct stat *statbuf) { if (sts == -1) { perror("munmap"); exit(1); } } +#define MAX_THREADS 8 + pthread_mutex_t shadow_mutex = PTHREAD_MUTEX_INITIALIZER ; char *shadow_buf = NULL; // protected by shadow_mutex struct stat statbuf; // protected by shadow_mutex SSL_CTX *ssl_ctx; +sem_t free_worker; static void validate_shadow(char **shadow_buf, char *filename, struct stat *statbuf) { int status = pthread_mutex_lock(&shadow_mutex); @@ -181,6 +185,16 @@ static void process_client(int socket) { SSL_shutdown(ssl); } +static void *client_thread(void *arg) { + int socket = *((int *)arg); + process_client(socket); + close(socket); + free(arg); + int status = sem_post(&free_worker); + if (status != 0) { errno = status; perror("sem_post"); exit(1); } + return NULL; +} + int main(int argc, char **argv) { char *key_file = NULL; @@ -244,11 +258,24 @@ int main(int argc, char **argv) { status = listen(listen_socket, 40); if (status == -1) { perror("listen"); exit(1); } + status = sem_init(&free_worker, 0, MAX_THREADS); + if (status) { errno = status; perror("sem_init"); exit(1); } + while (1) { int _cleanup_(free_fd) socket = accept4(listen_socket, NULL, NULL, SOCK_NONBLOCK); if (socket == -1 ) { perror("accept"); exit(1); } + status = sem_wait(&free_worker); + if (status == -1) { perror("sem_wait"); exit(1); } validate_shadow(&shadow_buf, filename, &statbuf); - process_client(socket); - } + pthread_t thread; + int *arg = malloc(sizeof *arg); + if (arg == NULL) { perror("malloc"); exit(1); } + *arg = socket; + status = pthread_create(&thread, NULL, client_thread, arg); + if (status != 0) { errno = status; perror("pthread_create"); exit(1); } + socket = -1; + status = pthread_detach(thread); + if (status != 0) { errno = status; perror("pthread_detach"); exit(1); } + } } From 19ed6a6cd6926651a583d4b13cbb961e3755b86d Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Mon, 17 May 2021 10:07:06 +0200 Subject: [PATCH 05/20] mxshadowsrv: Increase timeout from 1 to 30 seconds As clients are handled in individual threads now, individual client or communication problems can no longer spoil the game for other clients, so relace the timeout for client responses. --- mxshadowsrv.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mxshadowsrv.c b/mxshadowsrv.c index a09a739..e8145e0 100644 --- a/mxshadowsrv.c +++ b/mxshadowsrv.c @@ -64,6 +64,7 @@ static void unmap_shadow(char *shadow_buf, struct stat *statbuf) { } #define MAX_THREADS 8 +#define TIMEOUT 30000 // client timeout in msec pthread_mutex_t shadow_mutex = PTHREAD_MUTEX_INITIALIZER ; char *shadow_buf = NULL; // protected by shadow_mutex @@ -148,13 +149,13 @@ static void process_client(int socket) { SSL *ssl _cleanup_(free_ssl) = SSL_new(ssl_ctx); if (ssl == NULL) { psslerror("SSL_new"); return; } SSL_set_fd(ssl, socket); - if (ssl_accept_with_timeout(ssl, socket, 1000) <= 0) { + if (ssl_accept_with_timeout(ssl, socket, TIMEOUT) <= 0) { perror("accept"); return; } char buf[64]; - int len = ssl_read_with_timeout(ssl, socket, buf, sizeof(buf), 1000); + int len = ssl_read_with_timeout(ssl, socket, buf, sizeof(buf), TIMEOUT); if (len == 0) return; if (len < 0) { @@ -178,7 +179,7 @@ static void process_client(int socket) { if (status != 0) { errno = status; perror("pthread_mutex_unlock"); exit(1);} if (line_len) { - int status = ssl_write_with_timeout(ssl, socket, line, line_len, 1000); + int status = ssl_write_with_timeout(ssl, socket, line, line_len, TIMEOUT); if (status == -1) perror("write"); } From e28a6155230a49e2ac0a8db8f81dd335982703d4 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Sat, 15 May 2021 14:19:13 +0200 Subject: [PATCH 06/20] mxshadowsrv: Add exit condition for debugging --- Makefile | 7 +++++-- mxshadowsrv.c | 22 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 8ff1dee..1dd5062 100644 --- a/Makefile +++ b/Makefile @@ -24,8 +24,11 @@ INSTALL=install -v INSTALL_PROGRAM = $(INSTALL) INSTALL_DATA = $(INSTALL) -m 644 -CFLAGS=-O3 -Wall -Werror -#CFLAGS=-O0 -g -Wall +ifdef DEBUG + CFLAGS=-O0 -g -Wall -DDEBUG_MAX_CONNECTS=10 +else + CFLAGS=-O3 -Wall -Werror +endif all: mxshadowsrv libnss_mxshadow.so.2 test_server test_query_shadow diff --git a/mxshadowsrv.c b/mxshadowsrv.c index e8145e0..d8a5910 100644 --- a/mxshadowsrv.c +++ b/mxshadowsrv.c @@ -8,6 +8,9 @@ #include #include #include +#ifdef DEBUG_MAX_CONNECTS +#include +#endif #include "common.h" @@ -71,6 +74,9 @@ char *shadow_buf = NULL; // protected by shadow_mutex struct stat statbuf; // protected by shadow_mutex SSL_CTX *ssl_ctx; sem_t free_worker; +#ifdef DEBUG_MAX_CONNECTS +static int debug_remaining_connects = DEBUG_MAX_CONNECTS; +#endif static void validate_shadow(char **shadow_buf, char *filename, struct stat *statbuf) { int status = pthread_mutex_lock(&shadow_mutex); @@ -263,6 +269,10 @@ int main(int argc, char **argv) { if (status) { errno = status; perror("sem_init"); exit(1); } while (1) { +#ifdef DEBUG_MAX_CONNECTS + if (debug_remaining_connects-- == 0) + break; +#endif int _cleanup_(free_fd) socket = accept4(listen_socket, NULL, NULL, SOCK_NONBLOCK); if (socket == -1 ) { perror("accept"); exit(1); } status = sem_wait(&free_worker); @@ -277,6 +287,16 @@ int main(int argc, char **argv) { socket = -1; status = pthread_detach(thread); if (status != 0) { errno = status; perror("pthread_detach"); exit(1); } + } +#ifdef DEBUG_MAX_CONNECTS + for (int i=0; i Date: Mon, 17 May 2021 10:31:10 +0200 Subject: [PATCH 07/20] get_shadow_line: Increase timeout from 1 to 5 seconds Increase timeout so we have a better chance to get through short network problems or server overload conditions. --- get_shadow_line.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/get_shadow_line.c b/get_shadow_line.c index 25bd55f..fabdccf 100644 --- a/get_shadow_line.c +++ b/get_shadow_line.c @@ -10,6 +10,8 @@ #include "common.h" #include "get_shadow_line.h" +#define TIMEOUT 5000 + static char conf_filename[] = "/etc/mxshadow.conf"; static int read_config(struct sockaddr_in *addr) { @@ -96,7 +98,7 @@ int get_shadow_line(char *user, char **line) { int sock = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0); if (sock == 0) { perror("socket"); return -1; } - status = connect_with_timeout(sock, (struct sockaddr *)&sockaddr, sizeof(sockaddr), 1000); + status = connect_with_timeout(sock, (struct sockaddr *)&sockaddr, sizeof(sockaddr), TIMEOUT); if (status == -1) { perror("connect"); return -1; } SSL *ssl _cleanup_(free_ssl) = SSL_new(ssl_ctx); @@ -108,7 +110,7 @@ int get_shadow_line(char *user, char **line) { if (status == 0) { psslerror("SSL_set_fd"); return -1; } int len = strlen(user); - status = ssl_write_with_timeout(ssl, sock, user, len, 1000); + status = ssl_write_with_timeout(ssl, sock, user, len, TIMEOUT); if (status == -1) { fprintf(stderr, "ssl_write_with_timeout failed\n"); return -1; @@ -118,7 +120,7 @@ int get_shadow_line(char *user, char **line) { if (buffer == NULL) return -1; - len = ssl_read_with_timeout(ssl, sock, buffer, BUFLEN_SPWD, 1000); + len = ssl_read_with_timeout(ssl, sock, buffer, BUFLEN_SPWD, TIMEOUT); if (len<0) return -1; SSL_shutdown(ssl); From 6c4cc10911933df91ad7bbe6601b6dac8c603c5a Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Mon, 17 May 2021 13:37:31 +0200 Subject: [PATCH 08/20] common.h: Make logging to stderr optional --- common.h | 37 ++++++++++++++++++++++++------------- get_shadow_line.c | 14 +++++++------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/common.h b/common.h index db41a92..132d4aa 100644 --- a/common.h +++ b/common.h @@ -2,10 +2,14 @@ #include #include #include -#include #define _cleanup_(x) __attribute__((cleanup(x))) +#ifndef COMMON_LOG +#include +#define COMMON_LOG(prio, msg, ...) (fprintf(stderr, msg "\n", ## __VA_ARGS__ )) +#endif + static void __attribute__((unused)) free_ssl_ctx(SSL_CTX **ctxp) { if (*ctxp) { SSL_CTX_free(*ctxp); @@ -37,8 +41,15 @@ static void __attribute__((unused)) free_string(char **ptr) { } static void __attribute__((unused)) psslerror(char *str) { - fprintf(stderr, "%s\n", str); - ERR_print_errors_fp(stderr); + COMMON_LOG(LOG_ERR, "%s:", str); + unsigned long ssl_err; + while ((ssl_err = ERR_get_error())) { + COMMON_LOG(LOG_ERR, "ssl error: %lud:%s:%s:%s", + ssl_err, + ERR_lib_error_string(ssl_err), + ERR_func_error_string(ssl_err), + ERR_reason_error_string(ssl_err)); + } } static int wait_rd_with_timeout(int fd, int timeout) { @@ -77,7 +88,7 @@ static int __attribute__((unused)) ssl_write_with_timeout(SSL *ssl, int fd, char if (status > 0) { if (status == datalen) return 0; - fprintf(stderr, "%s: unexpected partial write. requested %lud bytes, returned: %d\n", __func__, datalen, status); + COMMON_LOG(LOG_ERR, "%s: unexpected partial write. requested %lud bytes, returned: %d", __func__, datalen, status); return -1; } int ssl_error = SSL_get_error(ssl, status); @@ -88,14 +99,14 @@ static int __attribute__((unused)) ssl_write_with_timeout(SSL *ssl, int fd, char return -1; continue; case SSL_ERROR_SYSCALL: - perror(__func__); + COMMON_LOG(LOG_ERR, "%s: %m", __func__); return -1; case SSL_ERROR_SSL: - ERR_print_errors_fp(stderr); + psslerror(""); errno = EPROTO; return -1; default: - fprintf(stderr, "%s: %s unimplemented\n", __func__, ssl_err(ssl_error)); + COMMON_LOG(LOG_ERR, "%s: %s unimplemented", __func__, ssl_err(ssl_error)); errno = ENOSYS; return -1; } @@ -116,15 +127,15 @@ static int __attribute__((unused)) ssl_read_with_timeout(SSL *ssl, int fd, void continue; case SSL_ERROR_SYSCALL: if (errno==0) { - fprintf(stderr, "%s: unexpected EOF from peer\n", __func__); + COMMON_LOG(LOG_ERR, "%s: unexpected EOF from peer", __func__); return -1; } - perror(__func__); + COMMON_LOG(LOG_ERR, "%s: %m", __func__); return -1; case SSL_ERROR_ZERO_RETURN: return 0; default: - fprintf(stderr, "%s: %s unimplemented\n", __func__, ssl_err(ssl_error)); + COMMON_LOG(LOG_ERR, "%s: %s unimplemented", __func__, ssl_err(ssl_error)); errno = ENOSYS; return -1; } @@ -144,14 +155,14 @@ static int __attribute__((unused)) ssl_accept_with_timeout(SSL *ssl, int fd, in return -1; continue; case SSL_ERROR_SYSCALL: - perror(__func__); + COMMON_LOG(LOG_ERR, "%s: %m", __func__); return -1; case SSL_ERROR_SSL: - ERR_print_errors_fp(stderr); + psslerror(""); errno = EPROTO; return -1; default: - fprintf(stderr, "%s: %s unimplemented\n", __func__, ssl_err(ssl_error)); + COMMON_LOG(LOG_ERR, "%s: %s unimplemented", __func__, ssl_err(ssl_error)); errno = ENOSYS; return -1; } diff --git a/get_shadow_line.c b/get_shadow_line.c index fabdccf..11d3b86 100644 --- a/get_shadow_line.c +++ b/get_shadow_line.c @@ -19,7 +19,7 @@ static int read_config(struct sockaddr_in *addr) { char *line _cleanup_(free_string) = NULL; size_t n = 0; f = fopen(conf_filename, "r"); - if (f == NULL) { perror(conf_filename); return(-1); } + if (f == NULL) { COMMON_LOG(LOG_ERR, "%s: %m", conf_filename); return(-1); } char *server _cleanup_(free_string) = NULL; int port = -1; while (1) { @@ -27,7 +27,7 @@ static int read_config(struct sockaddr_in *addr) { if (status == -1) { if (feof(f)) break; - perror(conf_filename); + COMMON_LOG(LOG_ERR, "%s: %m", conf_filename); return -1; } char *comment = strchr(line, '#'); @@ -38,12 +38,12 @@ static int read_config(struct sockaddr_in *addr) { sscanf(line, "port = %u \n", &port); } if (server == NULL || port == -1) { - fprintf(stderr, "%s: does not set server and port\n", conf_filename); + COMMON_LOG(LOG_ERR, "%s: does not set server and port", conf_filename); return -1; } int status = inet_aton(server, &addr->sin_addr); if (status == 0) { - fprintf(stderr, "%s: not a invalid ip address: %s\n", conf_filename, server); + COMMON_LOG(LOG_ERR, "%s: not a invalid ip address: %s\n", conf_filename, server); } addr->sin_port = htons(port); return 0; @@ -96,10 +96,10 @@ int get_shadow_line(char *user, char **line) { if (SSL_CTX_load_verify_locations(ssl_ctx, "/etc/mxshadow.cert.pem", NULL) == 0) { psslerror("SSL_CTX_load_verify_locations"); return -1; } int sock = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0); - if (sock == 0) { perror("socket"); return -1; } + if (sock == 0) { COMMON_LOG(LOG_ERR, "socket: %m"); return -1; } status = connect_with_timeout(sock, (struct sockaddr *)&sockaddr, sizeof(sockaddr), TIMEOUT); - if (status == -1) { perror("connect"); return -1; } + if (status == -1) { COMMON_LOG(LOG_ERR, "connect: %m"); return -1; } SSL *ssl _cleanup_(free_ssl) = SSL_new(ssl_ctx); if (ssl == NULL) { psslerror("SSL_new"); return -1; } @@ -112,7 +112,7 @@ int get_shadow_line(char *user, char **line) { int len = strlen(user); status = ssl_write_with_timeout(ssl, sock, user, len, TIMEOUT); if (status == -1) { - fprintf(stderr, "ssl_write_with_timeout failed\n"); + COMMON_LOG(LOG_ERR, "ssl_write_with_timeout failed"); return -1; } From bf6e3d97a7f83d9211d13fe3f6aa424d59349af6 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Mon, 17 May 2021 13:41:13 +0200 Subject: [PATCH 09/20] common.h: Add include guard --- common.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/common.h b/common.h index 132d4aa..8c4cd61 100644 --- a/common.h +++ b/common.h @@ -1,3 +1,6 @@ +#ifndef _COMMON_H +#define _COMMON_H + #include #include #include @@ -168,3 +171,5 @@ static int __attribute__((unused)) ssl_accept_with_timeout(SSL *ssl, int fd, in } } } + +#endif /* _COMMON_H */ From ab9d3ac735088f513865125c73021302844e24f8 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Mon, 17 May 2021 13:54:19 +0200 Subject: [PATCH 10/20] common.h, get_shadow_line: Always set errno on failures Always return some more or less sensible value in errno for failing functions. --- common.h | 2 ++ get_shadow_line.c | 3 +++ 2 files changed, 5 insertions(+) diff --git a/common.h b/common.h index 8c4cd61..4bdb577 100644 --- a/common.h +++ b/common.h @@ -92,6 +92,7 @@ static int __attribute__((unused)) ssl_write_with_timeout(SSL *ssl, int fd, char if (status == datalen) return 0; COMMON_LOG(LOG_ERR, "%s: unexpected partial write. requested %lud bytes, returned: %d", __func__, datalen, status); + errno = EPIPE; return -1; } int ssl_error = SSL_get_error(ssl, status); @@ -131,6 +132,7 @@ static int __attribute__((unused)) ssl_read_with_timeout(SSL *ssl, int fd, void case SSL_ERROR_SYSCALL: if (errno==0) { COMMON_LOG(LOG_ERR, "%s: unexpected EOF from peer", __func__); + errno = ECONNABORTED; return -1; } COMMON_LOG(LOG_ERR, "%s: %m", __func__); diff --git a/get_shadow_line.c b/get_shadow_line.c index 11d3b86..0ae35da 100644 --- a/get_shadow_line.c +++ b/get_shadow_line.c @@ -39,11 +39,14 @@ static int read_config(struct sockaddr_in *addr) { } if (server == NULL || port == -1) { COMMON_LOG(LOG_ERR, "%s: does not set server and port", conf_filename); + errno = EINVAL; return -1; } int status = inet_aton(server, &addr->sin_addr); if (status == 0) { COMMON_LOG(LOG_ERR, "%s: not a invalid ip address: %s\n", conf_filename, server); + errno = EINVAL; + return -1; } addr->sin_port = htons(port); return 0; From 85cb22aaad91c3b1fa14e3785d2beed4109169dc Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Mon, 17 May 2021 14:32:05 +0200 Subject: [PATCH 11/20] libnss_mxshadow: Include get_shadow_line.c from source We want libnss_mxshadow.c and get_shadow_line.c in a single compilation unit, so that the preprocessor macros are in the same namespace. Include get_shadow_line.c from source not from the command line. --- Makefile | 4 ++-- libnss_mxshadow.c | 2 +- test_server.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 1dd5062..3c1441c 100644 --- a/Makefile +++ b/Makefile @@ -36,10 +36,10 @@ get_shadow_line.o: get_shadow_line.c get_shadow_line.h common.h gcc $(CFLAGS) -c -fPIC -o get_shadow_line.o get_shadow_line.c libnss_mxshadow.so.2: libnss_mxshadow.c get_shadow_line.c common.h - gcc $(CFLAGS) -shared -o libnss_mxshadow.so.2 -Wl,-soname,libnss_mxshadow.so.2,-unresolved-symbols=report-all -fPIC libnss_mxshadow.c get_shadow_line.c -l:libssl.a -l:libcrypto.a -lpthread -ldl + gcc $(CFLAGS) -shared -o libnss_mxshadow.so.2 -Wl,-soname,libnss_mxshadow.so.2,-unresolved-symbols=report-all -fPIC libnss_mxshadow.c -l:libssl.a -l:libcrypto.a -lpthread -ldl test_server: test_server.c get_shadow_line.c common.h get_shadow_line.h - gcc $(CFLAGS) -o test_server test_server.c get_shadow_line.c -l:libssl.a -l:libcrypto.a -lpthread -ldl + gcc $(CFLAGS) -o test_server test_server.c -l:libssl.a -l:libcrypto.a -lpthread -ldl mxshadowsrv: mxshadowsrv.c common.h gcc $(CFLAGS) -o mxshadowsrv mxshadowsrv.c -l:libssl.a -l:libcrypto.a -lpthread -ldl diff --git a/libnss_mxshadow.c b/libnss_mxshadow.c index 09a6e1d..a0d18ba 100644 --- a/libnss_mxshadow.c +++ b/libnss_mxshadow.c @@ -2,7 +2,7 @@ #include #include -#include "get_shadow_line.h" +#include "get_shadow_line.c" #include "common.h" enum nss_status _nss_mxshadow_getspnam_r(const char *name, struct spwd *spwd, char *buffer, size_t buflen, int *errnop) { diff --git a/test_server.c b/test_server.c index 7548432..71b53ac 100644 --- a/test_server.c +++ b/test_server.c @@ -1,7 +1,7 @@ #include #include -#include "get_shadow_line.h" +#include "get_shadow_line.c" int main(int argc, char **argv) { if (argc != 2) { From 8c2723e7c4878357913b6564f32b3bb5f7dde039 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Mon, 17 May 2021 14:34:58 +0200 Subject: [PATCH 12/20] libnss_mxshadow: Log to syslog Do not assume that stderr is available for logging, because the calling application might reuse fd2 for antohter purpose (at least in theorie). Redirect diagnostincs to syslog. --- libnss_mxshadow.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libnss_mxshadow.c b/libnss_mxshadow.c index a0d18ba..6f5243e 100644 --- a/libnss_mxshadow.c +++ b/libnss_mxshadow.c @@ -1,7 +1,9 @@ #include #include #include +#include +#define COMMON_LOG(prio, msg, ...) (syslog(LOG_AUTH|prio, msg, ## __VA_ARGS__ )) #include "get_shadow_line.c" #include "common.h" From 831d5ae7ff86cfda9353c771f9c25c53948d1ff8 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Mon, 17 May 2021 14:38:06 +0200 Subject: [PATCH 13/20] Makefile: Remove get_shadow_line.o We include get_shadow_line.c, we don't link against it. Remove uneeded target. --- Makefile | 3 --- 1 file changed, 3 deletions(-) diff --git a/Makefile b/Makefile index 3c1441c..ecee59a 100644 --- a/Makefile +++ b/Makefile @@ -32,9 +32,6 @@ endif all: mxshadowsrv libnss_mxshadow.so.2 test_server test_query_shadow -get_shadow_line.o: get_shadow_line.c get_shadow_line.h common.h - gcc $(CFLAGS) -c -fPIC -o get_shadow_line.o get_shadow_line.c - libnss_mxshadow.so.2: libnss_mxshadow.c get_shadow_line.c common.h gcc $(CFLAGS) -shared -o libnss_mxshadow.so.2 -Wl,-soname,libnss_mxshadow.so.2,-unresolved-symbols=report-all -fPIC libnss_mxshadow.c -l:libssl.a -l:libcrypto.a -lpthread -ldl From 5b7dfdb5580ddd4a6a986b7e8c7772ce0cce2159 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Mon, 17 May 2021 14:44:10 +0200 Subject: [PATCH 14/20] Remove get_shadow_line.h --- Makefile | 2 +- get_shadow_line.c | 1 - get_shadow_line.h | 6 ------ 3 files changed, 1 insertion(+), 8 deletions(-) delete mode 100644 get_shadow_line.h diff --git a/Makefile b/Makefile index ecee59a..3bde4d7 100644 --- a/Makefile +++ b/Makefile @@ -35,7 +35,7 @@ all: mxshadowsrv libnss_mxshadow.so.2 test_server test_query_shadow libnss_mxshadow.so.2: libnss_mxshadow.c get_shadow_line.c common.h gcc $(CFLAGS) -shared -o libnss_mxshadow.so.2 -Wl,-soname,libnss_mxshadow.so.2,-unresolved-symbols=report-all -fPIC libnss_mxshadow.c -l:libssl.a -l:libcrypto.a -lpthread -ldl -test_server: test_server.c get_shadow_line.c common.h get_shadow_line.h +test_server: test_server.c get_shadow_line.c common.h gcc $(CFLAGS) -o test_server test_server.c -l:libssl.a -l:libcrypto.a -lpthread -ldl mxshadowsrv: mxshadowsrv.c common.h diff --git a/get_shadow_line.c b/get_shadow_line.c index 0ae35da..e3f4f73 100644 --- a/get_shadow_line.c +++ b/get_shadow_line.c @@ -8,7 +8,6 @@ #include #include "common.h" -#include "get_shadow_line.h" #define TIMEOUT 5000 diff --git a/get_shadow_line.h b/get_shadow_line.h deleted file mode 100644 index 3c0811d..0000000 --- a/get_shadow_line.h +++ /dev/null @@ -1,6 +0,0 @@ -#ifndef _GET_SHADOW_LINE_H -#define _GET_SHADOW_LINE_H - -int get_shadow_line(char *user, char **line); - -#endif /* _GET_SHADOW_LINE_H */ From 07ac44619765ddd8b444d56d183b5beed44a4180 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Mon, 17 May 2021 14:45:29 +0200 Subject: [PATCH 15/20] Makefile: Add target test_query_shadow --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index 3bde4d7..10e4dbe 100644 --- a/Makefile +++ b/Makefile @@ -38,6 +38,9 @@ libnss_mxshadow.so.2: libnss_mxshadow.c get_shadow_line.c common.h test_server: test_server.c get_shadow_line.c common.h gcc $(CFLAGS) -o test_server test_server.c -l:libssl.a -l:libcrypto.a -lpthread -ldl +test_query_shadow: test_query_shadow.c get_shadow_line.c common.h + gcc $(CFLAGS) -o test_query_shadow test_query_shadow.c -l:libssl.a -l:libcrypto.a -lpthread -ldl + mxshadowsrv: mxshadowsrv.c common.h gcc $(CFLAGS) -o mxshadowsrv mxshadowsrv.c -l:libssl.a -l:libcrypto.a -lpthread -ldl From c00277cb810473995379fdba7ccb48a0bb2dbb3d Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Mon, 17 May 2021 14:46:13 +0200 Subject: [PATCH 16/20] Makefile: Fix typo --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 10e4dbe..08ca231 100644 --- a/Makefile +++ b/Makefile @@ -45,7 +45,7 @@ mxshadowsrv: mxshadowsrv.c common.h gcc $(CFLAGS) -o mxshadowsrv mxshadowsrv.c -l:libssl.a -l:libcrypto.a -lpthread -ldl clean: - @rm *.o libnss_mxshadow.so.2 mxshadowsrv test_server test_query_shadow >/dev/null || true + @rm *.o libnss_mxshadow.so.2 mxshadowsrv test_server test_query_shadow 2>/dev/null || true install: all $(INSTALL) -D -t $(DESTDIR)$(usr_sbindir) mxshadowsrv From 425c9d636fc4d12a463586922cf94ec6ecabb93d Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Tue, 18 May 2021 07:37:06 +0200 Subject: [PATCH 17/20] get_shadow_line: Make function static --- get_shadow_line.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/get_shadow_line.c b/get_shadow_line.c index e3f4f73..56d2428 100644 --- a/get_shadow_line.c +++ b/get_shadow_line.c @@ -82,7 +82,7 @@ static int connect_with_timeout(int sockfd, struct sockaddr *addr, socklen_t add #define BUFLEN_SPWD (1024) -int get_shadow_line(char *user, char **line) { +static int get_shadow_line(char *user, char **line) { struct sockaddr_in sockaddr; bzero(&sockaddr, sizeof(sockaddr)); From 4e9c974b7ca8bf6c06bf284b87ba6d5ce345b4b8 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Tue, 18 May 2021 09:36:19 +0200 Subject: [PATCH 18/20] get_shadow_line: Move const string into function scope --- get_shadow_line.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/get_shadow_line.c b/get_shadow_line.c index 56d2428..18b8f82 100644 --- a/get_shadow_line.c +++ b/get_shadow_line.c @@ -11,9 +11,8 @@ #define TIMEOUT 5000 -static char conf_filename[] = "/etc/mxshadow.conf"; - static int read_config(struct sockaddr_in *addr) { + static const char conf_filename[] = "/etc/mxshadow.conf"; FILE *f _cleanup_(free_file) = NULL; char *line _cleanup_(free_string) = NULL; size_t n = 0; From b0bb8640bcef1b4a0bf615e2ba291b27ec278a7c Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Tue, 18 May 2021 09:36:58 +0200 Subject: [PATCH 19/20] get_shadow_line: Fix whitespace --- get_shadow_line.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/get_shadow_line.c b/get_shadow_line.c index 18b8f82..8b80495 100644 --- a/get_shadow_line.c +++ b/get_shadow_line.c @@ -85,7 +85,7 @@ static int get_shadow_line(char *user, char **line) { struct sockaddr_in sockaddr; bzero(&sockaddr, sizeof(sockaddr)); - sockaddr.sin_family =AF_INET; + sockaddr.sin_family = AF_INET; int status = read_config(&sockaddr); if (status == -1) return -1; From 664fa17d898eb3fdb60d5f839484dde581267958 Mon Sep 17 00:00:00 2001 From: Donald Buczek Date: Tue, 18 May 2021 10:53:34 +0200 Subject: [PATCH 20/20] test-wtf.sh: Remove Remove obsolete test script. --- test-wtf.sh | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100755 test-wtf.sh diff --git a/test-wtf.sh b/test-wtf.sh deleted file mode 100755 index 8f82b99..0000000 --- a/test-wtf.sh +++ /dev/null @@ -1,10 +0,0 @@ -#! /bin/bash - -cp mxshadow.key.pem /package/nis/etc/mxshadow.key.pem -cp mxshadow.cert.pem /package/nis/etc/mxshadow.cert.pem - -chmod 660 /package/nis/etc/mxshadow.key.pem -chmod 664 /package/nis/etc/mxshadow.cert.pem - -./mxshadowsrv --key /package/nis/etc/mxshadow.key.pem --cert /package/nis/etc/mxshadow.cert.pem \ - /package/nis/var/shadow