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 multiple values for HTTPS_PORT #3622

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions docs/administering/config-file.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,14 @@ typically a static publishing website, then serve a normal response.

### HTTPS_PORT

A port number for Sandstorm to bind on and listen for HTTPS. On a default install, if port 443
was available and the user chose to use Sandcats, this is 443. However, this may be set for any
Sandstorm-managed TLS configuration, including automated renewals of certificates with Let's Encrypt
with a supported DNS provider or a manually-uploaded certificate. If this config option is missing,
Sandstorm's built-in HTTPS server is disabled.
Like `PORT`, but these ports are served using HTTPS. If both `PORT` and `HTTPS_PORT` are specified,
the first `HTTPS_PORT` "First port" as described above, and all values in `PORT` are "alternate
ports."

On a default install, if port 443 was available and the user chose to use Sandcats, this is 443.
However, this may be set for any Sandstorm-managed TLS configuration, including automated renewals
of certificates with Let's Encrypt with a supported DNS provider or a manually-uploaded certificate.
If this config option is missing, Sandstorm's built-in HTTPS server is disabled.

If Sandstorm is started as root, Sandstorm binds to this port as root, allowing it to use
low-numbered ports. The socket is passed-through to code that does not run as root.
Expand All @@ -66,9 +69,6 @@ Example:
HTTPS_PORT=443
```

A HTTPS_PORT is automatically treated as the first port, in the context of "first port" vs.
"alternate ports."

### SMTP_LISTEN_PORT

A port number on which Sandstorm will bind, listening for inbound email. By default, 30025; if
Expand Down
56 changes: 17 additions & 39 deletions src/sandstorm/config.c++
Original file line number Diff line number Diff line change
Expand Up @@ -70,31 +70,17 @@ auto parser = p::sequence(delimited<' '>(assignment), p::discardWhitespace, p::e

// =======================================================================================

kj::Array<uint> parsePorts(kj::Maybe<uint> httpsPort, kj::StringPtr portList) {
auto portsSplitOnComma = split(portList, ',');
size_t numHttpPorts = portsSplitOnComma.size();
size_t numHttpsPorts;
kj::Array<uint> result;

// If the configuration has a https port, then add it first.
KJ_IF_MAYBE(portNumber, httpsPort) {
numHttpsPorts = 1;
result = kj::heapArray<uint>(numHttpsPorts + numHttpPorts);
result[0] = *portNumber;
} else {
numHttpsPorts = 0;
result = kj::heapArray<uint>(numHttpsPorts + numHttpPorts);
}

kj::Array<uint> parsePortList(kj::StringPtr key, kj::StringPtr portListStr) {
auto portsSplitOnComma = split(portListStr, ',');
auto result = kj::heapArrayBuilder<uint>(portsSplitOnComma.size());
for (size_t i = 0; i < portsSplitOnComma.size(); i++) {
KJ_IF_MAYBE(portNumber, parseUInt(trim(portsSplitOnComma[i]), 10)) {
result[i + numHttpsPorts] = *portNumber;
result.add(*portNumber);
} else {
KJ_FAIL_REQUIRE("invalid config value PORT", portList);
KJ_FAIL_REQUIRE("invalid config value ", key, portListStr);
}
}

return kj::mv(result);
return result.finish();
}

kj::Maybe<UserIds> getUserIds(kj::StringPtr name) {
Expand Down Expand Up @@ -170,10 +156,6 @@ Config readConfig(const char *path, bool parseUids) {
config.uids.uid = getuid();
config.uids.gid = getgid();

// Store the PORT and HTTPS_PORT values in variables here so we can
// process them at the end.
kj::Maybe<kj::String> maybePortValue = nullptr;

auto lines = splitLines(readAll(path));
for (auto& line: lines) {
auto equalsPos = KJ_ASSERT_NONNULL(line.findFirst('='), "Invalid config line", line);
Expand All @@ -190,13 +172,9 @@ Config readConfig(const char *path, bool parseUids) {
}
}
} else if (key == "HTTPS_PORT") {
KJ_IF_MAYBE(p, parseUInt(value, 10)) {
config.httpsPort = *p;
} else {
KJ_FAIL_REQUIRE("invalid config value HTTPS_PORT", value);
}
config.httpsPorts = parsePortList(key, value);
} else if (key == "PORT") {
maybePortValue = kj::mv(value);
config.ports = parsePortList(key, value);
} else if (key == "MONGO_PORT") {
KJ_IF_MAYBE(p, parseUInt(value, 10)) {
config.mongoPort = *p;
Expand Down Expand Up @@ -280,16 +258,16 @@ Config readConfig(const char *path, bool parseUids) {
}
}

// Now process the PORT setting, since the actual value in config.ports
// depends on if HTTPS_PORT was provided at any point in reading the
// config file.
//
// Outer KJ_IF_MAYBE so we only run this code if the config file contained
// a PORT= declaration.
KJ_IF_MAYBE(portValue, maybePortValue) {
auto ports = parsePorts(config.httpsPort, *portValue);
config.ports = kj::mv(ports);
// config.ports should actually include the HTTPS_PORTs as well
// (and they should come first):
auto allPorts = kj::heapArrayBuilder<uint>(config.ports.size() + config.httpsPorts.size());
for(uint port : config.httpsPorts) {
allPorts.add(port);
}
for(uint port : config.ports) {
allPorts.add(port);
}
config.ports = allPorts.finish();

return config;
}
Expand Down
2 changes: 1 addition & 1 deletion src/sandstorm/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ struct UserIds {
};

struct Config {
kj::Maybe<uint> httpsPort;
kj::Array<uint> httpsPorts;
kj::Array<uint> ports;
uint mongoPort = 3001;
UserIds uids;
Expand Down
27 changes: 16 additions & 11 deletions src/sandstorm/run-bundle.c++
Original file line number Diff line number Diff line change
Expand Up @@ -2509,15 +2509,22 @@ private:
KJ_FAIL_REQUIRE("backend died; gateway aborting too");
}));

auto isHttpsPort = [&](uint port) -> bool {
for(uint httpsPort : config.httpsPorts) {
if (httpsPort == port) {
return true;
}
}
return false;
};

// Listen on main port.
if (config.ports.size() > 0) {
auto port = config.ports[0];
auto listener = fdBundle.consume(port, *io.lowLevelProvider);
bool isHttps = false;
KJ_IF_MAYBE(p, config.httpsPort) {
isHttps = port == *p;
}
auto promise = isHttps ? tlsManager.listenHttps(*listener) : server.listenHttp(*listener);
auto promise = isHttpsPort(port)
? tlsManager.listenHttps(*listener)
: server.listenHttp(*listener);
promises = promises.exclusiveJoin(promise.attach(kj::mv(listener)));
}

Expand All @@ -2530,7 +2537,9 @@ private:
altPortServer = altPortServer.attach(kj::mv(altPortService));
for (auto port: config.ports.slice(1, config.ports.size())) {
auto listener = fdBundle.consume(port, *io.lowLevelProvider);
auto promise = altPortServer->listenHttp(*listener);
auto promise = isHttpsPort(port)
? tlsManager.listenHttps(*listener)
: altPortServer->listenHttp(*listener);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the issue be here? If I can parse this, it means the altPortServer is only being reached if it's an HTTP port?

promises = promises.exclusiveJoin(promise.attach(kj::mv(listener)));
}
promises = promises.attach(kj::mv(altPortServer));
Expand Down Expand Up @@ -2611,10 +2620,6 @@ private:
KJ_SYSCALL(setenv("HTTP_GATEWAY", "local", true));

KJ_SYSCALL(setenv("PORT", kj::str(config.ports[0]).cStr(), true));
KJ_IF_MAYBE(httpsPort, config.httpsPort) {
// TODO(cleanup): At this point, all this does is tell Sandcats to refresh certs.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fairly sure at this point this doesn't do anything; the sandcats logic doesn't look for this variable, and grepping around for it didn't come up with any uses.

KJ_SYSCALL(setenv("HTTPS_PORT", kj::str(*httpsPort).cStr(), true));
}

KJ_SYSCALL(setenv("MONGO_URL",
kj::str("mongodb://", authPrefix, "127.0.0.1:", config.mongoPort,
Expand All @@ -2628,7 +2633,7 @@ private:
kj::StringPtr scheme;
uint defaultPort;

if (config.httpsPort == nullptr) {
if (config.httpsPorts.size() == 0) {
scheme = "http://";
defaultPort = 80;
} else {
Expand Down