wernerd / ZRTPCPP

C++ Implementation of ZRTP protocol - GNU ZRTP C++
Other
116 stars 50 forks source link

openssl/zrtpDH DH3K use only 32 bytes for the private key while it has generated 64 bytes. #30

Open ghost opened 8 years ago

ghost commented 8 years ago

Shouldn't the following code from openssl/zrtpDH by changed?

From this:

        if (pkType == DH2K) {
            tmpCtx->p = BN_dup(bnP2048);
            RAND_bytes(random, 32);
            tmpCtx->priv_key = BN_bin2bn(random, 32, NULL);
        }
        else if (pkType == DH3K) {
            tmpCtx->p = BN_dup(bnP3072);
            RAND_bytes(random, 64);
            tmpCtx->priv_key = BN_bin2bn(random, 32, NULL);
        }

To this:

        if (pkType == DH2K) {
            tmpCtx->p = BN_dup(bnP2048);
            RAND_bytes(random, 32);
            tmpCtx->priv_key = BN_bin2bn(random, 32, NULL);
        }
        else if (pkType == DH3K) {
            tmpCtx->p = BN_dup(bnP3072);
            RAND_bytes(random, 64);
            tmpCtx->priv_key = BN_bin2bn(random, 64, NULL);
        }

I am not actually sure so I am probably wrong but shouldn't big number for DH3K use all the random 64 bytes instead simply the first 32 bytes?

wernerd commented 8 years ago

Am 28.03.2016 um 10:16 schrieb 0x616d:

Shouldn't the following code from openssl/zrtpDH https://github.com/wernerd/ZRTPCPP/blob/master/zrtp/crypto/openssl/zrtpDH.cpp#L237 by changed?

From this:

|if (pkType == DH2K) { tmpCtx->p = BN_dup(bnP2048); RAND_bytes(random, 32); tmpCtx->priv_key = BN_bin2bn(random, 32, NULL); } else if (pkType == DH3K) { tmpCtx->p = BN_dup(bnP3072); RAND_bytes(random, 64); tmpCtx->priv_key = BN_bin2bn(random, 32, NULL); } |

To this:

|if (pkType == DH2K) { tmpCtx->p = BN_dup(bnP2048); RAND_bytes(random, 32); tmpCtx->priv_key = BN_bin2bn(random, 32, NULL); } else if (pkType == DH3K) { tmpCtx->p = BN_dup(bnP3072); RAND_bytes(random, 64); tmpCtx->priv_key = BN_bin2bn(random, 64, NULL); } |

I am not actually sure so I am probably wrong but shouldn't big number for DH3K use all the random 64 bytes instead simply the first 32 bytes?

Maybe a copy/paste error :-) , maybe we decided to use 32 bytes only after some testing. Using the full 64bytes as the exponent requires a lot of CPU cycles. Can't remember because I haven't touched this specific code for a long time.

However, it's not a real security issue. Actually, the secret key (the secrect DH exponent) should be twice as long as the key of the symmetric encryption algo that's being used later, i.e. it's OK for AES-128, should be 64 if the client uses AES-256. As said using the full 64 bytes requires much more CPU cycles and may lead to timeouts etc.

If you like to use AES-256 or Twofish-256 I would propose to use the EC-DH public key algorithms.

Werner

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/wernerd/ZRTPCPP/issues/30

Werner Dittmann email: Werner.Dittmann@t-online.de cell: +49 173 44 37 659 PGP key: 82EF5E8B