Skip to content

Commit

Permalink
auth: Avoid abort() if len(secret) is not 32 bytes
Browse files Browse the repository at this point in the history
Before OpenSIPS 3.2, the "secret" modparam supported random-length
strings, to be hashed into an MD5 computation when generating the nonce.

Starting with 3.2 and the new AES-CBC based nonce generation algorithm,
the "secret" has been restricted to 32-bytes only, however OpenSIPS
would assert() -> abort() on startup without displaying any helper error
if the user supplied a different-length secret.

Many thanks to @thuroc for an accurate report on the assert() issue!

Fixes #3043
  • Loading branch information
liviuchircu committed Apr 10, 2023
1 parent d4b03b7 commit 00a19ed
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 20 deletions.
39 changes: 19 additions & 20 deletions lib/digest_auth/dauth_nonce.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,13 @@

#include "dauth_nonce.h"

#define RAND_SECRET_LEN 32
/*
* Length of nonce string in bytes
*/
#define NONCE_LEN 44

_Static_assert((NONCE_LEN * 6) % 8 == 0, "NONCE_LEN should not be padded");
_Static_assert((NONCE_LEN * 6) / 8 >= RAND_SECRET_LEN, "NONCE_LEN is too small");
_Static_assert((NONCE_LEN * 6) / 8 >= AUTH_SECRET_LEN, "NONCE_LEN is too small");

struct nonce_context_priv {
struct nonce_context pub;
Expand All @@ -65,10 +64,10 @@ struct nonce_payload {
uint64_t expires_usec:20;
} __attribute__((__packed__));

_Static_assert(sizeof(struct nonce_payload) <= RAND_SECRET_LEN / 2,
_Static_assert(sizeof(struct nonce_payload) <= AUTH_SECRET_LEN / 2,
"struct nonce_payload is too big");
_Static_assert(RAND_SECRET_LEN % sizeof(uint64_t) == 0,
"RAND_SECRET_LEN is not multiple of sizeof(uint64_t)");
_Static_assert(AUTH_SECRET_LEN % sizeof(uint64_t) == 0,
"AUTH_SECRET_LEN is not multiple of sizeof(uint64_t)");

static int Base64Encode(const str_const *message, char* b64buffer);
static int Base64Decode(const str_const *b64message, unsigned char* obuffer);
Expand Down Expand Up @@ -98,16 +97,16 @@ int calc_nonce(const struct nonce_context *pub, char* _nonce,
const struct nonce_params *npp)
{
struct nonce_context_priv *self = (typeof(self))pub;
unsigned char ebin[RAND_SECRET_LEN + 1];
unsigned char ebin[AUTH_SECRET_LEN + 1];
int rc, elen;
unsigned char dbin[RAND_SECRET_LEN], *bp;
unsigned char dbin[AUTH_SECRET_LEN], *bp;
unsigned char *riv = dbin;

rc = RAND_bytes(riv, RAND_SECRET_LEN / 2);
rc = RAND_bytes(riv, AUTH_SECRET_LEN / 2);
if (rc != 1)
return -1;

bp = dbin + RAND_SECRET_LEN / 2;
bp = dbin + AUTH_SECRET_LEN / 2;
struct nonce_payload npl;
memset(&npl, 0, sizeof(npl));
npl.expires_sec = npp->expires.tv_sec;
Expand All @@ -119,8 +118,8 @@ int calc_nonce(const struct nonce_context *pub, char* _nonce,
bp += sizeof(npl);
memset(bp, 0, sizeof(dbin) - (bp - dbin));

bp = dbin + RAND_SECRET_LEN / 2;
xor_bufs(bp, bp, dbin, RAND_SECRET_LEN / 2);
bp = dbin + AUTH_SECRET_LEN / 2;
xor_bufs(bp, bp, dbin, AUTH_SECRET_LEN / 2);

elen = 0;
rc = EVP_EncryptUpdate(self->ectx, ebin, &elen, dbin, sizeof(dbin));
Expand All @@ -140,10 +139,10 @@ int decr_nonce(const struct nonce_context *pub, const str_const * _n,
struct nonce_params *npp)
{
struct nonce_context_priv *self = (typeof(self))pub;
unsigned char bin[RAND_SECRET_LEN + 1];
unsigned char bin[AUTH_SECRET_LEN + 1];
const unsigned char *bp;
unsigned char *wbp;
unsigned char dbin[RAND_SECRET_LEN];
unsigned char dbin[AUTH_SECRET_LEN];
int rc;

if (_n->len != NONCE_LEN)
Expand All @@ -153,12 +152,12 @@ int decr_nonce(const struct nonce_context *pub, const str_const * _n,
return -1;
int dlen = 0;
bp = (const unsigned char *)bin;
rc = EVP_DecryptUpdate(self->dctx, dbin, &dlen, bp, RAND_SECRET_LEN);
rc = EVP_DecryptUpdate(self->dctx, dbin, &dlen, bp, AUTH_SECRET_LEN);
if (rc != 1 || dlen != sizeof(dbin))
return -1;

wbp = dbin + RAND_SECRET_LEN / 2;
xor_bufs(wbp, wbp, dbin, RAND_SECRET_LEN / 2);
wbp = dbin + AUTH_SECRET_LEN / 2;
xor_bufs(wbp, wbp, dbin, AUTH_SECRET_LEN / 2);

bp = (const unsigned char *)wbp;
struct nonce_payload npl;
Expand Down Expand Up @@ -208,21 +207,21 @@ int generate_random_secret(struct nonce_context *pub)
struct nonce_context_priv *self = (typeof(self))pub;
int rc;

self->sec_rand = (char*)pkg_malloc(RAND_SECRET_LEN);
self->sec_rand = (char*)pkg_malloc(AUTH_SECRET_LEN);
if (!self->sec_rand) {
LM_ERR("no pkg memory left\n");
goto e0;
}

/* the generator is seeded from the core */
rc = RAND_bytes((unsigned char *)self->sec_rand, RAND_SECRET_LEN);
rc = RAND_bytes((unsigned char *)self->sec_rand, AUTH_SECRET_LEN);
if(rc != 1) {
LM_ERR("RAND_bytes() failed, error = %lu\n", ERR_get_error());
goto e1;
}

pub->secret.s = self->sec_rand;
pub->secret.len = RAND_SECRET_LEN;
pub->secret.len = AUTH_SECRET_LEN;

/*LM_DBG("Generated secret: '%.*s'\n", pub->secret.len, pub->secret.s); */

Expand Down Expand Up @@ -347,5 +346,5 @@ static int Base64Decode(const str_const *b64message, unsigned char* obuffer)

rval = EVP_DecodeBlock(obuffer, (const unsigned char *)b64message->s,
b64message->len);
return (rval == (RAND_SECRET_LEN + 1)) ? 0 : -1;
return (rval == (AUTH_SECRET_LEN + 1)) ? 0 : -1;
}
2 changes: 2 additions & 0 deletions lib/digest_auth/dauth_nonce.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
#include "../../str.h"
#include <time.h>

#define AUTH_SECRET_LEN 32

struct nonce_context {
str_const secret; /* secret phrase used to generate nonce */
int nonce_len;
Expand Down
7 changes: 7 additions & 0 deletions modules/auth/auth_mod.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,13 @@ static int mod_init(void)
/* Otherwise use the parameter's value */
ncp->secret.s = sec_param;
ncp->secret.len = strlen(sec_param);

if (ncp->secret.len != AUTH_SECRET_LEN) {
LM_ERR("bad secret length, must be exactly 32 bytes"
" (256-bit AES key), given: %d (changed in OpenSIPS 3.2+)\n",
ncp->secret.len);
return -1;
}
}

if (dauth_noncer_init(ncp) < 0) {
Expand Down

0 comments on commit 00a19ed

Please sign in to comment.