Skip to content

Added Native and Automatic HTTPS support to Rest, fixes #280#284

Merged
fboucquez merged 10 commits intodevfrom
rest-ssl-automatic-native
Aug 17, 2021
Merged

Added Native and Automatic HTTPS support to Rest, fixes #280#284
fboucquez merged 10 commits intodevfrom
rest-ssl-automatic-native

Conversation

@yilmazbahadir
Copy link
Copy Markdown
Collaborator

Fixes #280

  • Added HTTPS support selection to the wizard (Native(default) | Automatic | None)
    • Native HTTPS support selection will ask user to provide SSL key and certificate, file contents will be stored in the custom-preset file as base64 and encrypted(key), will be copied over the target/gateways folder during compose
    • Automatic HTTPS support selection will result in setting gateways[].httpsProxy(true|number) which will integrate Https-Portal into the docker-compose.yml file during compose, default inbound port will be 3001
    • None: continue to have an insecure protocol as it used to be, HTTP
  • HttpsProxies preset is added to the ConfigPreset with the help of this preset user will be able to have more control over the parameters.
  • Added unit tests

Comment thread presets/testnet/network.yml Outdated
Comment thread test/reports/bootstrap-voting/rest-gateway-0-rest.json
Comment thread src/service/ConfigService.ts
Comment thread src/commands/wizard.ts Outdated
Comment thread src/service/ComposeService.ts Outdated
Comment thread src/service/ComposeService.ts Outdated
services.push(
await resolveService(n, {
container_name: n.name,
// user, // TODO fix the user permissions issue
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Does the user need Sudo to clean up the target folder? Are sudo files stored in the HTTP proxy volumen?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not I'm afraid, I think it's because of the 's6-overlay' that the https-portal docker file is using. Couldn't find a decent solution yet but will continue to research; I think that docker file needs to do the setup with root user and then use a less privileged user. We might end up extending that docker file and keeping a less privileged user.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

what if we don't use volumes and SSL cert is reevaluated on restart? not great but may solve the sudo issue?

You can create the empty volumen folder too, which solves some of the issues

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We might end up extending that docker file and keeping a less privileged user. - probably, but given lack of user above, I'm guessing this should be solved sooner rather than later (read: prior to merging to main)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I removed the user and the volume parts from ComposeService. The container will hold the certificates produced unless there is an image upgrade which doesn't happen frequently, so we are safe from the produced file permissions issues. There will be another PR for the symbol-bootstrap stop command improvement (which will prevent containers to be removed when stopped).

Comment thread presets/shared.yml Outdated
symbolFaucetImage: symbolplatform/symbol-faucet:1.0.1-alpha
symbolAgentImage: symbolplatform/symbol-node-rewards-agent:2.0.0
mongoImage: mongo:4.4.3-bionic
httpsPortalImage: steveltn/https-portal:1
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

we would need an alpha rest eventually to test the native implementation

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I'm currently working on the catapult-rest part.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Comment thread presets/mainnet/assembly-dual.yml
Comment thread presets/mainnet/network.yml Outdated
votingKeyDesiredLifetime: 360
votingKeyDesiredFutureLifetime: 60
lastKnownNetworkEpoch: 255
lastKnownNetworkEpoch: 271
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how is this related to this change?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It's an automatic unit test that patches these epochs. The idea is that when a user creates a new voting node, the voting file starts from the most current epoch (in offline mode, in online mode bootstrap queries the network). We can probably disable that unit tests and revert that change. Then manually update that value in other PRs.

Comment thread presets/shared.yml
Comment thread src/commands/wizard.ts
Comment thread src/model/ConfigPreset.ts
Comment thread src/model/ConfigPreset.ts Outdated
Comment thread test/reports/bootstrap-voting/rest-gateway-0-rest.json Outdated
Comment thread src/service/ConfigService.ts Outdated
Comment thread src/service/ConfigService.ts
@fboucquez
Copy link
Copy Markdown
Owner

fboucquez commented Aug 3, 2021

@yilmazbahadir @Wayonb @gimer , I have drafted how the community can eventually upgrade the nodes to HTTPs using Bootstrap. Please have an initial look so then @segfaultxavi can do his magic

https://hackmd.io/_F7gPWl2Td6WslNZRXeThA?view

@segfaultxavi
Copy link
Copy Markdown
Contributor

@yilmazbahadir @Wayonb @gimer , I have drafted how the community can eventually upgrade the nodes to HTTPs using Bootstrap. Please have an initial look so then @segfaultxavi can do his magic

https://hackmd.io/_F7gPWl2Td6WslNZRXeThA?view

Yeah, ping me when you're done reviewing... and when you give me access to the file :)

@fboucquez
Copy link
Copy Markdown
Owner

Moved to syndicate HackMD https://hackmd.io/Cx1KQCOxRaaHrNNwuNI7CQ?view

Comment thread presets/shared.yml Outdated
Comment thread src/service/ComposeService.ts Outdated
services.push(
await resolveService(n, {
container_name: n.name,
// user, // TODO fix the user permissions issue
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

what if we don't use volumes and SSL cert is reevaluated on restart? not great but may solve the sudo issue?

You can create the empty volumen folder too, which solves some of the issues

Comment thread test/reports/bootstrap-voting/rest-gateway-0-rest.json
Comment thread src/service/ConfigService.ts
Comment thread presets/mainnet/assembly-dual.yml
Comment thread src/model/ConfigPreset.ts
Comment thread src/model/ConfigPreset.ts Outdated
Comment thread src/service/ConfigService.ts
Comment thread src/model/ConfigPreset.ts Outdated
Comment thread src/model/ConfigPreset.ts Outdated
@fboucquez
Copy link
Copy Markdown
Owner

fboucquez commented Aug 11, 2021

I just realized that my comments were pending for the last 2 weeks. I just submitted the comments. Sorry for the confusion guys.

@yilmazbahadir yilmazbahadir force-pushed the rest-ssl-automatic-native branch from 9a7ebd7 to 2b1519a Compare August 13, 2021 12:43
Baha and others added 8 commits August 16, 2021 20:13
fixed typo and unit test.
improved package.json scripts
Removed httpProxy param from gateway, the httpProxies array can handle it.
Added native ssl unit compose unit test
…shared.yml as they are optional and bumped docker compose version in tests
…tal image version

Currently https-portal only works with the root user but this wouldn't be an issue since we don't mount any volumes any more.
symbolbootstrap stop command will actually stop containers and run command will start, unless there is a version upgrade(which will trigger a new container creation)
it wouldn't need to re-sign the certificates. So most of the time it will be reusing the same certificates.
@yilmazbahadir yilmazbahadir force-pushed the rest-ssl-automatic-native branch from 2b1519a to 417a0fa Compare August 16, 2021 19:15
Jaguar0625
Jaguar0625 previously approved these changes Aug 17, 2021
@yilmazbahadir yilmazbahadir force-pushed the rest-ssl-automatic-native branch from e90b154 to 4c7364c Compare August 17, 2021 12:45
@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@fboucquez fboucquez merged commit f340d72 into dev Aug 17, 2021
@fboucquez fboucquez deleted the rest-ssl-automatic-native branch August 17, 2021 14:44
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.

Add HTTPS support to Rest

5 participants