unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / mirror / code / Atom feed
From: Jeremy Evans <code@jeremyevans.net>
To: Eric Wong <e@80x24.org>
Cc: unicorn-public@bogomips.org
Subject: Re: Patch: Add after_worker_ready configuration option
Date: Thu, 23 Feb 2017 12:00:29 -0800	[thread overview]
Message-ID: <20170223200029.GD67612@jeremyevans.local> (raw)
In-Reply-To: <20170223092309.GA28620@whir>

On 02/23 09:23, Eric Wong wrote:
> Jeremy Evans <code@jeremyevans.net> wrote:
> > On 02/23 02:32, Eric Wong wrote:
> > > Jeremy Evans <code@jeremyevans.net> wrote:
> > > Anyways, I'm somewhat inclined to accept this as well, but will
> > > think about it a bit, too.
> > > 
> > > One problem I have with this change is that it requires users
> > > read more documentation and know more caveats to use.
> >  
> > It's unfortunate, but I think it's necessary in this case. Unicorn's
> > default behavior works for most users, but this makes it so you
> > don't have to override HttpServer#init_worker_process to get similar
> > functionality.
> Yeah.  Unfortunately, with every option comes more
> support/maintenance overhead.  So I'm trying to make sure we've
> explored every possible avenue before adding this new option.

I agree.  If I knew of a way to get this functionality without
overriding a private method, I would use it.  I'm not sure there is a
way currently, which I why I worked on the patch.

> > >     So I'm wondering if there can be some of that which could be
> > >     trimmed out someday or made optional, somehow.
> > 
> > I'm not sure how open you are to a plugin-like system for Unicorn, where
> > you can have separate files for separate features, and users can choose
> > which features the want to use.  This limits memory usage so that you
> > don't pay a memory cost for features you don't use.  This is the
> > approach used by Sequel, Roda, and Rodauth (other libraries I maintain),
> > and if you are interested in a similar system for Unicorn, I would be
> > happy to work on one.
> Not really.  As I've stated many times in the past, unicorn is
> just one of many servers available for Rack users; so I've been
> against people building too many dependencies on unicorn, that
> comes at the expense of other servers.
True, but there are features that only unicorn provides.  If you want
those features, you have to use unicorn anyway. 

> What I had in mind was having a shim process which does socket
> setup, user switching, etc. before exec-ing unicorn in cases
> where a systemd-like spawner is not used.  But it's probably too
> small to make an impact on real apps anyways, since Ruby still
> has tons of methods which never get called, either.

I could see that decreasing memory usage slightly, but I agree it's
unlikely to make a significant impact.

> > I do have a couple of additional patches I plan to send, but I think it
> > may be better to ask here first.
> > 
> > One is that SIGUSR1 handling doesn't work well with chroot if the log
> > file is outside of the chroot, since the worker process may not have
> > access to reopen the file.  One simple fix is to reopen the logs in
> > the master process, then send SIGQUIT instead of SIGUSR1 to the child
> > processes.  Are you open to a patch that does that?
> Right now, the worker exits with some log spew if reopening
> logs fails, and the worker respawns anyways, correct?
> Perhaps that mapping can be made automatic if chroot is
> enabled.  I wonder if that's too much magic...
> Or the log spew could be suppressed when the error
> is ENOENT and chroot is enabled.
> The sysadmin could also send SIGHUP to reload everything
> and respawn all children.
> So, I'm wondering if using SIGHUP is too much of a burden, or if
> automatic USR1 => QUIT mapping for chroot users is too magical.

SIGHUP is actually fine.  I didn't realize it reopened log files
in the master process, but apparently it does, so I can just use
SIGHUP instead of SIGUSR1.

> > The second feature is more exotic security feature called fork+exec.
> > The current issue with unicorn's direct forking of children is that
> > forked children share the memory layout of the server.  This is good
> > from a memory usage standpoint due to CoW memory.  However, if there
> > is a memory vulnerability in the unicorn application, this makes it
> > easier to exploit, since if the a unicorn worker process crashes, the
> > master process will fork a child with pretty much the same memory
> > layout.  This allows address space discovery attacks.  Using fork+exec,
> > each child process would have a unique address space for ASLR, on
> > most Unix-like systems (Linux, MacOS X, NetBSD, OpenBSD, Solaris).
> > There are additional security advantages from fork+exec on OpenBSD, but
> > all Unix-like systems that use ASLR can benefit from the unique
> > memory layout per process.
> It depends how intrusive the implementation is...

That I don't know yet.  I haven't implemented fork+exec before, so I'm
not sure how many changes are needed. I probably won't be able to work
on this right away.

> Is this merely calling exec in the worker?
> I'm not sure if your use of "fork+exec" is merely the two
> well-known syscalls combined, or if there's something else;
> I'll assume the former, since that's enough to have a
> unique address space for Ruby bytecode.
> Can this be done in an after_fork/after_worker_ready hook?
> Socket inheritance can be done the same way it handles USR2
> upgrades and systemd socket inheritance.

I expect in addition to socket inheritance, we will need to handle pipe
inheritance for the @to_io and @master pipes in the worker.


      reply	other threads:[~2017-02-23 20:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-23  0:49 Patch: Add after_worker_ready configuration option Jeremy Evans
2017-02-23  2:32 ` Eric Wong
2017-02-23  2:34   ` Eric Wong
2017-02-23  3:43   ` Jeremy Evans
2017-02-23  9:23     ` Eric Wong
2017-02-23 20:00       ` Jeremy Evans [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information: https://yhbt.net/unicorn/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170223200029.GD67612@jeremyevans.local \
    --to=code@jeremyevans.net \
    --cc=e@80x24.org \
    --cc=unicorn-public@bogomips.org \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).