use osmocom auth API instead of direct calls
diff mbox

Message ID 1401898914-23556-1-git-send-email-Max.Suraev@fairwaves.co
State Superseded
Headers show

Commit Message

Max June 4, 2014, 4:21 p.m. UTC
---
 openbsc/src/libmsc/auth.c | 68 +++++++++++++++++++----------------------------
 1 file changed, 28 insertions(+), 40 deletions(-)

Comments

Max June 6, 2014, 1:36 p.m. UTC | #1
This patch switches away from directly calling COMP128 to using generic osmocom
authentication API. The idea is that bypassing auth API will be deprecated in future
versions of libosmocore. Using generic API will allow us to trivially introduce
support for other algorithms like COMP128v23 for example.
Holger Freyther July 22, 2014, 10:01 a.m. UTC | #2
On Wed, Jun 04, 2014 at 06:21:54PM +0200, Max Suraev wrote:

Hi!

sorry for the late reply (and I also intend to reply for reviving
our wednesday meetings. I could talk about TCAP/MAP..)


> ---
>  openbsc/src/libmsc/auth.c | 68 +++++++++++++++++++----------------------------
>  1 file changed, 28 insertions(+), 40 deletions(-)
> 
> diff --git a/openbsc/src/libmsc/auth.c b/openbsc/src/libmsc/auth.c
> index 10d8edf..ab84975 100644
> --- a/openbsc/src/libmsc/auth.c
> +++ b/openbsc/src/libmsc/auth.c
> @@ -25,46 +25,11 @@
>  #include <openbsc/auth.h>
>  #include <openbsc/gsm_data.h>
>  
> -#include <osmocom/gsm/comp128.h>
> +#include <osmocom/crypt/auth.h>
>  
>  #include <stdlib.h>
>  
>  
> -static int
> -_use_xor(struct gsm_auth_info *ainfo, struct gsm_auth_tuple *atuple)
> -{
> -
> -	for (i=0; i<4; i++)
> -		atuple->sres[i] = atuple->rand[i] ^ ainfo->a3a8_ki[i];
> -	for (i=4; i<12; i++)
> -		atuple->kc[i-4] = atuple->rand[i] ^ ainfo->a3a8_ki[i];
> -
> -	return 0;
> -}
> -
> -static int
> -_use_comp128_v1(struct gsm_auth_info *ainfo, struct gsm_auth_tuple *atuple)
> -{
> -	if (ainfo->a3a8_ki_len != A38_COMP128_KEY_LEN) {
> -		LOGP(DMM, LOGL_ERROR, "Invalid COMP128v1 key (len=%d) %s\n",
> -			ainfo->a3a8_ki_len,
> -			osmo_hexdump(ainfo->a3a8_ki, ainfo->a3a8_ki_len));
> -		return -1;
> -	}
> -
> -	comp128(ainfo->a3a8_ki, atuple->rand, atuple->sres, atuple->kc);
> -
> -	return 0;
> -}
> -
>  /* Return values 
>   *  -1 -> Internal error
>   *   0 -> Not available
> @@ -76,6 +41,11 @@ int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple,
>  {
>  	struct gsm_auth_info ainfo;
>  	int i, rc;
> +	static struct osmo_sub_auth_data auth = {
> +	    .type = OSMO_AUTH_TYPE_GSM
> +	};

Why is that static?


>  
> +	memcpy(auth.u.gsm.ki, ainfo.a3a8_ki, sizeof(auth.u.gsm.ki));
> +
> +	if (osmo_auth_gen_vec(vec, &auth, atuple->rand) < 0)
> +		return -1;
> +
> +	memcpy(atuple->sres, vec->sres, 4);
> +	memcpy(atuple->kc, vec->kc, 8);

in terms of API, how hard would be an in-situ operation?
Max July 22, 2014, 12:48 p.m. UTC | #3
Curiously I do not see this patch in patchwork anymore. Comments are inline.

22.07.2014 12:01, Holger Hans Peter Freyther пишет:
> On Wed, Jun 04, 2014 at 06:21:54PM +0200, Max Suraev wrote:
> 
> Hi!
> 
> sorry for the late reply (and I also intend to reply for reviving
> our wednesday meetings. I could talk about TCAP/MAP..)
> 

Neat.


>> @@ -76,6 +41,11 @@ int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple,
>>  {
>>  	struct gsm_auth_info ainfo;
>>  	int i, rc;
>> +	static struct osmo_sub_auth_data auth = {
>> +	    .type = OSMO_AUTH_TYPE_GSM
>> +	};
> 
> Why is that static?
> 

Probably crawled from static functions I've replaced :)
It's not really needed here.

>>  
>> +	memcpy(auth.u.gsm.ki, ainfo.a3a8_ki, sizeof(auth.u.gsm.ki));
>> +
>> +	if (osmo_auth_gen_vec(vec, &auth, atuple->rand) < 0)
>> +		return -1;
>> +
>> +	memcpy(atuple->sres, vec->sres, 4);
>> +	memcpy(atuple->kc, vec->kc, 8);
> 
> in terms of API, how hard would be an in-situ operation?
> 

Ideally we should use osmocom's auth_vec directly instead of our own atuple. But this
change would be more intrusive and I recall Sylvain been working on something like
that so I've decided to go for smaller patch which does the job of using proper API.

Patch
diff mbox

diff --git a/openbsc/src/libmsc/auth.c b/openbsc/src/libmsc/auth.c
index 10d8edf..ab84975 100644
--- a/openbsc/src/libmsc/auth.c
+++ b/openbsc/src/libmsc/auth.c
@@ -25,46 +25,11 @@ 
 #include <openbsc/auth.h>
 #include <openbsc/gsm_data.h>
 
-#include <osmocom/gsm/comp128.h>
+#include <osmocom/crypt/auth.h>
 
 #include <stdlib.h>
 
 
-static int
-_use_xor(struct gsm_auth_info *ainfo, struct gsm_auth_tuple *atuple)
-{
-	int i, l = ainfo->a3a8_ki_len;
-
-	if ((l > A38_XOR_MAX_KEY_LEN) || (l < A38_XOR_MIN_KEY_LEN)) {
-		LOGP(DMM, LOGL_ERROR, "Invalid XOR key (len=%d) %s\n",
-			ainfo->a3a8_ki_len,
-			osmo_hexdump(ainfo->a3a8_ki, ainfo->a3a8_ki_len));
-		return -1;
-	}
-
-	for (i=0; i<4; i++)
-		atuple->sres[i] = atuple->rand[i] ^ ainfo->a3a8_ki[i];
-	for (i=4; i<12; i++)
-		atuple->kc[i-4] = atuple->rand[i] ^ ainfo->a3a8_ki[i];
-
-	return 0;
-}
-
-static int
-_use_comp128_v1(struct gsm_auth_info *ainfo, struct gsm_auth_tuple *atuple)
-{
-	if (ainfo->a3a8_ki_len != A38_COMP128_KEY_LEN) {
-		LOGP(DMM, LOGL_ERROR, "Invalid COMP128v1 key (len=%d) %s\n",
-			ainfo->a3a8_ki_len,
-			osmo_hexdump(ainfo->a3a8_ki, ainfo->a3a8_ki_len));
-		return -1;
-	}
-
-	comp128(ainfo->a3a8_ki, atuple->rand, atuple->sres, atuple->kc);
-
-	return 0;
-}
-
 /* Return values 
  *  -1 -> Internal error
  *   0 -> Not available
@@ -76,6 +41,11 @@  int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple,
 {
 	struct gsm_auth_info ainfo;
 	int i, rc;
+	static struct osmo_sub_auth_data auth = {
+	    .type = OSMO_AUTH_TYPE_GSM
+	};
+	struct osmo_auth_vector _vec;
+	struct osmo_auth_vector *vec = &_vec;
 
 	/* Get subscriber info (if any) */
 	rc = db_get_authinfo_for_subscr(&ainfo, subscr);
@@ -109,13 +79,23 @@  int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple,
 		return 0;
 
 	case AUTH_ALGO_XOR:
-		if (_use_xor(&ainfo, atuple))
-			return 0;
+		auth.algo = OSMO_AUTH_ALG_XOR;
+		if ((ainfo.a3a8_ki_len > A38_XOR_MAX_KEY_LEN) || (ainfo.a3a8_ki_len < A38_XOR_MIN_KEY_LEN)) {
+			LOGP(DMM, LOGL_ERROR, "Invalid XOR key (len=%d) %s\n",
+			     ainfo.a3a8_ki_len,
+			     osmo_hexdump(ainfo.a3a8_ki, ainfo.a3a8_ki_len));
+			return -1;
+		}
 		break;
 
 	case AUTH_ALGO_COMP128v1:
-		if (_use_comp128_v1(&ainfo, atuple))
-			return 0;
+		auth.algo = OSMO_AUTH_ALG_COMP128v1;
+		if (ainfo.a3a8_ki_len != A38_COMP128_KEY_LEN) {
+			LOGP(DMM, LOGL_ERROR, "Invalid COMP128v1 key (len=%d) %s\n",
+			     ainfo.a3a8_ki_len,
+			     osmo_hexdump(ainfo.a3a8_ki, ainfo.a3a8_ki_len));
+			return -1;
+		}
 		break;
 
 	default:
@@ -124,6 +104,14 @@  int auth_get_tuple_for_subscr(struct gsm_auth_tuple *atuple,
 		return 0;
 	}
 
+	memcpy(auth.u.gsm.ki, ainfo.a3a8_ki, sizeof(auth.u.gsm.ki));
+
+	if (osmo_auth_gen_vec(vec, &auth, atuple->rand) < 0)
+		return -1;
+
+	memcpy(atuple->sres, vec->sres, 4);
+	memcpy(atuple->kc, vec->kc, 8);
+
         db_sync_lastauthtuple_for_subscr(atuple, subscr);
 
 	DEBUGP(DMM, "Need to do authentication and ciphering\n");