For purposes of ssh, is it ever right to look into the non-dynamic symbols? And if not, should it use dlsym() instead?

In D57034#1306950, @kib wrote:

For purposes of ssh, is it ever right to look into the non-dynamic symbols? And if not, should it use dlsym() instead?

OpenSSH uses nlist(3) to check if a shared object provides a certain symbol before loading it. It could use dlsym(), but that requires loading the object first, which will execute constructors even if the shared object turns out not to be a functioning PKCS#11 provider. I have no idea how this works upstream since OpenBSD's nlist(3) is the same as ours and doesn't work on shared objects.

I just noticed another problem with the original code: if there is no SHT_SYMTAB section at all, it still tries to mmap() offset 0 of length 0, which is nbd on 14 and prior as it just fails with errno == EINVAL, but on 15 and newer it produces an extended error which specifically references mmap(2). This problem does not exist in my version as it only ever tries to mmap(2) an actual section.

For purposes of ssh, is it ever right to look into the non-dynamic symbols? And if not, should it use dlsym() instead?

As DES mentions, after this nlist() call OpenSSH just calls dlsym (lib_contains_symbol checks nlist's return):

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());
        goto fail;
}
if ((getfunctionlist = dlsym(handle, "C_GetFunctionList")) == NULL)
        fatal("dlsym(C_GetFunctionList) failed: %s", dlerror());

openssh-portable has a lib_contains_symbol implementation for systems without nlist() that just mmaps the file and searches for the string with memmem. But in any case our nlist implementation and OpenSSH's use of nlist both seem a little janky.

Maybe we could have a dlopen(3) flag that avoids executing constructors and any other processing beyond parsing the library, and another interface to do so? So we'd have something like:

handle = dlopen(provider_id, RTLD_NOW | RTLD_DEFER_INIT /* or whatever */);
getfunctionlist = dlsym(handle, "C_GetFunctionList"));
if (!getgetfunctionlist)
        error();
rtld_deferred_init(handle)

But after the dlopen(RTLD_DEFER_INIT), would the object participate in the symbol resolution? If yes, it is unsafe. If no, it is probably too hard to implement for the effect.

BTW, this nlist() check would fail for a properly stripped objects as well. Sections can be legitimately absent from the objects. Why doing it at all? What do they try to prevent but that check?

We'll want to update nlist.3 to describe this; I posted D57065 to document the current behaviour.

Why doing it at all? What do they try to prevent but that check?

It came from:

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

Now presumably if you can cause the application to load a malicious .so you can also just include the symbol it's looking for. But, there could be some existing .so with a buggy constructor that an exploit would make use of.

Probably also Fixes: 77909f597881 ("Initial elf nlist support, ...")

They clearly must be zero-terminated

Yes they're null terminated. The common optimization / special case with ELF string tables is that strings with a common suffix may be combined.

In D57034#1307991, @emaste wrote:

Yes they're null terminated. The common optimization / special case with ELF string tables is that strings with a common suffix may be combined.

Right, that's a common optimization for string tables in general (see for instance JVM string pools), it's all the same to us as long as they're terminated.