From: Daniel Willmann dwillmann@sysmocom.de
The following patches uses RAND_bytes() from openssl instead of rand() to generate TMSI/P-TMSI and auth tuples.
I didn't change the use of rand_r() in gb-proxy because the gbproxy test seems to depend on predictable randomness.
Daniel Willmann (4): libmsc: Use RAND_bytes when choosing a tmsi gprs: Use RAND_bytes for p-tmsi libmsc: Use RAND_bytes to choose auth tuple libmsc: Use RAND_bytes to generate a token
openbsc/configure.ac | 2 +- openbsc/src/gprs/gprs_sgsn.c | 4 +++- openbsc/src/libmsc/Makefile.am | 2 +- openbsc/src/libmsc/auth.c | 9 +++++++-- openbsc/src/libmsc/db.c | 12 ++++++++++-- openbsc/src/osmo-nitb/Makefile.am | 2 +- openbsc/tests/channel/Makefile.am | 2 +- openbsc/tests/db/Makefile.am | 2 +- 8 files changed, 25 insertions(+), 10 deletions(-)
From: Daniel Willmann dwillmann@sysmocom.de
Require openssl version to be >= 0.9.5 because we rely on the RAND_bytes return value. --- openbsc/configure.ac | 2 +- openbsc/src/libmsc/Makefile.am | 2 +- openbsc/src/libmsc/db.c | 7 ++++++- openbsc/src/osmo-nitb/Makefile.am | 2 +- openbsc/tests/channel/Makefile.am | 2 +- openbsc/tests/db/Makefile.am | 2 +- 6 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/openbsc/configure.ac b/openbsc/configure.ac index 78302dd..fc30b5e 100644 --- a/openbsc/configure.ac +++ b/openbsc/configure.ac @@ -27,13 +27,13 @@ PKG_CHECK_MODULES(LIBOSMOGSM, libosmogsm >= 0.7.0) PKG_CHECK_MODULES(LIBOSMOABIS, libosmoabis >= 0.2.0) PKG_CHECK_MODULES(LIBOSMOGB, libosmogb >= 0.6.4) PKG_CHECK_MODULES(LIBOSMONETIF, libosmo-netif >= 0.0.1) +PKG_CHECK_MODULES(LIBCRYPTO, libcrypto >= 0.9.5)
# Enabke/disable the NAT? AC_ARG_ENABLE([nat], [AS_HELP_STRING([--enable-nat], [Build the BSC NAT. Requires SCCP])], [osmo_ac_build_nat="$enableval"],[osmo_ac_build_nat="no"]) if test "$osmo_ac_build_nat" = "yes" ; then PKG_CHECK_MODULES(LIBOSMOSCCP, libosmo-sccp >= 0.0.2) - PKG_CHECK_MODULES(LIBCRYPTO, libcrypto) fi AM_CONDITIONAL(BUILD_NAT, test "x$osmo_ac_build_nat" = "xyes") AC_SUBST(osmo_ac_build_nat) diff --git a/openbsc/src/libmsc/Makefile.am b/openbsc/src/libmsc/Makefile.am index aa7d8ae..18bfa0c 100644 --- a/openbsc/src/libmsc/Makefile.am +++ b/openbsc/src/libmsc/Makefile.am @@ -1,6 +1,6 @@ AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include -I$(top_builddir) AM_CFLAGS=-Wall $(LIBOSMOCORE_CFLAGS) $(LIBOSMOVTY_CFLAGS) \ - $(LIBOSMOABIS_CFLAGS) $(COVERAGE_CFLAGS) + $(LIBOSMOABIS_CFLAGS) $(COVERAGE_CFLAGS) $(LIBCRYPTO_CFLAGS)
noinst_HEADERS = meas_feed.h
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index 035202d..faae982 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -38,6 +38,8 @@ #include <osmocom/core/statistics.h> #include <osmocom/core/rate_ctr.h>
+#include <openssl/rand.h> + /* Semi-Private-Interface (SPI) for the subscriber code */ void subscr_direct_free(struct gsm_subscriber *subscr);
@@ -1194,7 +1196,10 @@ int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber) char *tmsi_quoted;
for (;;) { - subscriber->tmsi = rand(); + if (RAND_bytes(&subscriber->tmsi, sizeof(subscriber->tmsi)) != 1) { + LOGP(DDB, LOGL_ERROR, "RAND_bytes failed\n"); + return 1; + } if (subscriber->tmsi == GSM_RESERVED_TMSI) continue;
diff --git a/openbsc/src/osmo-nitb/Makefile.am b/openbsc/src/osmo-nitb/Makefile.am index 57a9284..d3b97f8 100644 --- a/openbsc/src/osmo-nitb/Makefile.am +++ b/openbsc/src/osmo-nitb/Makefile.am @@ -16,4 +16,4 @@ osmo_nitb_LDADD = \ $(top_builddir)/src/libcommon/libcommon.a \ -ldbi $(LIBCRYPT) \ $(LIBOSMOGSM_LIBS) $(LIBOSMOVTY_LIBS) $(LIBOSMOCORE_LIBS) \ - $(LIBOSMOCTRL_LIBS) $(LIBOSMOABIS_LIBS) $(LIBSMPP34_LIBS) + $(LIBOSMOCTRL_LIBS) $(LIBOSMOABIS_LIBS) $(LIBSMPP34_LIBS) $(LIBCRYPTO_LIBS) diff --git a/openbsc/tests/channel/Makefile.am b/openbsc/tests/channel/Makefile.am index 519efbd..51b2f83 100644 --- a/openbsc/tests/channel/Makefile.am +++ b/openbsc/tests/channel/Makefile.am @@ -11,4 +11,4 @@ channel_test_LDADD = \ $(top_builddir)/src/libmsc/libmsc.a \ $(top_builddir)/src/libcommon/libcommon.a \ $(LIBOSMOCORE_LIBS) \ - -ldbi $(LIBOSMOGSM_LIBS) + -ldbi $(LIBOSMOGSM_LIBS) $(LIBCRYPTO_LIBS) diff --git a/openbsc/tests/db/Makefile.am b/openbsc/tests/db/Makefile.am index 647b519..be3af5f 100644 --- a/openbsc/tests/db/Makefile.am +++ b/openbsc/tests/db/Makefile.am @@ -13,5 +13,5 @@ db_test_LDADD = $(top_builddir)/src/libbsc/libbsc.a \ $(top_builddir)/src/libtrau/libtrau.a \ $(top_builddir)/src/libcommon/libcommon.a \ $(LIBOSMOCORE_LIBS) $(LIBOSMOABIS_LIBS) \ - $(LIBOSMOGSM_LIBS) $(LIBSMPP34_LIBS) $(LIBOSMOVTY_LIBS) -ldbi + $(LIBOSMOGSM_LIBS) $(LIBSMPP34_LIBS) $(LIBOSMOVTY_LIBS) $(LIBCRYPTO_LIBS) -ldbi
On 08 Oct 2015, at 16:10, Daniel Willmann dwilllmann@sysmocom.de wrote:
for (;;) {
subscriber->tmsi = rand();
if (RAND_bytes(&subscriber->tmsi, sizeof(subscriber->tmsi)) != 1) {LOGP(DDB, LOGL_ERROR, "RAND_bytes failed\n");
db.c:1199:18: warning: passing argument 1 of ‘RAND_bytes’ from incompatible pointer type if (RAND_bytes(&subscriber->tmsi, sizeof(subscriber->tmsi)) != 1) {
:}
From: Daniel Willmann dwillmann@sysmocom.de
--- openbsc/src/gprs/gprs_sgsn.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/openbsc/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c index f40de0b..2af1e97 100644 --- a/openbsc/src/gprs/gprs_sgsn.c +++ b/openbsc/src/gprs/gprs_sgsn.c @@ -525,7 +525,9 @@ uint32_t sgsn_alloc_ptmsi(void) int max_retries = 100;
restart: - ptmsi = rand(); + if (RAND_bytes(&ptmsi, sizeof(ptmsi)) != 1) + goto failed; + /* Enforce that the 2 MSB are set without loosing the distance between * identical values. Since rand() has no duplicate values within a * period (because the size of the state is the same like the size of
On 08 Oct 2015, at 16:10, Daniel Willmann dwilllmann@sysmocom.de wrote:
From: Daniel Willmann dwillmann@sysmocom.de
* missing include for the RAND_bytes prototype * missing cat of ptmsi * missing linking to LIBCRYPTO_LIBS * missing linking to LIBCRYPTO_LIBS in the test
and after that.. the sgsn tests fail because of the TMSI situation. I have now reverted the code. Please look at it with Jacob and extract the TMSI from the SGSN state instead of resetting the RNG in the test.
holger
On Mon, 2015-10-12 at 09:59 +0200, Holger Freyther wrote:
On 08 Oct 2015, at 16:10, Daniel Willmann dwilllmann@sysmocom.de wrote:
From: Daniel Willmann dwillmann@sysmocom.de
- missing include for the RAND_bytes prototype
- missing cat of ptmsi
- missing linking to LIBCRYPTO_LIBS
- missing linking to LIBCRYPTO_LIBS in the test
and after that.. the sgsn tests fail because of the TMSI situation. I have now reverted the code. Please look at it with Jacob and extract the TMSI from the SGSN state instead of resetting the RNG in the test.
...sorry, it seems that it never even executed the test on my new system due to missing libgtp.
I'll send another patchset which solves this issue (and does the same for gbproxy).
Daniel
From: Daniel Willmann dwillmann@sysmocom.de
--- openbsc/src/libmsc/auth.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/openbsc/src/libmsc/auth.c b/openbsc/src/libmsc/auth.c index 10d8edf..93ee71f 100644 --- a/openbsc/src/libmsc/auth.c +++ b/openbsc/src/libmsc/auth.c @@ -27,6 +27,8 @@
#include <osmocom/gsm/comp128.h>
+#include <openssl/rand.h> + #include <stdlib.h>
@@ -100,8 +102,11 @@ int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple, /* Generate a new one */ atuple->use_count = 1; atuple->key_seq = (atuple->key_seq + 1) % 7; - for (i=0; i<sizeof(atuple->rand); i++) - atuple->rand[i] = random() & 0xff; + + if (RAND_bytes(atuple->rand, sizeof(atuple->rand)) != 1) { + LOGP(DMM, LOGL_NOTICE, "RAND_bytes failed, can't generate new auth tuple\n"); + return -1; + }
switch (ainfo.auth_algo) { case AUTH_ALGO_NONE:
From: Daniel Willmann dwillmann@sysmocom.de
--- openbsc/src/libmsc/db.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c index faae982..863a092 100644 --- a/openbsc/src/libmsc/db.c +++ b/openbsc/src/libmsc/db.c @@ -1275,7 +1275,10 @@ int db_subscriber_alloc_token(struct gsm_subscriber *subscriber, uint32_t *token uint32_t try;
for (;;) { - try = rand(); + if (RAND_bytes(&try, sizeof(try)) != 1) { + LOGP(DDB, LOGL_ERROR, "RAND_bytes failed\n"); + return 1; + } if (!try) /* 0 is an invalid token */ continue; result = dbi_conn_queryf(conn,
-
Daniel Willmann -
Daniel Willmann -
Holger Freyther