code review 7197048: crypto/tls: allow alternate low-level cryptographic fun...

crypto/tls: allow alternate low-level cryptographic functions

crypto/tls relies on the crypto/ implementations of AES, 3DES,
MD5, SHA1 and RC4. For performance or security reasons it may
be preferable to use alternate implementations (such as a Go
wrapper to OpenSSL).

This change enables use of alternate AES, 3DES, MD5, SHA1 and
RC4 function by setting global functions that crypto/tls uses
to create cipher or hash objects. By default the standard Go
implementations are used.

[jgc, 2013-01-23 16:31:02]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[rsc, 2013-01-23 22:19:59]

If you want to sub in faster implementations, why not compile a copy of Go
with those implementations in the standard places?

Russ

[jgc_jgc.org, 2013-01-23 22:23:11]

On Wed, Jan 23, 2013 at 10:19 PM, Russ Cox <rsc@golang.org> wrote:

> If you want to sub in faster implementations, why not compile a copy of Go
> with those implementations in the standard places?


I can, but then it means that I am maintaining a local copy of Go which I'd
rather avoid. And it may be the case that others have the same speed
critical need for using OpenSSL (or similar) primitives.

John.

[bradfitz, 2013-01-23 22:46:58]

On Wed, Jan 23, 2013 at 2:22 PM, John Graham-Cumming <jgc@jgc.org> wrote:

> On Wed, Jan 23, 2013 at 10:19 PM, Russ Cox <rsc@golang.org> wrote:
>
>> If you want to sub in faster implementations, why not compile a copy of
>> Go with those implementations in the standard places?
>
>
> I can, but then it means that I am maintaining a local copy of Go which
> I'd rather avoid. And it may be the case that others have the same speed
> critical need for using OpenSSL (or similar) primitives.


One fear is that this crutch leads to everybody doing this, and Go's crypto
not getting faster.

[agl2, 2013-01-23 22:52:41]

We already have a registry of hash functions in crypto. I think a
patch that gets the hash function from that registry rather than
directly would be fine. That registry was designed to allow for
optional hash functions, but using it to switch out implementations
isn't a bother.

However, this change adds an extra registry just in crypto/tls, and
just for a few functions. I don't think that's good. That's why I
suggested that the cipher implementations couldn't be switched out on
the golang-nuts thread.


Cheers

AGL

[jgc_jgc.org, 2013-01-23 23:03:19]

On Wed, Jan 23, 2013 at 10:52 PM, Adam Langley <agl@google.com> wrote:

> We already have a registry of hash functions in crypto. I think a
> patch that gets the hash function from that registry rather than
> directly would be fine. That registry was designed to allow for
> optional hash functions, but using it to switch out implementations
> isn't a bother.
>

Yes, it would be possible to use the hash registry, but RC4 is also needed
so it would be necessary create a block cipher (or just cipher) registry as
well so that crypto/tls could get RC4 from it.

I'm happy to write code to do that if people think it would be acceptable.
If that's not acceptable then I'll withdraw this CL and will maintain a
local Go.

[dfc, 2013-01-24 01:17:32]

> One fear is that this crutch leads to everybody doing this, and Go's crypto
> not getting faster.

I understand that John has a deadline, and the squeaky wheel gets the
grease, but I share Brad's concern. Additionally I see this as an
optional feature of the standard library, which means it'll probably
break, and nobody will notice for a long time.

Although unqualified, I am happy to assist manually translating
(properly licensed) code from other public domain projects into plan 9
asm if that will help.

[jgc, 2013-01-28 09:51:44]

On 2013/01/24 01:17:32, dfc wrote:
> I understand that John has a deadline, and the squeaky wheel gets the
> grease, but I share Brad's concern. Additionally I see this as an
> optional feature of the standard library, which means it'll probably
> break, and nobody will notice for a long time.

I will withdraw this change and keep a locally maintained version of Go.

[bradfitz, 2013-06-05 21:04:00]

R=close

rejected / retracted.