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

Security by Default / SSL Options #1024

Closed
mdconaway opened this issue May 17, 2022 · 2 comments · May be fixed by #1027
Closed

Security by Default / SSL Options #1024

mdconaway opened this issue May 17, 2022 · 2 comments · May be fixed by #1027
Labels
enhancement New feature or request

Comments

@mdconaway
Copy link

mdconaway commented May 17, 2022

Issues:
-Initial RTL config/setup is insecure by default on LAN networks / intranets with many devices. The RTL node.js server defaults to listening on host "localhost" which exposes the RTL server to all devices on the LAN. This could allow an attacker with LAN access to brute-force their way into an RTL server and gain control of any funds accessible to RTL. This can be fixed by defaulting unspecified RTL host values to 127.0.0.1, which is the default for wallets such as Specter.
-Additionally, while the RTL README outlines how to setup SSL via nginx, it is also possible to use SSL modules directly on the node.js app instance, to include loading a certificate authority into the SSL module. Adding this SSL feature as an RTL config option could enable future RTL features such as PKI-enabled authentication schemes instead of username-password authentication, with config.json specified lists of PKI clients allowed to manage the server. In such a setup, an RTL admin could use easy-rsa to build a PKI certificate authority chain, load the server cert, key and CA in node.js, read an incoming browser/client cert, and take potential further node.js context actions based on the incoming certificate value. This would greatly enhance secure RTL use in corporate environments, large LAN, or WAN scenarios. Use of SSL directly on the node.js app would also reduce port utilization/exposure on the RTL machine as it would only require the use of the node.js port and not an nginx port.

RTL Maintainer Input:
I would be happy to contribute to or fix the above issues, add SSL features, and submit a pull request, but would like input/guidance from the RTL maintainers before coding / submitting. What features/defaults/config var names would the maintainers prefer or accept for merge?

Alternatives:
-An alternative solution to the localhost problem would be to add additional information to the README to recommend to users they should change the "host" option to 127.0.0.1 if they intend to use RTL only on the local machine.
-There isn't a simple alternative to the node.j SSL feature request. While client certificate values can be passed to RTL via headers from nginx or apache, header values are an easy attack vector when targetting a cleartext http server.

Would be happy to continue the discussion on this as well!

mdconaway added a commit to mdconaway/RTL that referenced this issue May 18, 2022
-Add native SSL support
-Add deprecation warning for missing host value
-Add self-signed certificate factory
-Add prettier formatter for server code
-Update README
@mdconaway
Copy link
Author

The pull request only looks big because I added prettier to format the backend code while it is still in typescript format. Hopefully that helps some with the server-side linting moving forward!

mdconaway added a commit to mdconaway/RTL that referenced this issue May 18, 2022
-Add native SSL support
-Add deprecation warning for missing host value
-Add self-signed certificate factory
-Add prettier formatter for server code
-Update README
mdconaway added a commit to mdconaway/RTL that referenced this issue May 24, 2022
-Add native SSL support
-Add deprecation warning for missing host value
-Add self-signed certificate factory
-Add prettier formatter to package.json
-Update README
@saubyk saubyk added the enhancement New feature or request label Aug 14, 2022
@saubyk saubyk added this to the Release 0.13.3-beta milestone Nov 22, 2022
@ShahanaFarooqui ShahanaFarooqui modified the milestones: Release 0.14.1-beta, Release 0.14.2-beta Oct 4, 2023
@ShahanaFarooqui ShahanaFarooqui modified the milestones: Release 0.14.2-beta, Release 0.15.0-beta Oct 13, 2023
@saubyk saubyk modified the milestones: 0.15.1, 0.15.2 Mar 19, 2024
@saubyk
Copy link
Collaborator

saubyk commented Mar 19, 2024

Closing this issue as adding a native SSL capability also adds other dependencies which can result in vulnerability creep

@saubyk saubyk closed this as completed Mar 19, 2024
@saubyk saubyk removed this from the 0.15.2 milestone Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants