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

API for OpenSSL #3865

Merged
merged 8 commits into from Jan 16, 2017
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 49 additions & 0 deletions src/openssl.cr
@@ -1,5 +1,54 @@
require "./openssl/lib_ssl"

# ### Example
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe have a header? Something as basic as # OpenSSL integration

Maybe also have an introduction that says that:

  • TLS sockets need a context, potentially with keys (required for servers) and configuration.
  • TLS sockets will wrap the underlying TCP socket, and any further communication must happen through the OpenSSL::SSL::Socket only.

Then continue with:

  • the default configuration (sane defaults, uses Mozilla Intermediate, best effort, please contribute)
  • the server example —I'm not sure we should add an example about how to generate the certificates, one should buy a certificate instead;
  • the client example.

# ## Server side
# for the below "server" example to work, a key pair should be created, using openssl it can be done like that
# - Generate keys to /tmp/
# - openssl req -x509 -sha256 -nodes -days 365 -newkey rsa:2048 -keyout /tmp/private.key -out /tmp/certificate.crt
# ```crystal
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for crystal prefix here

# require "socket"
# require "openssl"
#
# def server
# socket = TCPServer.new(5555) # Bind new TCPSocket to port 5555
# context = OpenSSL::SSL::Context::Server.new
# # Define which ciphers to use with OpenSSL
# # recommended ciphers can be taken from
# # - https://www.owasp.org/index.php/Transport_Layer_Protection_Cheat_Sheet#Rule_-_Only_Support_Strong_Cryptographic_Ciphers
# # - https://cipherli.st/
# # - Full list is available at: https://wiki.openssl.org/index.php/Manual:Ciphers(1)#CIPHER_STRINGS
# context.ciphers = "EECDH+AESGCM:EDH+AESGCM:AES256+EECDH:AES256+EDH"
Copy link
Member

@jhass jhass Jan 10, 2017

Choose a reason for hiding this comment

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

I'm not sure about adding a valid value here, people should be forced to actually follow the links given rather than copy pasting whatever is here. Also it should be made clear that we do provide a sane enough default.

# context.private_key = "/tmp/private.key"
# context.certificate_chain = "/tmp/certificate.crt"
# # Those options are to enhance the security of the server by not using deprecated SSLv2 and SSLv3 protocols
# # It is also advised to disable Compression and enable only TLS1.2
# context.add_options(OpenSSL::SSL::Options::NO_SSLV2 | OpenSSL::SSL::Options::NO_SSLV3)
Copy link
Member

Choose a reason for hiding this comment

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

Same, those recommendations might get outdated and we do provide sane enough defaults. Or if we don't we should.

# puts "server is up"
# socket.accept do |client|
# puts "got client"
# ssl_socket = OpenSSL::SSL::Socket::Server.new(client, context)
# slice = Slice(UInt8).new(20)
# ssl_socket.read(slice)
# puts String.new(slice)
# end
# end
# ```
# ## Client side
# ```crystal
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

# require "socket"
# require "openssl"
#
# def client
# socket = TCPSocket.new("127.0.0.1", 5555)
# context = OpenSSL::SSL::Context::Client.new
# context.ciphers = "EECDH+AESGCM:EDH+AESGCM:AES256+EECDH:AES256+EDH"
# context.add_options(OpenSSL::SSL::Options::NO_SSLV2 | OpenSSL::SSL::Options::NO_SSLV3)
# context.verify_mode = OpenSSL::SSL::VerifyMode::NONE
Copy link
Member

@jhass jhass Jan 10, 2017

Choose a reason for hiding this comment

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

I wouldn't even mention this option in a usage example. People should pay reasonable research effort into this, so they understand what they're doing with it.

# ssl_socket = OpenSSL::SSL::Socket::Client.new(socket, context)
# ssl_socket.write("Testing".to_slice)
# end
# ```

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove redundant newline.

module OpenSSL
class Error < Exception
getter! code : LibCrypto::ULong?
Expand Down