diff options
author | djm@openbsd.org <djm@openbsd.org> | 2023-07-19 14:02:27 +0000 |
---|---|---|
committer | Damien Miller <djm@mindrot.org> | 2023-07-20 00:21:31 +1000 |
commit | 29ef8a04866ca14688d5b7fed7b8b9deab851f77 (patch) | |
tree | bd26f6642dc682b6b0ca12e077cf3c75dc69d178 | |
parent | 1f2731f5d7a8f8a8385c6031667ed29072c0d92a (diff) |
upstream: Ensure FIDO/PKCS11 libraries contain expected symbols
This checks via nlist(3) that candidate provider libraries contain one
of the symbols that we will require prior to dlopen(), which can cause
a number of side effects, including execution of constructors.
Feedback deraadt; ok markus
OpenBSD-Commit-ID: 1508a5fbd74e329e69a55b56c453c292029aefbe
-rw-r--r-- | misc.c | 78 | ||||
-rw-r--r-- | misc.h | 3 | ||||
-rw-r--r-- | ssh-pkcs11.c | 6 | ||||
-rw-r--r-- | ssh-sk.c | 8 |
4 files changed, 89 insertions, 6 deletions
@@ -1,4 +1,4 @@ -/* $OpenBSD: misc.c,v 1.183 2023/07/14 07:44:21 dtucker Exp $ */ +/* $OpenBSD: misc.c,v 1.184 2023/07/19 14:02:27 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * Copyright (c) 2005-2020 Damien Miller. All rights reserved. @@ -22,6 +22,7 @@ #include <sys/types.h> #include <sys/ioctl.h> +#include <sys/mman.h> #include <sys/socket.h> #include <sys/stat.h> #include <sys/time.h> @@ -35,6 +36,9 @@ #ifdef HAVE_POLL_H #include <poll.h> #endif +#ifdef HAVE_NLIST_H +#include <nlist.h> +#endif #include <signal.h> #include <stdarg.h> #include <stdio.h> @@ -2920,3 +2924,75 @@ ptimeout_isset(struct timespec *pt) { return pt->tv_sec != -1; } + +/* + * Returns zero if the library at 'path' contains symbol 's', nonzero + * otherwise. + */ +int +lib_contains_symbol(const char *path, const char *s) +{ +#ifdef HAVE_NLIST_H + struct nlist nl[2]; + int ret = -1, r; + + memset(nl, 0, sizeof(nl)); + nl[0].n_name = xstrdup(s); + nl[1].n_name = NULL; + if ((r = nlist(path, nl)) == -1) { + error_f("nlist failed for %s", path); + goto out; + } + if (r != 0 || nl[0].n_value == 0 || nl[0].n_type == 0) { + error_f("library %s does not contain symbol %s", path, s); + goto out; + } + /* success */ + ret = 0; + out: + free(nl[0].n_name); + return ret; +#else /* HAVE_NLIST_H */ + int fd, ret = -1; + struct stat st; + void *m = NULL; + size_t sz = 0; + + memset(&st, 0, sizeof(st)); + if ((fd = open(path, O_RDONLY)) < 0) { + error_f("open %s: %s", path, strerror(errno)); + return -1; + } + if (fstat(fd, &st) != 0) { + error_f("fstat %s: %s", path, strerror(errno)); + goto out; + } + if (!S_ISREG(st.st_mode)) { + error_f("%s is not a regular file", path); + goto out; + } + if (st.st_size < 0 || + (size_t)st.st_size < strlen(s) || + st.st_size >= INT_MAX/2) { + error_f("%s bad size %lld", path, (long long)st.st_size); + goto out; + } + sz = (size_t)st.st_size; + if ((m = mmap(NULL, sz, PROT_READ, MAP_PRIVATE, fd, 0)) == MAP_FAILED || + m == NULL) { + error_f("mmap %s: %s", path, strerror(errno)); + goto out; + } + if (memmem(m, sz, s, strlen(s)) == NULL) { + error_f("%s does not contain expected string %s", path, s); + goto out; + } + /* success */ + ret = 0; + out: + if (m != NULL && m != MAP_FAILED) + munmap(m, sz); + close(fd); + return ret; +#endif /* HAVE_NLIST_H */ +} @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.h,v 1.102 2023/03/03 02:37:58 dtucker Exp $ */ +/* $OpenBSD: misc.h,v 1.103 2023/07/19 14:02:27 djm Exp $ */ /* * Author: Tatu Ylonen <ylo@cs.hut.fi> @@ -96,6 +96,7 @@ int parse_absolute_time(const char *, uint64_t *); void format_absolute_time(uint64_t, char *, size_t); int path_absolute(const char *); int stdfd_devnull(int, int, int); +int lib_contains_symbol(const char *, const char *); void sock_set_v6only(int); diff --git a/ssh-pkcs11.c b/ssh-pkcs11.c index 0b51e775..8e2b9cb9 100644 --- a/ssh-pkcs11.c +++ b/ssh-pkcs11.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-pkcs11.c,v 1.57 2023/07/19 13:55:53 djm Exp $ */ +/* $OpenBSD: ssh-pkcs11.c,v 1.58 2023/07/19 14:02:27 djm Exp $ */ /* * Copyright (c) 2010 Markus Friedl. All rights reserved. * Copyright (c) 2014 Pedro Martelletto. All rights reserved. @@ -1532,6 +1532,10 @@ pkcs11_register_provider(char *provider_id, char *pin, debug_f("provider already registered: %s", provider_id); goto fail; } + if (lib_contains_symbol(provider_id, "C_GetFunctionList") != 0) { + error("provider %s is not a PKCS11 library", provider_id); + goto fail; + } /* open shared pkcs11-library */ if ((handle = dlopen(provider_id, RTLD_NOW)) == NULL) { error("dlopen %s failed: %s", provider_id, dlerror()); @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk.c,v 1.39 2022/07/20 03:29:14 djm Exp $ */ +/* $OpenBSD: ssh-sk.c,v 1.40 2023/07/19 14:02:27 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -133,10 +133,12 @@ sshsk_open(const char *path) goto fail; #endif } - if ((ret->dlhandle = dlopen(path, RTLD_NOW)) == NULL) { - error("Provider \"%s\" dlopen failed: %s", path, dlerror()); + if (lib_contains_symbol(path, "sk_api_version") != 0) { + error("provider %s is not an OpenSSH FIDO library", path); goto fail; } + if ((ret->dlhandle = dlopen(path, RTLD_NOW)) == NULL) + fatal("Provider \"%s\" dlopen failed: %s", path, dlerror()); if ((ret->sk_api_version = dlsym(ret->dlhandle, "sk_api_version")) == NULL) { error("Provider \"%s\" dlsym(sk_api_version) failed: %s", |