[PATCH] ctrl: Extend ctrl command to optionally handle alg+ki
holger at freyther.de
Thu Apr 7 07:28:08 UTC 2016
> On 07 Apr 2016, at 02:52, Neels Hofmeyr <nhofmeyr at sysmocom.de> wrote:
> You didn't bump to v2 because it is supposedly backwards compatible to
> subscriber-modify-v1 without auth info, right?
right. The v1 testcase is still passing and the customer using this interface will not notice any change.
> 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
IMSI has changed == new subscriber. Also for a CTRL interface only case we could not have set the keys before, so clearing the auth tuples has no effect. But anyway, I have sent a v2 that moves this handling into an if so there is no semantic change for the current usage of the interface.
> Maybe it's better / less complex to have a separate command for auth or bump to
I didn't want to open the can of deprecation and removing old commands yet. The v1 behavior is tested (the test should actually check the DB for the state or such but that is a different topic). I found it easier to add to the existing command and based on the ctrl test I am confident to not break the users of that interface.
>> + 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"?\
The CTRL command has a READ, VERIFY and WRITE method. VERIFY will be executed before WRITE. The VERIFY method can only return yes or no. We can add improving/extending the VERIFY to our backlog.
> 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"'.
fixed locally but that is not in the v2 patch
More information about the OpenBSC