Permalink
Browse files

Change context handling for hash and hmac

A security review found a few problems with malloc handling (missing NULL
pointer check on malloc return).

ZRTP and SRTP now provide the memory for the hash and hmac context inside
their class thus don't rely on dynamic memory allocation. This reduces the
number of malloc calls and also removes chances to run out-of-memory during
runtime.
  • Loading branch information...
wernerd committed Jan 16, 2015
1 parent 89abd98 commit 648d5517feae52da84e7789c0609a600aab0db02
View
@@ -656,7 +656,7 @@ static int ecDoublePointEd(const EcCurve *curve, EcPoint *R, const EcPoint *P)
/* Compute Ry */
bnCopy(curve->t2, R->x);
bnSubMod_(curve->t2, R->y, curve->p); /* C - D */
bnMulMod_(R->y, curve->t1, curve->t2, curve->p, curve); /* E * t3; Ry */
bnMulMod_(R->y, curve->t1, curve->t2, curve->p, curve); /* E * t2; Ry */
/* Compute Rx */
bnSubMod_(curve->t0, curve->t1, curve->p); /* B - E; sub result */
@@ -852,7 +852,7 @@ static int ecAddPointEd(const EcCurve *curve, EcPoint *R, const EcPoint *P, cons
ptQ = Q;
/* Compute A, C, D first */
bnMulMod_(R->z, ptP->z, ptQ->z, curve->p, curve); /* Rz -> A; (Z1 * z2); Rz becomes R3 */
bnMulMod_(R->z, ptP->z, ptQ->z, curve->p, curve); /* Rz -> A; (Z1 * Z2); Rz becomes R3 */
bnMulMod_(R->x, ptP->x, ptQ->x, curve->p, curve); /* Rx -> C; (X1 * X2); Rx becomes R1 */
bnMulMod_(R->y, ptP->y, ptQ->y, curve->p, curve); /* Ry -> D; (Y1 * Y2); Ry becomes R2 */
@@ -989,13 +989,16 @@ int ecGenerateRandomNumber(const EcCurve *curve, BigNum *d)
return curve->randomOp(curve, d);
}
#define MAX_RANDOM_BYTES 128 // Curve 521 (our biggest curve) requires 66 (+8) bytes of random data
static int ecGenerateRandomNumberNist(const EcCurve *curve, BigNum *d)
{
BigNum c, nMinusOne;
uint8_t ran[MAX_RANDOM_BYTES];
size_t randomBytes = ((bnBits(curve->n) + 64) + 7) / 8;
uint8_t *ran = malloc(randomBytes);
if (randomBytes > MAX_RANDOM_BYTES)
return -1;
bnBegin(&c);
bnBegin(&nMinusOne);
@@ -1015,7 +1018,6 @@ static int ecGenerateRandomNumberNist(const EcCurve *curve, BigNum *d)
bnEnd(&c);
bnEnd(&nMinusOne);
free(ran);
return 0;
}
@@ -1100,8 +1102,8 @@ static int ecCheckPubKey3617(const EcCurve *curve, const EcPoint *pub)
bnCopy(curve->t3, curve->t1); /* Load t3 */
bnAddMod_(curve->t3, curve->t2, curve->p); /* t3 = t1 + t2, (x^2+y^2)*/
bnMulMod_(curve->t0, curve->a, curve->t1, curve->p, curve); /* t0 = a * t1, (3617 * x^2) */
bnMulMod_(curve->t0, curve->t0, curve->t2, curve->p, curve); /* t0 = t0 * t1, (3617 * x^2 * y^2) */
bnMulMod_(curve->t0, curve->a, curve->t1, curve->p, curve); /* t0 = a * t1, (3617 * y^2) */
bnMulMod_(curve->t0, curve->t0, curve->t2, curve->p, curve); /* t0 = t0 * t2, (3617 * x^2 * y^2) */
bnAddMod_(curve->t0, mpiOne, curve->p); /* t0 = t0 + 1, (3617 * x^2 * y^2 + 1) */
if (bnCmp (curve->t0, curve->t3) != 0) {
View
@@ -212,6 +212,8 @@ int ecGetAffine(const EcCurve *curve, EcPoint *R, const EcPoint *P);
* @param curve the NIST curve to use.
*
* @param d receives the generated random number.
*
* @return 0 if random data generation is OK, <0 in case of an error.
*/
int ecGenerateRandomNumber(const NistECpCurve *curve, BigNum *d);
@@ -194,21 +194,19 @@ int main(int argc,char **argv) {
displayError("recvfrom(2)");
}
// hexdump("Data before processing", buffer, length);
// if (!session->isStarted(CtZrtpSession::AudioStream))
// session->start(uiSSRC, CtZrtpSession::AudioStream);
/*
* process incoming data
*/
size_t newLength;
size_t newLength = 0;
int rc = session->processIncomingRtp(buffer, length, &newLength, CtZrtpSession::AudioStream);
fprintf(stderr, "processing returns: %d\n", rc);
fprintf(stderr, "processing returns: %d, length: %ld, newLength: %ld\n", rc, length, newLength);
// hexdump("Data after processing", buffer, newLength);
if (rc == 0)
continue; // drop packet
fprintf(stderr, "Received data: %s\n", &buffer[12]); // assume normal RTP packet for debug printout
if (buffer[12] == 'e')
break;
}
/*
* Close the socket and exit:
View
@@ -52,12 +52,23 @@ void* createSkeinMacContext(uint8_t* key, int32_t key_length,
int32_t mac_length, SkeinSize_t skeinSize)
{
SkeinCtx_t* ctx = (SkeinCtx_t*)malloc(sizeof(SkeinCtx_t));
if (ctx == NULL)
return NULL;
skeinCtxPrepare(ctx, skeinSize);
skeinMacInit(ctx, key, key_length, mac_length);
return ctx;
}
void* initializeSkeinMacContext(void* ctx, uint8_t* key, int32_t key_length, int32_t mac_length, SkeinSize_t skeinSize)
{
SkeinCtx_t* pctx = (SkeinCtx_t*)ctx;
skeinCtxPrepare(pctx, skeinSize);
skeinMacInit(pctx, key, key_length, mac_length);
return (void*)pctx;
}
void macSkeinCtx(void* ctx, const uint8_t* data, uint32_t data_length,
uint8_t* mac)
{
View
@@ -94,10 +94,29 @@ void macSkein( uint8_t* key, int32_t key_length,
* Integer that contains the length of the MAC in bits (not bytes).
* @param skeinSize
* The Skein size to use.
* @return Returns a pointer to the initialized context or @c NULL in case of an error.
*/
void* createSkeinMacContext(uint8_t* key, int32_t key_length, int32_t mac_length, SkeinSize_t skeinSize);
/**
* Initialize a Skein MAC context.
*
* An application uses this context to hash several data with on Skein MAC
* Context with the same key, key length and mac length
*
* @param ctx
* Pointer to initialized Skein MAC context
* @param key
* The MAC key.
* @param key_length
* Lenght of the MAC key in bytes
* @param mac_length
* Integer that contains the length of the MAC in bits (not bytes).
* @param skeinSize
* The Skein size to use.
* @return Returns a pointer to the initialized context
*/
void* createSkeinMacContext(uint8_t* key, int32_t key_length,
int32_t mac_length, SkeinSize_t skeinSize);
void* initializeSkeinMacContext(void* ctx, uint8_t* key, int32_t key_length, int32_t mac_length, SkeinSize_t skeinSize);
/**
* Compute Skein MAC.
View
@@ -28,8 +28,6 @@
#include <CryptoContext.h>
#include <crypto/SrtpSymCrypto.h>
#include <crypto/hmac.h>
#include <cryptcommon/macSkein.h>
CryptoContext::CryptoContext( uint32_t ssrc,
int32_t roc,
@@ -158,19 +156,6 @@ CryptoContext::~CryptoContext() {
delete f8Cipher;
f8Cipher = NULL;
}
if (macCtx != NULL) {
switch(aalg) {
case SrtpAuthenticationSha1Hmac:
freeSha1HmacContext(macCtx);
break;
case SrtpAuthenticationSkeinHmac:
freeSkeinMacContext(macCtx);
break;
}
}
ealg = SrtpEncryptionNull;
aalg = SrtpAuthenticationNull;
}
void CryptoContext::srtpEncrypt(uint8_t* pkt, uint8_t* payload, uint32_t paylen, uint64_t index, uint32_t ssrc ) {
@@ -324,11 +309,14 @@ void CryptoContext::deriveSrtpKeys(uint64_t index)
// Initialize MAC context with the derived key
switch (aalg) {
case SrtpAuthenticationSha1Hmac:
macCtx = createSha1HmacContext(k_a, n_a);
macCtx = &hmacCtx.hmacSha1Ctx;
macCtx = initializeSha1HmacContext(macCtx, k_a, n_a);
break;
case SrtpAuthenticationSkeinHmac:
macCtx = &hmacCtx.hmacSkeinCtx;
// Skein MAC uses number of bits as MAC size, not just bytes
macCtx = createSkeinMacContext(k_a, n_a, tagLength*8, Skein512);
macCtx = initializeSkeinMacContext(macCtx, k_a, n_a, tagLength*8, Skein512);
break;
}
memset(k_a, 0, n_a);
View
@@ -42,6 +42,8 @@ const int SrtpEncryptionTWOF8 = 4;
#ifndef CRYPTOCONTEXTCTRL_H
#include <stdint.h>
#include <crypto/hmac.h>
#include <cryptcommon/macSkein.h>
class SrtpSymCrypto;
@@ -413,6 +415,11 @@ class CryptoContext {
CryptoContext* newCryptoContextForSSRC(uint32_t ssrc, int roc, int64_t keyDerivRate);
private:
typedef union _hmacCtx {
SkeinCtx_t hmacSkeinCtx;
hmacSha1Context hmacSha1Ctx;
} HmacCtx;
uint32_t ssrcCtx;
uint32_t mkiLength;
@@ -449,6 +456,7 @@ class CryptoContext {
bool seqNumSet;
void* macCtx;
HmacCtx hmacCtx;
SrtpSymCrypto* cipher;
SrtpSymCrypto* f8Cipher;
View
@@ -30,8 +30,6 @@
#include <CryptoContext.h>
#include <crypto/SrtpSymCrypto.h>
#include <crypto/hmac.h>
#include <cryptcommon/macSkein.h>
CryptoContextCtrl::CryptoContextCtrl(uint32_t ssrc,
@@ -157,19 +155,6 @@ CryptoContextCtrl::~CryptoContextCtrl(){
delete f8Cipher;
f8Cipher = NULL;
}
if (macCtx != NULL) {
switch(aalg) {
case SrtpAuthenticationSha1Hmac:
freeSha1HmacContext(macCtx);
break;
case SrtpAuthenticationSkeinHmac:
freeSkeinMacContext(macCtx);
break;
}
}
ealg = SrtpEncryptionNull;
aalg = SrtpAuthenticationNull;
}
void CryptoContextCtrl::srtcpEncrypt( uint8_t* rtp, int32_t len, uint32_t index, uint32_t ssrc )
@@ -321,11 +306,14 @@ void CryptoContextCtrl::deriveSrtcpKeys()
// Initialize MAC context with the derived key
switch (aalg) {
case SrtpAuthenticationSha1Hmac:
macCtx = createSha1HmacContext(k_a, n_a);
macCtx = &hmacCtx.hmacSha1Ctx;
macCtx = initializeSha1HmacContext(macCtx, k_a, n_a);
break;
case SrtpAuthenticationSkeinHmac:
macCtx = &hmacCtx.hmacSkeinCtx;
// Skein MAC uses number of bits as MAC size, not just bytes
macCtx = createSkeinMacContext(k_a, n_a, tagLength*8, Skein512);
macCtx = initializeSkeinMacContext(macCtx, k_a, n_a, tagLength*8, Skein512);
break;
}
memset(k_a, 0, n_a);
View
@@ -26,6 +26,9 @@
* @{
*/
#include <crypto/hmac.h>
#include <cryptcommon/macSkein.h>
class SrtpSymCrypto;
/**
@@ -294,6 +297,11 @@ class CryptoContextCtrl {
private:
typedef union _hmacCtx {
SkeinCtx_t hmacSkeinCtx;
hmacSha1Context hmacSha1Ctx;
} HmacCtx;
uint32_t ssrcCtx;
uint32_t mkiLength;
uint8_t* mki;
@@ -326,6 +334,7 @@ class CryptoContextCtrl {
uint8_t labelBase;
void* macCtx;
HmacCtx hmacCtx;
SrtpSymCrypto* cipher;
SrtpSymCrypto* f8Cipher;
View
@@ -36,15 +36,8 @@
#include <stdint.h>
#include <string.h>
#include <stdio.h>
#include "crypto/sha1.h"
#include "crypto/hmac.h"
typedef struct _hmacSha1Context {
sha1_ctx ctx;
sha1_ctx innerCtx;
sha1_ctx outerCtx;
} hmacSha1Context;
static int32_t hmacSha1Init(hmacSha1Context *ctx, const uint8_t *key, uint32_t kLength)
{
int32_t i;
@@ -146,11 +139,21 @@ void hmac_sha1( uint8_t* key, int32_t keyLength, const uint8_t* dataChunks[], ui
void* createSha1HmacContext(uint8_t* key, int32_t keyLength)
{
hmacSha1Context *ctx = reinterpret_cast<hmacSha1Context*>(malloc(sizeof(hmacSha1Context)));
if (ctx == NULL)
return NULL;
hmacSha1Init(ctx, key, keyLength);
return ctx;
}
void* initializeSha1HmacContext(void* ctx, uint8_t* key, int32_t keyLength)
{
hmacSha1Context *pctx = (hmacSha1Context*)ctx;
hmacSha1Init(pctx, key, keyLength);
return pctx;
}
void hmacSha1Ctx(void* ctx, const uint8_t* data, uint32_t dataLength,
uint8_t* mac, int32_t* macLength)
{
View
@@ -47,11 +47,19 @@
*/
#include <stdint.h>
#include "crypto/sha1.h"
#ifndef SHA1_DIGEST_LENGTH
#define SHA1_DIGEST_LENGTH 20
#endif
typedef struct _hmacSha1Context {
sha1_ctx ctx;
sha1_ctx innerCtx;
sha1_ctx outerCtx;
} hmacSha1Context;
/**
* Compute SHA1 HMAC.
*
@@ -109,10 +117,25 @@ void hmac_sha1( uint8_t* key, int32_t key_length,
* The MAC key.
* @param key_length
* Lenght of the MAC key in bytes
* @return Returns a pointer to the initialized context
* @return Returns a pointer to the initialized context or @c NULL in case of an error.
*/
void* createSha1HmacContext(uint8_t* key, int32_t key_length);
/**
* Initialize a SHA1 HMAC context.
*
* An application uses this context to create several HMAC with the same key.
*
* @param ctx
* Pointer to initialized SHA1 HMAC context
* @param key
* The MAC key.
* @param key_length
* Lenght of the MAC key in bytes
* @return Returns a pointer to the initialized context.
*/
void* initializeSha1HmacContext(void* ctx, uint8_t* key, int32_t key_length);
/**
* Compute SHA1 HMAC.
*
Oops, something went wrong.

0 comments on commit 648d551

Please sign in to comment.