Remove Crystal::Digest implementations

[straight-shoota]

Crystal::Digest::MD5 is an internal implementation used by the compiler, in only one location:

crystal/src/compiler/crystal/compiler.cr

Line 644 in 33c97d4

@name = "#{@name[0..16]}-#{::Crystal::Digest::MD5.hexdigest(@name)}"

But this use case doesn't need MD5 at all. The purpose is to restrict long names to 50 characters while still having a unique identifier. So there's really no need for a cryptographic hash function.

The same can be achieved much simpler. For example String#hash should do as a hash function. Or it could use a counter and names get assigned sequential numeric identifiers.

If we change that, we can remove Crystal::Digest::MD5 from the code base.

---

There's also Crystal::Digest::SHA1 which has a similar purpose only for the compiler's playground when compiled without openssl.

crystal/src/http/web_socket/protocol.cr

Lines 330 to 332 in 33c97d4

	{% if flag?(:without_openssl) %}
	::Crystal::Digest::SHA1.base64digest(key + GUID)
	{% else %}

Techincally it's also used for any program that uses the websocket implementation is compiled without openssl. But that's not really an intended use case.

Since the playground is only a development tool security concerns really don't matter much. But it seems impossible to avoid because the Sec-WebSocket-Accept header with a SHA1 hashed value is mandatory for establishing a websocket connection.

[j8r]

Ideally any program wanting to use WebSocket should compile with openssl, for security reasons and also performance.

The playground should not be needed to bootstrap the compiler. An option is to not have the Playground at all when openssl is not available. Then, no Digest will be needed anymore.

[straight-shoota]

Compiler builds are usually not linked against openssl (AFAIK because static linking is difficult). It would mean the playground would generally not be available. That's a major change and basically drop the playground from the compiler entirely.

[asterite]

There's simply no rush at all to change this.

[j8r]

You're both right. Then, for the moment, just changing the MD5 use in crystal/src/compiler/crystal/compiler.cr will be good.

I don't know how the playground could possibly use the local host's openssl library after being compiled.

As a side note, if digests are dropped from this repository, could they be moved in a repository?

[straight-shoota]

@asterite No rush, yes. But there can still be an issue to track this.

As a side note, if digests are dropped from this repository, could they be moved in a repository?

This is only about internal implementations. They're not intended for public use. So there's no reason to make it available in separate repo. At least not in crystal-lang, if somebody wants to publish a shard there's noone stopping you. I wouldn't recommend it though.

[j8r]

It would be good for educational purposes, and some non security related cases. Their implementations could even be improved and audited, who knows.

[SamantazFox]

Why would you remove those digests? It's kinda useful to be able to compute a MD5/SHA* sum in order to veryfy a download, in a os-agnostic way and without having to write to file.

Or am I confugisng this with some other implementation?

[straight-shoota]

These are pure Crystal implementations and neither optimized for performance nor security (not that the latter matters too much anymore).

Both digest methods are available in stdlib via bindings to ˋlibcryptoˋ in the ˋOpenSSLˋ namespace. So as long as you can link ˋlibcryptoˋ, you already have a much better implementation available and there's no need for primitive implementation in pure Crystal.