Skip to content
This repository has been archived by the owner on Jun 10, 2022. It is now read-only.

Add native HTTPS support #622

Open
fboucquez opened this issue May 17, 2021 · 9 comments
Open

Add native HTTPS support #622

fboucquez opened this issue May 17, 2021 · 9 comments
Labels
P3 Issue Low severity cosmetic issues with minor inconvenience

Comments

@fboucquez
Copy link
Collaborator

fboucquez commented May 17, 2021

Atm, Catapult Rest only allows HTTP at port 3000.

Although node admins can add SSL to the Rest server by adding a ngnix style proxy or via a Cloud provider balancer, it would be nice if Rest accepts a custom certificate. If the custom certificate is provided, Rest can start its Restify server with SSL. In this case, the new default port should be different, like 3001 or 3443.

Changing the default protocol and port may affect services like the reward controller, explorer, or wallet. The way the rest URLs are stored needs to be revisited.

Bootstrap would need to be updated so the node admin can provide a custom certificate via a custom preset.

@fboucquez fboucquez added the P3 Issue Low severity cosmetic issues with minor inconvenience label May 17, 2021
@gimre-xymcity
Copy link
Member

gimre-xymcity commented May 17, 2021

this should not be done in rest layer, it gives much more flexibility to do it via some external webserver (be it nginx, h2o, apache, lighttpd) or any other thing.

@fboucquez
Copy link
Collaborator Author

fboucquez commented May 17, 2021

Built-in rest HTTPs would be an optional setup for "out-of-the-box" secure connection. We are using SSL connections in express agents already. Node admins would have the option to have it separate if they need fine-tuning, but SSL on Restify may be good and simple enough setup.

If we want to keep HTTPs separate, we would need to document how to create the webserver for manual setup (different os flavors) and add a webserver docker service to bootstrap docker-compose.

@gimre-xymcity
Copy link
Member

add a webserver docker service to bootstrap docker-compose.

why is that? I thought bootstrap is supposed to get thinner not fatter.

@fboucquez
Copy link
Collaborator Author

fboucquez commented May 17, 2021

why is that? I thought bootstrap is supposed to get thinner not fatter.

If we want bootstrap to support out-of-the-box https, the options are:

  1. Https Restify
  2. Ngnix like webserver running next to rest in the same image/container
  3. Ngnix like webserver running in its own container/service in docker compose

Another option would be for the node owner to manually install a webserver in the host machine outside docker. The installation will depend on the host's OS. This a manual option n4

My vote is for no 1, built-in HTTPS in rest. It doesn't require a separate server, it works for bootstrap a manual node, less documentation, it's not OS dependent, and bootstrap doesn't get any fatter.

@fboucquez
Copy link
Collaborator Author

The community is already doing n3 manually https://nemlog.nem.social/blog/58808. The issue is that the upgrade process will override any manual change of the target folder like the docker-compose file

@u2yasan
Copy link

u2yasan commented May 18, 2021

this method: https://nemlog.nem.social/blog/58808 does not change any file in symbol-bootstrap directory. this creates another docker-compose.yml file. docker-compose can communicate with other docker ps from other docker-compse.

@gimre-xymcity
Copy link
Member

If we want bootstrap to support out-of-the-box https, the options are:

that's exactly why I'm asking, I don't see reason why bootstrap should be doing this "out of the box"

@fboucquez
Copy link
Collaborator Author

Thanks @u2yasan , the blog post it's actually option 4. Custom installed web-server (in this case via another docker-compose file).

Adding out-of-the-box SSL to catapult rest is just 2 files setup.

var server = restify.createServer({
    certificate: fs.readFileSync("./certs/server.crt"),
    key : fs.readFileSync("./certs/server.key"),
});

The file locations can be provided via the rest.json file like the user already provides the certificates for the rest -> server API communication. This is an optional rest feature the user may or may not want to use it. The bootstrap update would be to add those to the mounted volume the same way the server certificates are provided.

@gimre-xymcity
Copy link
Member

btw, turns out we already have ~600 public nodes with https support...
https://symbolnodes.org/nodes/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P3 Issue Low severity cosmetic issues with minor inconvenience
Projects
None yet
Development

No branches or pull requests

3 participants