Hapi: Create connection via file descriptor instead of port

[sholladay]

Node's Server#listen() method accepts a fd option and I am wondering if hapi could support something like that.

The reason this would be useful to me is that I want to use substack's tcp-bind and sindresorhus' root-check to drop privileges as soon as possible.

As a simplified example, I wish something like this worked when run with sudo:

const fd = require('tcp-bind')(80);
require('root-check')();

const { Server } = require('hapi'),
const server = new Server();

server.connection({ fd });
server.start((err) => {
    if (err) {
        throw err;
    }

    console.log('Server is ready.');
});

The benefit here is that I can drop privileges as soon as possible, without waiting for the start callback, etc.

[hueniverse]

I'll take a PR where port is an object with { fd }

[thebergamo]

Just commenting this issue to keep informed about that. If in this half time no ones send the PR I will try to do this request.

[hueniverse]

@thebergamo next time just subscribe to it. No need to waste everyone's time.

[thebergamo]

sorry!

[kanongil]

Note that you might be able to do this already using { port:/dev/fd/${fd}}

[btmorex]

Not that I think this would be a bad change, but you can already do this:

server.connection({autoListen: false});
server.listener.listen(...);
server.start(...);

[sholladay]

I appreciate the workarounds, everyone.

I have posted a $40 bounty as I don't currently have time to work on it. But this will be very useful, particularly for those who care about security.

[mtharrison]

@sholladay is there a way to get an fd which is bound to a port via the public API? The tcp-bind module seems to dip into process.binding() which is private API.

This addition itself is simple to implement but having to use non-public API for tests might be a deal-breaker.

[sholladay]

That does suck. I'm not aware of any other way.

On the other hand, there is a very minimal amount of code that would be necessary in a test. Also, process.binding() has pretty widespread use in the ecosystem and it is unlikely they will break this anytime soon. They specifically discussed it and decided against it, mainly because of ecosystem breakage.

There was even some discussion of exposing this particular use of process.binding(). It seems like they acknowledge that they need to deal with it responsibly.

[mtharrison]

That discussion stills seems very much open to a change in position. Personally, I think until there's a properly documented or accepted way to use it, the workarounds given above are your best option.

Unless @hueniverse disagrees and is happy to use process.binding() in tests?

[sholladay]

Hmm well fd is a simple number. And Node is at least as responsible for testing that its interface works appropriately with those numbers. I feel like a clever test writer could write a somewhat meaningful test without process.binding().

We can at least assert that fd must be a number. And by defining it as a getter property, we can also assert that it is read by the framework. Probably we could also figure out a way to assert that Node received it correctly (e.g. by listening for error 32 ENOTSOCK).

I'm not saying this is perfect. But it seems like with a bit of work one ought to be able to meet the threshold to get a PR accepted. I mean the important part here IMO is making sure that none of Hapi's internals freak out, not that Node follows its own documentation.

Just trying to be pragmatic here, since I believe that accepting a fd like this will meaningfully move the ball forward in terms of security.

[mtharrison]

It's not really about making sure Node follows its own documentation, it's about how to test that the feature works with hapi internals.

For example, you'd ideally want to check that hapi is setting the right address and info values here which are only populated once the listening event is fired. So you'd want your test to actually start the server using the fd value you specify and make sure it propagates through properly.

You would also want to test making a request to that port and making sure it reaches your handler logic.

So we could easily _allow_ specifying an fd and update the validation accordingly but it wouldn't be a thorough test IMO.

[sholladay]

you'd ideally want to check that hapi is setting the right address

Yeah, that's exactly what I was thinking. I can't think of a way to test that without process.binding(), but maybe a test that doesn't do that could still have some value.

Assuming that is off the table, I guess the question is what would the tests have to do for such a PR to get accepted?

[hueniverse]

If this becomes messy, I am going to reject it. There is a workaround and no one else has asked for this. Whatever anyone proposes has to be so trivial, I don't have to think about it.

[mtharrison]

The feature can be trivially added, it just can't be trivially tested, as far as I can tell.

[hueniverse]

Closing for now. If a PR shows up we can reconsider.

[lock[bot]]

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.