[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Linphone-developers] bzrtp support for AES with 256-bit keys

From: Johan Pascal
Subject: Re: [Linphone-developers] bzrtp support for AES with 256-bit keys
Date: Tue, 03 Mar 2015 00:15:14 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

Hi Ben,
I eventually pushed your patches to linphone and mediastreamer2 but I left the default to AES128.

Make sure you fetch bzrtp too as I fixed few bugs.


On 12/02/15 06:14, Ben Sartor wrote:
Hi Johan,

thanks for merging.

Here are some more patches which should implement your suggestion. The first
ones apply to mediastreamer2 and the second ones to linphone.


Hi Ben,
thanks. It seems all fine. I'll push it soon.

I won't be able to work on liblinphone/mediastreamer2 before mid-february.
The idea would be to use the already existing SRTP setting in the
.linphonerc config file to set the cipher block algo preference and add
a field in this file to select key agreement algo(to be able to disable
non mandatory DH2k).



On 27/01/15 05:30, Ben Sartor wrote:
Hi Johan,

thanks for reviewing.

Hi Ben,
and thanks for the patch.

I had a quick look at it, very nice rework on the tests cases!

Few remarks:
- I think we should stick to AES128 being the default (switch in
bzrtpCrypto_getAvailableCryptoTypes). The user will set it to AES256 if

Agreed, especially as it is possible to update bzrtp without updating
mediastream2 this way.  Updated patches are attached.

   By the way, this is about bzrtp and (lib)linphone may still choose
   AES256 as>
default in the future.

- You may want to export also the algorithm defines and not only the
types in bzrtp.h (commit 4) otherwise the user won't be able to safely
use the get/set functions.


- We shall ensure that mandatory algorithms are always present in the
context supported list, by either checking their presence in the input
supportedTypes of bzrtp_setSupportedCryptoTypes and queing them at the
end if they are missing or tweaking the selectCommonAlgo to add them if
they are missing.(first one seems cleaner)

- Same kind of problem (which was already there before your patches)
with the incoming HelloPacket parsing: we shall ensure that if the
packet doesn't contain a mandatory algo, it is added at the end of the
list as specified in the RFC section 5.2:
"If a mandatory algorithm is not included in the list, it

      is implicitly added to the end of the list for preference."

Which means the in function bzrtp_packetParser, all the tests on
messageData->xx == 0 to set the mandatory algo shall be replaced by
something checking the presence of the mandatory algo in the
messageData->xx field after parsing it from the packet.

Patches implementing this are attached.



- min function seems fine to me as it is.

I'll have to rework the mediastream patch as I've done some
modifications in zrtp.c, they're in branch dev_dtls for now but will be
merge to master soon.



On 23/01/15 03:58, Ben Sartor wrote:
Hi Johan,

thanks for you answer. I have tried to implement things like you

Patches 0001 and 0002 are unchanged.

Patches 0003 to 0005 add the the getter and setter you suggested.

Patches 0006 to 0009 implement the tests. I completely refactored the
function "test_algoAgreement". Is it ok for you to write tests this way?

Patches 0010 to 0012 are some code cleanup suggestions and were not
discussed before. Let me know what you think.
Is there a better place for the min-function? Maybe even a macro like
discussed here [1]? Is "__typeof__" ok?


The mediastreamer2 patch of my initial post still applies and may be
for testing.

Kind Regards,


Hi Ben,

Yes. Just to be sure, did you mean implementing functions like this:

void bzrtp_setSupportedCipherTypes(bzrtpContext_t *zrtpContext,
availableTypes[7], const uint8_t availableTypesCount);

uint8_t bzrtp_getSupportedCipherTypes(bzrtpContext_t *zrtpContext,

Yes but you want to add an uint8_t algoType argument(just like
bzrtpCrypto_getAvailableCryptoTypes function) to both of them in order
to use the same function to get/set the available algos for block
cipher/key exchange/SAS rendering/Hash.

This means we also must add a way to store the user configuration in
linphone. I was thinking the easiest way would be to store it in the
config file and access it only manually for now. I can implement this
you're lost on the way linphone manage the config file.

I haven't had a look to  linphone config file management, yet. Let's
how far I get or if you find time first.

It's quite simple, but if you struggle tell me and I'll have a look at
it when you're done with bzrtp. We can use an already existing config
setting used to select SRTP protection profile(see misc.c const
MSCryptoSuite * linphone_core_get_srtp_crypto_suites(LinphoneCore
for the block cipher algo selection and do something for the other algo
types when needed.

Last, this must be covered by automatic tests.(Key exchange between
users using different set of cipher block algo)

I'm not sure what you mean: Would you prefer a test similar to the
"test_algoAgreement" or would it be better to write a test for the
"selectCommonAlgo" directly?

I was thinking of extending the test_algoAgreement to include block
cipher selection and some test on linphone call to check correct
of the configuration from file, but it can't harm to have a test for
selectCommonAlgo too.

Thanks for your contribution.

Have a good day


Linphone-developers mailing list

Linphone-developers mailing list

Linphone-developers mailing list

Linphone-developers mailing list

Linphone-developers mailing list

Linphone-developers mailing list

reply via email to

[Prev in Thread] Current Thread [Next in Thread]