Skip to content

Commit

Permalink
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
  • Loading branch information
djmdjm committed Jul 19, 2023
1 parent 7bc29a9 commit f8f5a6b
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 6 deletions.
31 changes: 30 additions & 1 deletion usr.bin/ssh/misc.c
@@ -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 Down Expand Up @@ -40,6 +40,7 @@
#include <pwd.h>
#include <libgen.h>
#include <limits.h>
#include <nlist.h>
#include <poll.h>
#include <signal.h>
#include <stdarg.h>
Expand Down Expand Up @@ -2814,3 +2815,31 @@ 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)
{
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;
}
3 changes: 2 additions & 1 deletion usr.bin/ssh/misc.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>
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 *);

struct passwd *pwcopy(struct passwd *);
const char *ssh_gai_strerror(int);
Expand Down
6 changes: 5 additions & 1 deletion usr.bin/ssh/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.
Expand Down Expand Up @@ -1507,6 +1507,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
9 changes: 6 additions & 3 deletions usr.bin/ssh/ssh-sk.c
@@ -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 @@ -113,15 +113,18 @@ sshsk_open(const char *path)
ret->sk_load_resident_keys = ssh_sk_load_resident_keys;
return ret;
}
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) {
error("Provider \"%s\" dlopen failed: %s", path, dlerror());
goto fail;
}
if ((ret->sk_api_version = dlsym(ret->dlhandle,
"sk_api_version")) == NULL) {
error("Provider \"%s\" dlsym(sk_api_version) failed: %s",
fatal("Provider \"%s\" dlsym(sk_api_version) failed: %s",
path, dlerror());
goto fail;
}
version = ret->sk_api_version();
debug_f("provider %s implements version 0x%08lx", ret->path,
Expand Down

0 comments on commit f8f5a6b

Please sign in to comment.