openssl / openssl/openssl

It's very complicated to set CLOEXEC on sockets created by this API

Open
#2,162 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

triaged: feature
Dominant language
C
Stars
30.8k
Forks
11.5k
Avg merge
10m
Merged PRs (30d)
1

Description

Hello.
In UNIX a process created by exec*() inherits all open file descriptors from the previous process if they aren't explicitly marked with the CLOEXEC flag. In situations where this is not the intention of the program, it creates security holes and floods the system with fds that'll be never closed. Most applications never use this behavior (besides for stdin, out, err), so they have to carefully set this flag on every fd they create. I personally think they should change the standards and at least create a global flag, that affects the whole application, but this is not the case so we have to live with that.

So as I started using this library (which is well done by the way 👍) in my project, I read through it's source code to find ways to do this on connection BIOs and imagined two ways:

  1. By setting an info callback on the connection BIO that sets the flag with fcntl() every time after the socket is created. But because every BIO on a stack implements the callback_ctrl, this looks awful:
    BIO *bio = BIO_new_buffer_ssl_connect(ctx);
    BIO_set_info_callback(BIO_find_type(bio, BIO_TYPE_CONNECT), &cloexec_cb);
    For me, this whole callback interface looks more as for debugging purposes, as their are some adventurous casts performed with the callback.
    Also this solution could lead to race conditions in MT-applications, when one thread forks+execs while another thread is between the socket call and the fcntl call.
  2. By not using the connection BIOs and instead doing everything on my own and then using a socket BIO. But with this solution I can't benefit from this great API.

And as this is a quite common need (every application that execs and doesn't want to spread their fds all over the system should use it) I think there should be a easier way like a flag in the connect BIO.

EDIT: As I read through the source code I found out that BIO_find_type(bio, BIO_TYPO_CONNECT) would be better than bio->prev_bio->prev_bio in my first solution.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the connection BIO API around BIO_new_buffer_ssl_connect and BIO_TYPE_CONNECT, then trace where its sockets are created. Compare the proposed flag-based approach with the existing fcntl callback workaround, paying attention to fork/exec races. Done means providing a documented way to create these sockets with CLOEXEC without requiring a separate socket setup.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
api, networking, security
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.