OpenSSL PR - notes

Here's a first version of my PR for issue #21770.

Hopefully the overall approach makes sense. Very much welcome feedback!

User Scenario

A user executing an application on linux that utilizes open ssl libraries cannot make use of quantum-safe encryption algorithms, using OPENSSL_MODULES and OPENSSL_CONF, if that application needs the ability to bind to a port below 1024 by enabling CAP_NET_BIND_SERVICE.

Setting this privilege is common in server environments to run on well known ports.

Technical Problem Summary

When using GLIBC > 2.17 (linux?), OpenSSL uses the secure_getenv call in glibc to retrieve all environment variables. If the process is setuid/setgid, OR has additional capabilities set, this call will return NULL.

This is intentional, since we do not want a non-priviliged user to inject malicious code, or otherwise compromise the configuration of the application.

Proposed change of behavior

The exact test as to whether to return NULL is slightly modified, in that it is only applied IF, and ONLY IF, the ONLY additional capability granted is CAP_NET_BIND_SERVICE.

Risks

The risks to evaluate for this solution include:

An error in the design such that the intended test allows for malicious configuration.
An error in the implementation such that the code does not implement the intended design.
Poor documentation, such that the feature is unknown, or unclear to use
Library/kernel changes affecting the implementation behaviour in future

Proposed Implementation in this PR

The intent is to keep checks as safe as possible, and make minimal changes to existing code.

The changes are protected by compile time flags, and the restriction is only lifted for SELECT environment variables, with ONLY the specified capability.

Add new compile-time define OPENSSL_NETCAP_ALLOW_ENV, so that the modified code is only compiled if specified at compile time ie using CFLAGS=-DOPENSSL_NETCAP_ALLOW_ENV (all)
If set, and we are on linux (linux defined) uses new function restricted_getenv() to retrieve specified environment variable. Question - is the preprocessor check here appropriate? (getenv.c)
restricted_getenv`` uses a modified **OpenSSL_issetugid** (below) to check we aren't in a secure mode. This is a change in behaviour from previously, where this function was only used if glibc was too old to support secure_getenv` (getenv.c)
OpenSSL_issetugid is modified slightly so that it queries the processes's capabilities if we are in a secure mode. If the only capability set is for bind, we now return false (0) . This follows the intent of the function to indicate we need to act very securely in general ie for setuid etc., but allows for this proposed loosening of the check. (uid.c)
restricted_getenv then checks if the requested environment variable is a member of a configured (hardcoded) list (getenv.c)
If so, it is returned directly. If not it reverts to using secure_getenv.

Build

For a 'local' development build to support testing, making use of a pause check at startup (see below).

Install into a local directory (not overwriting system version)

./Configure --prefix=/home/jonesn/opt/openssl --openssldir=/home/jonesn/usr/local/ssl

Build with appropriate flags:

make CFLAGS=-DOPENSSL_NETCAP_ALLOW_ENV -g -Wl,-rpath,/home/jonesn/src/openssl

Alternative implementations considered

There are many ways of setting up the preprocessor checks, and exact code sequence -- and the flow could likely be made simpler, but it does result in more changes, so more risk.

There are also some utility functions in libcap. However it is hard to check for only a select, permitted capability with the limited API. Additionally it would introduce an additional development and runtime dependency, albeit a fairly standard one.

Future enhancements

Making this feature available by default

OpenSSL libraries are used by many applications. We need to make it easier for users of these applications to enable quantum-safe cryptography schemes by deployment/app configuration, and not requiring a rebuild of openssl libraries.

To do this, I would like to suggest that if this PR is acceptable to the reviewers, that either within this PR, or in a later PR we can either

remove the build flag entirely
default the value to being enabled

Runtime configuration of this feature

Do we need to allow this feature to be disabled at runtime, in the configuration
Can we make the list of environment variables permitted configurable?

Both have one notable issue - where to store the config, especially if we already allow OPENSSL_CONF to be passed

Is it safe to permit all?

Testing

Recommendations on the best way to test welcomed.

So far testing has been done by using the 'openssl' command, and validating if environment variables are passed by inspection of command output, or by using a debugger (remotely attached so as not to affect permissions).

For example

setcap cap_net_bind_service=+ep  apps/openssl
export OPENSSL_CONF=/home/jonesn/etc/openssl.cnf 
./apps/openssl engine -c qatengine

Capability was added to the apps/openssl executable where needed for the test using setcap cap_net_bind_service=+ep apps/openssl. Other capabilities can also be set to verify these environment variables are no longer allowed to pass.

It was not possible to test (in Visual Studio Code) using gdb directly, as if run under debug no extra capabilities set set. Neither can the process be run as root, since then it has too many permissions. Instead the apps/openssl app was run, and then attached to remotely, using gdb running as root.

To do this the openssl command was paused at startup using this at the top of apps/openssl.c:

#ifdef OPENSSL_TEST_PAUSE_FOR_ATTACH
#include <signal.h>
#endif

then later, at the beginning of main:

#if defined(OPENSSL_TEST_PAUSE_FOR_ATTACH)
    if(getenv("OPENSSL_TEST_PAUSE_FOR_ATTACH"))
    {
        raise(SIGSTOP);
    }
#endif

This can be used by defining appropriate values at build/runtime.

If this is useful I am happy to submit further changes, but it was very much just a mechanism to support my testing given the complexity of capabilities. It does not form part of the current PR. Similarly I can add more docs on visual studio code build/test if it helps.

I'll continue testing, to verify that the user scenario works, i.e. using the liboqs provider & libraries:

Documentation

There should be some end-user documentation of this feature. NOTES-UNIX.md may be one location? Should information be added elsewhere?

planetf1/pr-notes.md