Add SSL support to mug

[arnu515]

Hi! I've added some SSL support to mug using Erlang's ssl module. It lives in the mug/ssl module and has an API very similar to the existing TCP API.

Here's an example of connecting to an SSL host:

import gleam/bit_array
import gleam/io
import mug
import mug/ssl

pub fn main() {
  let assert Ok(socket) =
    mug.new("localhost", port: 12345)  // connect to a TCP/SSL echo server
    |> ssl.with_ssl()
    |> ssl.with_cacerts(ssl.NoVerification)  // don't verify if certs are valid (useful for local debugging)
    |> ssl.connect()

  let assert Ok(Nil) = ssl.send(socket, <<"Hello, Joe!\n":utf8>>)
  let assert Ok(packet) = ssl.receive(socket, timeout_milliseconds: 100)
  io.println(bit_array.to_string(packet))  // "Hello, Joe!\n"
}

There are still a few bits and bobs missing, like the ability to upgrade from a regular TCP connection to an encrypted TLS one. There is also an unimplemented function for adding the certs_keys option since I'm not able to figure out how to do it right now (I'll give it a try soon, I'm new to both Gleam and Erlang).

I've added a few tests in test/mug_ssl_test.gleam which are built off of test/mug_test.gleam. One of them isn't currently working because I'm not able to figure out some ca certificate nonsense (hence, it has been commented out).

Please let me know if any changes have to be done, and I'd request you check my doc comments for any grammatical errors, since English is not my first language. Thanks!

[arnu515]

Hi! I've added the ability to upgrade a TCP connection to a TLS connection via upgrade and upgrade3.

There are still two things left:

• Supporting certs_keys
• Fixing that CA certificate test

(closed by mistake, sorry!)

[lpil]

This is looking really nice! Very clean!! I've left some minor notes inline.

When you are ready for a review please un-draft this PR. Thank you!

[arnu515]

I've merged the latest changes from main, but looks like GitHub isn't showing it here for some reason

[lpil]

Great work, thank you! I've left notes on the API largely. The TLS side I'm less qualified to speak about, but it looks good from what I can tell.

[lpil]

What's this dynamic?

[arnu515]

The docs types it as any, and doesn't explain what it's supposed to be, but from conventions I assume that this is raised if a passed option is invalid.

[lpil]

This is large a duplication of the existing error type, let's merge them into one.

[lpil]

Why do we load the certs rather than have the SSL application do that? It would be able to cache them, for example.

[arnu515]

I don't quite understand what you mean. Does the SSL application automatically load system cacerts?

[arnu515]

And caching is done by erlang's ssl module automatically

[lpil]

This module seems to be almost exactly the same as the existing module. We could merge them into one rather than having duplicate functions for the programmer to pick between.

[arnu515]

For merging them into one, I wonder how common methods like send and recv can be realised. Do I add prefixes, like ssl_send, or do I have some kind of record that holds metadata and match on it?

I'd request you to give me another review of the code, and bring to my notice any other problems, which I'll fix before doing this refactoring.

[lpil]

That sounds good! I presume we cannot cheaply and reliably introspect the connection to determine what mode it is in.

[arnu515]

Hi! I've merged the two files into one. There's a unified API now for both TCP and TLS.

A simple example:

let assert Ok(socket) =
  mug.new("echo.example.com", port: 443)
  |> mug.timeout(milliseconds: 10_000)
  |> mug.with_tls()  // enable TLS!
  |> mug.connect()
let assert Ok(Nil) = mug.send(socket, <<"Hello, Joe!\n":utf8>>)
let assert Ok(data) = mug.receive(socket, 500)
should.equal(data, <<"Hello, Joe!\n":utf8>>)
let assert Ok(_) = mug.shutdown(socket)

[lpil]

Thank you! Great work here! Here's some notes:

The tests fail cryptically when run now due to certs being missing. The developer-experience needs to be good, so people don't get stuck there not knowing what to do. It would be good if it was automatic, or for the script being at the root (so it's more easy to run) and for instructions to be given to the programmer telling them what to do, instead of a cryptic crash.

The terms TLS and SSL are both used here, but it's not clear to me why either is picked in each case or what the difference is in this package. Could you expand on this a little please? 🙏

Some of the functions are missing type annotations, could you ensure they all have all their annotations please.

For a few names abbreviations have been used and wordshavebeenwrittenlikethis. Please use full words always and put underscores between words, with the exception for existing established abbreviations or initialisms or acronyms such as TLS.

Remove all the use of catch all patterns (_ -> ...) please, they cause bugs when types change as the logic doesn't get updated.

The builder API can silently fail to configure TLS if you give it certificates but TLS has not been enabled, which seems like it could be error prone. Let's remove these builder functions and have the programmer explicitly construct the TLS configuration, to make it clear as possible.

And thank you for waiting while I dealt with some family matters.

[lpil]

Suggested change

	certfile: String,
	keyfile: String,
	certificate_path: String,
	key_path: String,

[lpil]

Suggested change

	DerEncodedKey(alg: String, key: BitArray)
	DerEncodedKey(algorithm: String, key: BitArray)

If the string has to be one of a known finite variants then this should be a custom type rather than a string.

[lpil]

Suggested change

	certs_keys: List(CertsKeys),
	certificate_keys: List(CertsKeys),

[lpil]

This custom type is plural, but the data within is singular. Is this correct?

[arnu515]

That's because the certfile has a chain of certificates inside it.

[lpil]

It looks like one can use system certs wit custom certs, but the custom certs must all be der or pem. How come that specific restriction?

[arnu515]

If PEM certificates are passed, then the ssl application ignores DER certificates, including system cacerts, which are DER encoded. Thus if the user passes like a hundred DER certificates, passing just one PEM certificate will result in all those DER certs being ignored.