[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Linphone-developers] bzrtp support for AES with 256-bit keys
From: |
Ben Sartor |
Subject: |
Re: [Linphone-developers] bzrtp support for AES with 256-bit keys |
Date: |
Tue, 03 Mar 2015 01:10:27 +0100 |
User-agent: |
KMail/4.14.1 (Linux/3.16.0-4-amd64; KDE/4.14.2; x86_64; ; ) |
Hi Johan,
thanks for reviewing and merging.
Cheers,
Ben
> 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.
>
> johan
>
> 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.
> >
> > Cheers,
> >
> > Ben
> >>
> >> 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).
> >>
> >> regards,
> >>
> >> johan
> >>
> >> 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
> >>>> needed.
> >>>
> >>> 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.
> >>>
> >>> Done.
> >>>
> >>>> - 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.
> >>>
> >>> Cheers,
> >>>
> >>> Ben
> >>>>
> >>>> - 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.
> >>>>
> >>>> regards,
> >>>>
> >>>> johan
> >>>>
> >>>> On 23/01/15 03:58, Ben Sartor wrote:
> >>>>> Hi Johan,
> >>>>>
> >>>>> thanks for you answer. I have tried to implement things like you
> >>>>> suggested.
> >>>>>
> >>>>> 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?
> >>>>>
> >>>>> [1] http://stackoverflow.com/questions/3437404/min-and-max-in-c
> >>>>>
> >>>>> The mediastreamer2 patch of my initial post still applies and may be
> >>>>> used
> >>>>> for testing.
> >>>>>
> >>>>> Kind Regards,
> >>>>>
> >>>>> Ben
> >>>>>>
> >>>>>> Hi Ben,
> >>>>>>
> >>>>>>> Yes. Just to be sure, did you mean implementing functions like this:
> >>>>>>>
> >>>>>>> void bzrtp_setSupportedCipherTypes(bzrtpContext_t *zrtpContext,
> >>>>>>> uint8_t
> >>>>>>> availableTypes[7], const uint8_t availableTypesCount);
> >>>>>>>
> >>>>>>> uint8_t bzrtp_getSupportedCipherTypes(bzrtpContext_t *zrtpContext,
> >>>>>>> uint8_t
> >>>>>>> availableTypes[7]);
> >>>>>>
> >>>>>> 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
> >>>>>>>> if
> >>>>>>>> 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
> >>>>>>> see
> >>>>>>> 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
> >>>>>> *lc);)
> >>>>>> 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
> >>>>>>>> two
> >>>>>>>> users using different set of cipher block algo)
> >>>>>>>
> >>>>>>> I'm not sure what you mean: Would you prefer a test similar to the
> >>>>>>> existing
> >>>>>>> "test_algoAgreement" or would it be better to write a test for the
> >>>>>>> function
> >>>>>>> "selectCommonAlgo" directly?
> >>>>>>
> >>>>>> I was thinking of extending the test_algoAgreement to include block
> >>>>>> cipher selection and some test on linphone call to check correct
> >>>>>> reading
> >>>>>> of the configuration from file, but it can't harm to have a test for
> >>>>>> the
> >>>>>> selectCommonAlgo too.
> >>>>>>
> >>>>>> Thanks for your contribution.
> >>>>>>
> >>>>>> Have a good day
> >>>>>>
> >>>>>> johan
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> Linphone-developers mailing list
> >>>>>> address@hidden
> >>>>>> https://lists.nongnu.org/mailman/listinfo/linphone-developers
> >>>>>
> >>>>> _______________________________________________
> >>>>> Linphone-developers mailing list
> >>>>> address@hidden
> >>>>> https://lists.nongnu.org/mailman/listinfo/linphone-developers
> >>>>
> >>>> _______________________________________________
> >>>> Linphone-developers mailing list
> >>>> address@hidden
> >>>> https://lists.nongnu.org/mailman/listinfo/linphone-developers
> >>>
> >>> _______________________________________________
> >>> Linphone-developers mailing list
> >>> address@hidden
> >>> https://lists.nongnu.org/mailman/listinfo/linphone-developers
> >>
> >> _______________________________________________
> >> Linphone-developers mailing list
> >> address@hidden
> >> https://lists.nongnu.org/mailman/listinfo/linphone-developers
> >
> > _______________________________________________
> > Linphone-developers mailing list
> > address@hidden
> > https://lists.nongnu.org/mailman/listinfo/linphone-developers
>
> _______________________________________________
> Linphone-developers mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/linphone-developers
--
———————————————————
https://www.simlar.org
free and secure calls
fon: +49-(0)221-999 999 30
fax: +49-(0)221-999 999 31
mail: address@hidden
github: https://github.com/simlar/
———————————————————