chiark / gitweb /
site.c: Pass the length of the actual shared secret to the transform.
authorMark Wooding <mdw@distorted.org.uk>
Sat, 29 Apr 2017 12:55:40 +0000 (13:55 +0100)
committerMark Wooding <mdw@distorted.org.uk>
Wed, 25 Sep 2019 12:46:59 +0000 (13:46 +0100)
commit3f488f14527df0f3da6616bcc26c2cf69f627df9
treee0814237f52ce8df9b34eb3d6f20a900f409c7e5
parent5f679c36ccb3e125ee6e8372a60d0f2e8bf410fe
site.c: Pass the length of the actual shared secret to the transform.

The `set_new_transform' function used to grow its `sharedsecret' buffer
to accommodate the chosen transform's desired key length, and then tells
the transform that this is the size of its secret.

Unfortunately this is pretty much a lie.  In particular, the traditional
DH closure doesn't actually do anything to fill the rest of the buffer
with random stuff.  Probably there ought to be a KDF here, but:

  * we can't introduce a KDF globally without breaking compatibility
    with old clients; and

  * the new EAX-based transform has its own cheap-and-cheerful (but
    effective) SHA512-based KDF baked into it.

Anyway, the result is that, if the DH group produces short shared
secrets, and the transform has an explicit key size it wants, then
everything will seem to work right up until the transform tries to use
uninitialized memory as key material.  Then the good news is that the
two sites likely end up using different keys and can't talk to each
other.  The /bad/ news is that their keys don't have enough entropy, and
an adversary may be able to impersonate them to each other.

We're probably not in this situation yet.  We have two transforms and
one DH group type.  One transform has its own KDF, so is unaffected by
this.  The other, the old `serpent256-cbc (or is it `serpent-cbc256'?)
transform, wants 608 bits (76 bytes) of key.  It gets these directly
from the big-endian base-256 encoded DH shared secret, so we OK unless
the DH field is smaller than 608 bits.  But if it is then you have other
problems.

Surprisingly, the fix is for the site code to ignore the transform's
reported key size entirely.  It tells the transform the size of the
shared secret, and if the transform is unhappy then it can fail or apply
a KDF by itself.

Of course, now we're doing this, there's no need for the transform to
advertise a desired key length, so remove this.  Also, this means that
the shared secret buffer isn't going to change size any more, so we can
remove all of the machinery for that, too.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
secnet.h
site.c
transform-cbcmac.c
transform-eax.c