From f7540cd5c4047675d03b2426bb6c32d3ff811bf7 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Fri, 24 Sep 2010 22:03:24 +1000 Subject: - djm@cvs.openbsd.org 2010/09/20 04:50:53 [jpake.c schnorr.c] check that received values are smaller than the group size in the disabled and unfinished J-PAKE code. avoids catastrophic security failure found by Sebastien Martini --- schnorr.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'schnorr.c') diff --git a/schnorr.c b/schnorr.c index c17ff3241..8da2feaad 100644 --- a/schnorr.c +++ b/schnorr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: schnorr.c,v 1.3 2009/03/05 07:18:19 djm Exp $ */ +/* $OpenBSD: schnorr.c,v 1.4 2010/09/20 04:50:53 djm Exp $ */ /* * Copyright (c) 2008 Damien Miller. All rights reserved. * @@ -138,6 +138,10 @@ schnorr_sign(const BIGNUM *grp_p, const BIGNUM *grp_q, const BIGNUM *grp_g, error("%s: g_x < 1", __func__); return -1; } + if (BN_cmp(g_x, grp_p) >= 0) { + error("%s: g_x > g", __func__); + return -1; + } h = g_v = r = tmp = v = NULL; if ((bn_ctx = BN_CTX_new()) == NULL) { @@ -264,6 +268,10 @@ schnorr_verify(const BIGNUM *grp_p, const BIGNUM *grp_q, const BIGNUM *grp_g, error("%s: g_x < 1", __func__); return -1; } + if (BN_cmp(g_x, grp_p) >= 0) { + error("%s: g_x >= p", __func__); + return -1; + } h = g_xh = g_r = expected = NULL; if ((bn_ctx = BN_CTX_new()) == NULL) { -- cgit v1.2.3 From 7336b904ffab8c8b412b8ef19d7d0387a584ec58 Mon Sep 17 00:00:00 2001 From: Darren Tucker Date: Sun, 5 Dec 2010 09:00:30 +1100 Subject: - (dtucker) OpenBSD CVS Sync - djm@cvs.openbsd.org 2010/12/03 23:49:26 [schnorr.c] check that g^x^q === 1 mod p; recommended by JPAKE author Feng Hao (this code is still disabled, but apprently people are treating it as a reference implementation) --- ChangeLog | 6 ++++++ schnorr.c | 30 ++++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 6 deletions(-) (limited to 'schnorr.c') diff --git a/ChangeLog b/ChangeLog index 458f7330f..26c9b477c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,12 @@ 20101205 - (dtucker) openbsd-compat/openssl-compat.c] remove sleep leftover from debugging. Spotted by djm. + - (dtucker) OpenBSD CVS Sync + - djm@cvs.openbsd.org 2010/12/03 23:49:26 + [schnorr.c] + check that g^x^q === 1 mod p; recommended by JPAKE author Feng Hao + (this code is still disabled, but apprently people are treating it as + a reference implementation) 20101204 - (djm) [openbsd-compat/bindresvport.c] Use arc4random_uniform(range) diff --git a/schnorr.c b/schnorr.c index 8da2feaad..4d54d6881 100644 --- a/schnorr.c +++ b/schnorr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: schnorr.c,v 1.4 2010/09/20 04:50:53 djm Exp $ */ +/* $OpenBSD: schnorr.c,v 1.5 2010/12/03 23:49:26 djm Exp $ */ /* * Copyright (c) 2008 Damien Miller. All rights reserved. * @@ -258,14 +258,15 @@ schnorr_verify(const BIGNUM *grp_p, const BIGNUM *grp_q, const BIGNUM *grp_g, const BIGNUM *r, const BIGNUM *e) { int success = -1; - BIGNUM *h, *g_xh, *g_r, *expected; + BIGNUM *h = NULL, *g_xh = NULL, *g_r = NULL, *gx_q = NULL; + BIGNUM *expected = NULL; BN_CTX *bn_ctx; SCHNORR_DEBUG_BN((g_x, "%s: g_x = ", __func__)); /* Avoid degenerate cases: g^0 yields a spoofable signature */ if (BN_cmp(g_x, BN_value_one()) <= 0) { - error("%s: g_x < 1", __func__); + error("%s: g_x <= 1", __func__); return -1; } if (BN_cmp(g_x, grp_p) >= 0) { @@ -280,6 +281,7 @@ schnorr_verify(const BIGNUM *grp_p, const BIGNUM *grp_q, const BIGNUM *grp_g, } if ((g_xh = BN_new()) == NULL || (g_r = BN_new()) == NULL || + (gx_q = BN_new()) == NULL || (expected = BN_new()) == NULL) { error("%s: BN_new", __func__); goto out; @@ -288,6 +290,17 @@ schnorr_verify(const BIGNUM *grp_p, const BIGNUM *grp_q, const BIGNUM *grp_g, SCHNORR_DEBUG_BN((e, "%s: e = ", __func__)); SCHNORR_DEBUG_BN((r, "%s: r = ", __func__)); + /* gx_q = (g^x)^q must === 1 mod p */ + if (BN_mod_exp(gx_q, g_x, grp_q, grp_p, bn_ctx) == -1) { + error("%s: BN_mod_exp (g_x^q mod p)", __func__); + goto out; + } + if (BN_cmp(gx_q, BN_value_one()) != 0) { + error("%s: Invalid signature (g^x)^q != 1 mod p", __func__); + goto out; + } + + SCHNORR_DEBUG_BN((g_xh, "%s: g_xh = ", __func__)); /* h = H(g || g^v || g^x || id) */ if ((h = schnorr_hash(grp_p, grp_q, grp_g, evp_md, e, g_x, id, idlen)) == NULL) { @@ -322,9 +335,14 @@ schnorr_verify(const BIGNUM *grp_p, const BIGNUM *grp_q, const BIGNUM *grp_g, BN_CTX_free(bn_ctx); if (h != NULL) BN_clear_free(h); - BN_clear_free(g_xh); - BN_clear_free(g_r); - BN_clear_free(expected); + if (gx_q != NULL) + BN_clear_free(gx_q); + if (g_xh != NULL) + BN_clear_free(g_xh); + if (g_r != NULL) + BN_clear_free(g_r); + if (expected != NULL) + BN_clear_free(expected); return success; } -- cgit v1.2.3