Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow SSL when used under Server::Starter #147

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kgoess
Copy link

@kgoess kgoess commented Feb 6, 2023

This commit goes along with a PR I'm submitting to
p5-Net-Server-SS-PreFork.

It allows you to run starman under start_server like this:

start_server \
    --port 1.1.1.1:111 \
    --port 2.2.2.2:222 \
    -- \
    starman \
    --enable-ssl \
    --ssl_cert ./etc/ccard/kgoess7-sandbox-ccweb.crt \
    --ssl_key ./etc/ccard/kgoess7-sandbox-ccweb.key \
    --net_server_SSL_cipher_list whatever

Or instead of enabling SSL on all the ports, it can be done selectively
by adding the ":ssl" suffix to the individual "--port" arguments.

The maybe_upgrade_to_ssl function allows Net::Server::SS::PreFork's
pre_bind method to look at $self->{options}, which is where Starman::Server has
stashed the SSL arguments.

See also a PR I'm submitting to p5-Server-Starter that allows
start_server's --port argument to take an ":ssl" suffix, like "--port
IP:PORT:ssl".

Kevin M. Goess and others added 4 commits August 23, 2022 15:19
This commit goes along with a PR I'm submitting to
p5-Net-Server-SS-PreFork.

It allows you to run starman under start_server like this:

    start_server \
        --port 1.1.1.1:111 \
        --port 2.2.2.2:222 \
        -- \
        starman \
        --enable-ssl \
        --ssl_cert ./etc/ccard/kgoess7-sandbox-ccweb.crt \
        --ssl_key ./etc/ccard/kgoess7-sandbox-ccweb.key \
        --net_server_SSL_cipher_list whatever

Or instead of enabling SSL on all the ports, it can be done selectively
by adding the ":ssl" suffix to the individual "--port" arguments.

The maybe_upgrade_to_ssl function allows Net::Server::SS::PreFork's
pre_bind method to look at $self->{options}, which is where Starman::Server has
stashed the SSL arguments.

See also a PR I'm submitting to p5-Server-Starter that allows
start_server's --port argument to take an ":ssl" suffix, like "--port
IP:PORT:ssl".
Because that's what IO::Socket::SSL is expecting. Lacking that this
server code will treat it as a client cert and ask the other side to
present its cert for verification, which isn't what you want.

For instance, apache's proxy would warn:

    downstream server wanted client certificate but none are configured
Before 2.011 Net::Server had a hack to manually call the underlying
socket's ->accept (and later manually call SSL_accept). They've undone
this by passing in 'SSL_startHandshake => 0' which then flips their code
to use IO::Socket::SSL's ->accept.

From Net::Server's Changes:

    2.011  Dec 01 2022
        - Change SSL to use IO::Socket::SSL SSL_startHandshake

Since we're configuring SSL on the socket ourselves, we need to include
that argument as well to work on newer versions of Net::Server

The option also works on older Net::Server because of their hack in
that version which bypasses the logic that makes use of that option, and
works on older IO::Socket::SSL because it just ignores the argument.
@kgoess kgoess marked this pull request as ready for review February 6, 2023 21:43
@miyagawa
Copy link
Owner

miyagawa commented Feb 6, 2023

(Hi!)

The patch looks reasonable to me. I'll wait and see if this is accepted in Server-Starter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants