[PATCH] ctrl: Extend ctrl command to optionally handle alg+ki

[Neels Hofmeyr]

On Wed, Apr 06, 2016 at 10:44:52PM +0200, Holger Hans Peter Freyther wrote:
> +
> +	/* handle optional ciphering */
> +	if (!alg || strcasecmp(alg, "none") == 0)
> +		db_sync_authinfo_for_subscr(NULL, subscr);
> +	else {
> +		struct gsm_auth_info ainfo = { 0, };
> +		/* the verify should make sure that this is okay */
> +		OSMO_ASSERT(alg);
> +		OSMO_ASSERT(ki);
> +
> +		if (strcasecmp(alg, "xor") == 0)
> +			ainfo.auth_algo = AUTH_ALGO_XOR;
> +		else if (strcasecmp(alg, "comp128v1") == 0)
> +			ainfo.auth_algo = AUTH_ALGO_COMP128v1;
> +
> +		rc = osmo_hexparse(ki, ainfo.a3a8_ki, sizeof(ainfo.a3a8_ki));
> +		if (rc < 0) {
> +			subscr_put(subscr);
> +			talloc_free(tmp);
> +			cmd->reply = "Failed to parse KI";
> +			return CTRL_CMD_ERROR;
> +		}
> +
> +		ainfo.a3a8_ki_len = rc;
> +		db_sync_authinfo_for_subscr(&ainfo, subscr);
> +		rc = 0;
> +	}
> +
> +	db_sync_lastauthtuple_for_subscr(NULL, subscr);

You didn't bump to v2 because it is supposedly backwards compatible to
subscriber-modify-v1 without auth info, right?

But consider that a subscriber-modify-v1 will now clear out the auth info when
the newly added parameters are omitted. So it's not strictly backwards
compatible, right? With the previous v1 I could only modify IMSI and MSISDN and
leave algo and KI untouched. After this patch I will always either have to pass
algo+KI again or they will be cleared out.

Also, this will always clear out the lastauthtuple, regardless of the auth info
being changed or not. Even if I again pass the same algo+KI as were stored
previously. OTOH clearing might be good when the IMSI has changed?? :P

Maybe it's better / less complex to have a separate command for auth or bump to
v2...


> +        r = self.do_set('subscriber-modify-v1', '2620345,445566,xor')
> +        self.assertEquals(r['mtype'], 'ERROR')
> +        self.assertEquals(r['error'], 'Value failed verification.')

I'm not familiar with the ctrl interface, but couldn't the error message be
more descriptive, like "missing KI argument"?


log msg:
> The algorithm and ki are considered optional but if a valid
> and non none algorithm is passed, a KI must be passed as well.

rather say 'not-none' or 'a valid algorithm other than "none"'.

~Neels

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.osmocom.org/pipermail/openbsc/attachments/20160407/5f248df2/attachment.bin>