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

HTTP::Server crash with self-signed certificate #6577

Closed
bararchy opened this issue Aug 20, 2018 · 3 comments · Fixed by #6590
Closed

HTTP::Server crash with self-signed certificate #6577

bararchy opened this issue Aug 20, 2018 · 3 comments · Fixed by #6590

Comments

@bararchy
Copy link
Contributor

It is possible in the latest version to crash without rescuable warnings an HTTP::Server that uses openssl

Example code:

require "openssl"
require "http/server"

server = HTTP::Server.new do |context|
  context.response.content_type = "text/plain"
  context.response.print "Hello world!"
end

context = OpenSSL::SSL::Context::Server.new
context.certificate_chain = "cert.pem"
context.private_key = "key.pem"
server.bind_ssl "127.0.0.1", 443, context

server.listen

Using a curl https://127.0.0.1 will crash the application.

When using a self-signed certificate, or when the connecting client will generate an OpenSSL "SSL certificate problem: unable to get local issuer certificate" the ssl server process will exit.

This is problematic as basically every exposed crystal server that uses SSL can be crashed using a simple ssl request that has an empty cert store.

@straight-shoota
Copy link
Member

The handling of an HTTP request by a handler is guarded against crashing the entire application. All exceptions are rescued and generate an appropriate HTTP request.

But there is no proper error handling while establishing a client connection in HTTP::Server.listen and setting up the HTTP protocol processing in HTTP::Server::RequestProcessor.process.

The question is, how should the server respond to such an error? Just printing it to STDOUT could be a quick fix but it's not a good solution. Having some sort of error handling with structured concurrency like proposed in #6468 would be great for this.

@bcardiff
Copy link
Member

Closing the connection with some minimum http response like 495 sounds good enough to me in this case.

The whole listen could be guarded to avoid crashing the server. Logging of that can be deferd for now IMO.

@RX14
Copy link
Contributor

RX14 commented Aug 21, 2018

definitely having multiple layers of protection is good for security, guarding the whole listen is a very good idea

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 a pull request may close this issue.

4 participants