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

Add TLS configuration for server #190

Merged
merged 2 commits into from
Sep 22, 2024
Merged

Conversation

hixichen
Copy link
Contributor

@hixichen hixichen commented Sep 7, 2024

Currently, the server is configured to use an unsecured HTTP connection via http.ListenAndServe() in the main function. This poses a security risk, particularly for the handleIndex function , which may transmit potentially sensitive personal information over an unencrypted channel.

Rationale:

  1. Data Protection: Implementing TLS encryption will safeguard potentially sensitive information from being intercepted in transit.
  2. Security Best Practices: Using HTTPS is standard for modern web applications, especially those handling personal data.
  3. Mitigation of Man-in-the-Middle Attacks: TLS will help prevent unauthorized access to data through network eavesdropping.

@Luzifer
Copy link
Owner

Luzifer commented Sep 13, 2024

Please explain the reasoning behind this PR further. OTS was never intended to be directly exposed to the internet but put behind a TLS terminating ingress / gateway / loadbalancer / proxy: Serve with HTTPs

Why adding TLS support to the server directly? The server should always be shielded by a properly configured LB.

@mshedsilegx
Copy link

mshedsilegx commented Sep 17, 2024

If I may comment, this idea would be of value for compliance reasons, ie when policies require end to end encryption. In many companies, the LB would not be collocated, so the requirement would be: client -> TLS -> LB -> TLS -> OTS:3000

@tchbla
Copy link

tchbla commented Sep 17, 2024

If I may comment, this idea would be of value for compliance reasons, ie when policies require end to end encryption. In many companies, the LB would not be collocated, so the requirement would be: client -> SSL -> LB -> SSL -> OTS:3000

this sounds like the legacy on premise design where a centralized LB is used to publish applications from multiple subnets.
in that case you use small lb container, like nginx running on the same docker / k8s network in order to process the requests and handle also the TLS (hope SSL is not used in you company anymore 😉) this also brings extended capabilities for logging and much more a dedicated edge like nginx will

Signed-off-by: Knut Ahlers <knut@ahlers.me>
@Luzifer
Copy link
Owner

Luzifer commented Sep 22, 2024

Valid points. Hope it doesn't lead too many peeps to expose the Go HTTP server to the public but use a proxy in front of it.

Thank you for your contribution! 🙂

@Luzifer Luzifer changed the title Add tls configuration for server Add TLS configuration for server Sep 22, 2024
@Luzifer Luzifer merged commit 496ace3 into Luzifer:master Sep 22, 2024
3 checks passed
@mshedsilegx
Copy link

Yes, TLSv1.2+, corrected.
This end to end encryption requirement (LB frontend VIP -> backend) will apply also in the cloud, if using a native cloud LB, so it is not only a "legacy" requirement.
Agree, that we should never expose directly the go HTTPS listener externally.

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.

4 participants