Skip to content

Commit

Permalink
upstream: Ensure FIDO/PKCS11 libraries contain expected symbols
Browse files Browse the repository at this point in the history
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
  • Loading branch information
djmdjm committed Jul 19, 2023
1 parent 1f2731f commit 29ef8a0
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 6 deletions.
78 changes: 77 additions & 1 deletion misc.c
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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>
Expand All @@ -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>
Expand Down Expand Up @@ -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 */
}
3 changes: 2 additions & 1 deletion misc.h
Original file line number Diff line number Diff line change
@@ -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>
Expand Down Expand Up @@ -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);

Expand Down
6 changes: 5 additions & 1 deletion ssh-pkcs11.c
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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());
Expand Down
8 changes: 5 additions & 3 deletions ssh-sk.c
Original file line number Diff line number Diff line change
@@ -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
*
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 29ef8a0

Please sign in to comment.