[hipl-dev] Re: [Merge] lp:~stefan.goetz/hipl/hidb-db into lp:hipl
- From: Stefan Götz <stefan.goetz@xxxxxxxxxxxxxxxxx>
- To: mp+61832@xxxxxxxxxxxxxxxxxx
- Date: Mon, 06 Jun 2011 11:01:48 -0000
Hi René!
Thanks for reviewing!
>> - if (hip_get_host_id_and_priv_key(HIP_DB_LOCAL_HID, hit_our, HIP_HI_RSA,
>> + if (hip_get_host_id_and_priv_key(hit_our, HIP_HI_RSA,
>> &entry->our_pub, &entry->our_priv_key))
>> {
>> - HIP_IFEL(hip_get_host_id_and_priv_key(HIP_DB_LOCAL_HID, hit_our,
>> - HIP_HI_DSA, &entry->our_pub,
>> &entry->our_priv_key),
>> + HIP_IFEL(hip_get_host_id_and_priv_key(hit_our,
>> + HIP_HI_DSA,
>> + &entry->our_pub,
>> + &entry->our_priv_key),
>> -1, "Local host identity not found\n");
>> }
>
> Would it be possible to further abstract from the HI algorithm here? Right
> now, we need to look for an RSA key first and then fall back to DSA. What
> happens when we add ECDSA? We will need to add that case to many if not all
> calls to hip_get_host_id_and_priv_key().
Further abstraction is certainly possible but right now not the focus
of this branch. I have a keys branch for toying around with an
algorithm-agnostic interface to crypto functionality but that is at an
infant's stage. Expect it to be ready anytime around 2020 :)
>> === modified file 'hipd/hidb.c'
> [...]
>> - id = hip_get_hostid_entry_by_lhi_and_algo(db, &hit, HIP_ANY_ALGO, -1);
>> + id = hip_get_hostid_entry_by_lhi_and_algo(&hit, HIP_ANY_ALGO, -1);
>
> Is there any call to hip_get_hostid_entry_by_lhi_and_algo() with a parameter
> different from HIP_ANY_ALGO? Furthermore, is there any call with a parameter
> different from (anon=) -1? If not, these point to another possibility of
> cleaning up the API.
We cannot get rid of this function altogether but similar cleanup will
be part of future merge proposals on the HIDB API.
>> void hip_uninit_host_id_dbs(void);
>
> This function seems to introduce unnecessary call indirection seeing that
> hip_uninit_hostid_db() does exactly the same.
You are correct in terms of functionality but these are two different
interfaces, one for the HIDB, one for the LSIDB, and they should not
be mixed. A user of the HIDB (who might want to uninitialize it at
some point via this function) should not be exposed to the LSIDB API.
Performance is obviously not an issue here.
> Otherwise, this merge proposal seems good to me. However, you made changes to
> core HI handling components of HIPL, so please test your changes extensively.
> I suggest running tests in your own VMs including BEX and UPDATE exchanges,
> running Valgrind and making a test deployment in our testbed before
> committing these changes to trunk.
Will do that once the regression test framework is in place and the
current bugs are fixed. That might be a while.
Stefan
--
https://code.launchpad.net/~stefan.goetz/hipl/hidb-db/+merge/61832
Your team HIPL core team is subscribed to branch lp:hipl.
Other related posts:
