[jitsi-dev] Re: [jitsi~svn:9769] Adds SDES for XMPP. Works with Jingle and GTalk (only for gmail web app,

Ingo_Bauersachs · August 2, 2012, 6:59pm

Hey Vincent

Subject: [jitsi~svn:9769] Adds SDES for XMPP. Works with Jingle and GTalk
(only for gmail web app,

Project: jitsi
Repository: svn
Revision: 9769
Author: vincent_lucas
Date: 2012-08-02 08:53:41 UTC
Link:

Log Message:
------------
Adds SDES for XMPP. Works with Jingle and GTalk (only for gmail web app,
doesn not work on "talk" for android).

Great to see the SDES support expanded to Jingle!
I have some comments/questions on your patch:

- CryptoPacketExtension:
Would it help to have some of the code from this class in the SDES-Library? E.g. to avoid code duplication as the initialize and toSrtpCryptoAttribute methods particularly look like something that assembles information into/from strings which are defined by RFC4568.

- Order of added Crypto Controls in SIP
I can't currently test your modifications, but from memory and looking at CallPeerMediaHandlerSipImpl (@@ -586,8 +588,10 @@) it seems that ZRTP now has preference over SDES. The order of updating the media description matters. The new order causes that SDES is now only enabled if the default encryption is disabled or if the extreme rare case of adding the zrtp-hash to the SDP fails.

This is only from looking at the commit, so I'm sorry if I came up with wrong conclusions.

Regards,
Ingo

Vincent_Lucas · August 3, 2012, 8:36am

Hi Ingo,

Hey Vincent

Subject: [jitsi~svn:9769] Adds SDES for XMPP. Works with Jingle and GTalk
(only for gmail web app,

Project: jitsi
Repository: svn
Revision: 9769
Author: vincent_lucas
Date: 2012-08-02 08:53:41 UTC
Link:

Log Message:
------------
Adds SDES for XMPP. Works with Jingle and GTalk (only for gmail web app,
doesn not work on "talk" for android).

Great to see the SDES support expanded to Jingle!
I have some comments/questions on your patch:

- CryptoPacketExtension:
Would it help to have some of the code from this class in the SDES-Library? E.g. to avoid code duplication as the initialize and toSrtpCryptoAttribute methods particularly look like something that assembles information into/from strings which are defined by RFC4568.

Indeed, it would help. For example, it would be interesting to add to the SrtpCryptoAttribute at least the 4 following functions:

- public String getKeyParamsString()
// returns a string representation of the list of key params separated // by ";".

- public String getSessionParamsString()
// returns a string representation of the list of session params
// separated by " ".

- public void setKeyParamsString(String keyParamsString)
// Parses the keyParamsString using the ";" separator in order to get
// a SrtpKeyParam[] list and store it as the keyParams attribute.

- public void setSessionParamsString(String sessionParamsString)
// Parses the sessionParamsString using the " " separator in order to
// get a SrtpSessionParam[] list and store it as the sessionParams
// attribute.

For the SDes library, these functions can be used inside
"CryptoAttribute.create" and "CryptoAttribute.encode"

- Order of added Crypto Controls in SIP
I can't currently test your modifications, but from memory and looking at CallPeerMediaHandlerSipImpl (@@ -586,8 +588,10 @@) it seems that ZRTP now has preference over SDES. The order of updating the media description matters. The new order causes that SDES is now only enabled if the default encryption is disabled or if the extreme rare case of adding the zrtp-hash to the SDP fails.

True, if both ZRTP and SDes are available, then ZRTP is chosen by default. SDes is only used if at least one peer has disabled the "Indicate support of ZRTP in signaling protocol".

I do not have enough knowledge to judge which encryption is more "robust" than the other. But, the only point I understand is that SDes security is pointless if the signaling protocol is not encrypted (which the situation with most of my SIP providers). If SDes (for any good reason) is preferable than ZRTP, maybe we can discuss of a new behavior such as:
- If both ZRTP and SDes are available and the signaling protocol is unsecure, then select ZRTP.
- If both ZRTP and SDes are available and the signaling protocol is secure, then select SDes.

This question (of ZRTP preference) is also raised for XMPP. Although, SDes for XMPP does not work yet in every case: for example, it will works for GTalk with the gmail web app, but not with "Talk" (the android implementation).

This is only from looking at the commit, so I'm sorry if I came up with wrong conclusions.

Thank you for this review Ingo. Any question, remark or discussion is always welcome.

Regards,
Vincent

···

On 08/02/2012 08:59 PM, Ingo Bauersachs wrote:

Regards,
Ingo

--
Vincent Lucas, Ph.D. Jitsi developer
chenzo@jitsi.org http://jitsi.org
.

Ingo_Bauersachs · August 3, 2012, 8:58pm

Great to see the SDES support expanded to Jingle!
I have some comments/questions on your patch:

- CryptoPacketExtension:
Would it help to have some of the code from this class in the SDES-Library?
E.g. to avoid code duplication as the initialize and toSrtpCryptoAttribute
methods particularly look like something that assembles information into/from
strings which are defined by RFC4568.

Indeed, it would help. For example, it would be interesting to add to
the SrtpCryptoAttribute at least the 4 following functions:

Would it be possible for you to create patches for the SDES Library? I currently don't have any more time left than to review some patches that I consider important.
If you wish I can also add your account to the Google Code project directly.

[snip]

- Order of added Crypto Controls in SIP
I can't currently test your modifications, but from memory and looking at
CallPeerMediaHandlerSipImpl (@@ -586,8 +588,10 @@) it seems that ZRTP
now has preference over SDES. The order of updating the media
description matters. The new order causes that SDES is now only enabled
if the default encryption is disabled or if the extreme rare case of
adding the zrtp-hash to the SDP fails.

True, if both ZRTP and SDes are available, then ZRTP is chosen by
default. SDes is only used if at least one peer has disabled the
"Indicate support of ZRTP in signaling protocol".

I made exactly the opposite choice during the original implementation. The reason was that SDES is only enabled by advanced users and should therefore be the preference. Also I think there's not really a point in relying on the zrtp-hash because that can be stripped (you could call that a "downgrade to SDES" attack).
Ideally I think there is a choice of preference for the user, but I postponed that until I get the time to look at the MIKEY stuff again. (Which sadly is probably not in this decade...)

Another reason to put SDES first was the in-band ability of ZRTP: If there is no other encryption in place during the session setup, automatically fall back to ZRTP with its probing.

I do not have enough knowledge to judge which encryption is more
"robust" than the other. But, the only point I understand is that SDes
security is pointless if the signaling protocol is not encrypted (which
the situation with most of my SIP providers). If SDes (for any good
reason) is preferable than ZRTP, maybe we can discuss of a new behavior
such as:
- If both ZRTP and SDes are available and the signaling protocol is
unsecure, then select ZRTP.
- If both ZRTP and SDes are available and the signaling protocol is
secure, then select SDes.

ZRTP is definitely more secure because it's an end-to-end encryption (at least as long as you trust it's heuristic approach). Currently the UI does not indicate an SDES connection as secure if the connection to the server is not protected by TLS. But ideally this should again be a user choice. I guess SDES is most often used in corporate environments where you explicitly don't want end-to-end.

This question (of ZRTP preference) is also raised for XMPP. Although,
SDes for XMPP does not work yet in every case: for example, it will
works for GTalk with the gmail web app, but not with "Talk" (the android
implementation).

I guess this decision is protocol independent.
Do you know why it doesn't work with Talk? AFAIK Talk uses libsrtp and a pretty basic setup. It was at least this way when I created the SIP implementation, but I never tried it.

[snip]

Regards,
Vincent

Regards,
Ingo

Vincent_Lucas · August 27, 2012, 1:58pm

Hi Ingo,

Here you can find a patch to get the key and session params as strings.
If you integrate it to "sdes4j" and thereafter update the sdes4j.jar to Jitsi, then I will correspondingly modify the CryptoPacketExtension.java.

Regards,
Vincent

getKeyAndSessionParamsStringFromCryptoAttribute.patch (1.36 KB)

···

On 8/3/12 10:36 AM, Vincent Lucas wrote:

Hi Ingo,

On 08/02/2012 08:59 PM, Ingo Bauersachs wrote:

Hey Vincent

Subject: [jitsi~svn:9769] Adds SDES for XMPP. Works with Jingle and
GTalk
(only for gmail web app,

Project: jitsi
Repository: svn
Revision: 9769
Author: vincent_lucas
Date: 2012-08-02 08:53:41 UTC
Link:

Log Message:
------------
Adds SDES for XMPP. Works with Jingle and GTalk (only for gmail web app,
doesn not work on "talk" for android).

Great to see the SDES support expanded to Jingle!
I have some comments/questions on your patch:

- CryptoPacketExtension:
Would it help to have some of the code from this class in the
SDES-Library? E.g. to avoid code duplication as the initialize and
toSrtpCryptoAttribute methods particularly look like something that
assembles information into/from strings which are defined by RFC4568.

Indeed, it would help. For example, it would be interesting to add to
the SrtpCryptoAttribute at least the 4 following functions:

- public String getKeyParamsString()
// returns a string representation of the list of key params separated
// by ";".

- public String getSessionParamsString()
// returns a string representation of the list of session params
// separated by " ".

- public void setKeyParamsString(String keyParamsString)
// Parses the keyParamsString using the ";" separator in order to get
// a SrtpKeyParam list and store it as the keyParams attribute.

- public void setSessionParamsString(String sessionParamsString)
// Parses the sessionParamsString using the " " separator in order to
// get a SrtpSessionParam list and store it as the sessionParams
// attribute.

For the SDes library, these functions can be used inside
"CryptoAttribute.create" and "CryptoAttribute.encode"

- Order of added Crypto Controls in SIP
I can't currently test your modifications, but from memory and looking
at CallPeerMediaHandlerSipImpl (@@ -586,8 +588,10 @@) it seems that
ZRTP now has preference over SDES. The order of updating the media
description matters. The new order causes that SDES is now only
enabled if the default encryption is disabled or if the extreme rare
case of adding the zrtp-hash to the SDP fails.

True, if both ZRTP and SDes are available, then ZRTP is chosen by
default. SDes is only used if at least one peer has disabled the
"Indicate support of ZRTP in signaling protocol".

I do not have enough knowledge to judge which encryption is more
"robust" than the other. But, the only point I understand is that SDes
security is pointless if the signaling protocol is not encrypted (which
the situation with most of my SIP providers). If SDes (for any good
reason) is preferable than ZRTP, maybe we can discuss of a new behavior
such as:
- If both ZRTP and SDes are available and the signaling protocol is
unsecure, then select ZRTP.
- If both ZRTP and SDes are available and the signaling protocol is
secure, then select SDes.

This question (of ZRTP preference) is also raised for XMPP. Although,
SDes for XMPP does not work yet in every case: for example, it will
works for GTalk with the gmail web app, but not with "Talk" (the android
implementation).

This is only from looking at the commit, so I'm sorry if I came up
with wrong conclusions.

Thank you for this review Ingo. Any question, remark or discussion is
always welcome.

Regards,
Vincent

Regards,
Ingo

--
Vincent Lucas, Ph.D. Jitsi developer
chenzo@jitsi.org http://jitsi.org

Earl1 · August 5, 2012, 11:20pm

Ingo and Vincent,

I would like to express, as end user, my strong opposition to SDES having any priority over ZRTP.
Perhaps I have misunderstood your comments, but if the users wish to use ZRTP, then SDES
should never be used and have zero priority.

I see no reason to prefer SDES over ZRTP.

>>

- If both ZRTP and SDes are available and the signaling protocol is
secure, then select SDes.

Negative. ALWAYS select ZRTP and give ZRTP priority.

My opinion.

Regards, Earl

···

On 08/3/2012 22:58, Ingo Bauersachs wrote:

Great to see the SDES support expanded to Jingle!
I have some comments/questions on your patch:

- CryptoPacketExtension:
Would it help to have some of the code from this class in the SDES-Library?
E.g. to avoid code duplication as the initialize and toSrtpCryptoAttribute
methods particularly look like something that assembles information into/from
strings which are defined by RFC4568.

Indeed, it would help. For example, it would be interesting to add to
the SrtpCryptoAttribute at least the 4 following functions:

Would it be possible for you to create patches for the SDES Library? I currently don't have any more time left than to review some patches that I consider important.
If you wish I can also add your account to the Google Code project directly.

[snip]

- Order of added Crypto Controls in SIP
I can't currently test your modifications, but from memory and looking at
CallPeerMediaHandlerSipImpl (@@ -586,8 +588,10 @@) it seems that ZRTP
now has preference over SDES. The order of updating the media
description matters. The new order causes that SDES is now only enabled
if the default encryption is disabled or if the extreme rare case of
adding the zrtp-hash to the SDP fails.

True, if both ZRTP and SDes are available, then ZRTP is chosen by
default. SDes is only used if at least one peer has disabled the
"Indicate support of ZRTP in signaling protocol".

I made exactly the opposite choice during the original implementation. The reason was that SDES is only enabled by advanced users and should therefore be the preference. Also I think there's not really a point in relying on the zrtp-hash because that can be stripped (you could call that a "downgrade to SDES" attack).
Ideally I think there is a choice of preference for the user, but I postponed that until I get the time to look at the MIKEY stuff again. (Which sadly is probably not in this decade...)

Another reason to put SDES first was the in-band ability of ZRTP: If there is no other encryption in place during the session setup, automatically fall back to ZRTP with its probing.

I do not have enough knowledge to judge which encryption is more
"robust" than the other. But, the only point I understand is that SDes
security is pointless if the signaling protocol is not encrypted (which
the situation with most of my SIP providers). If SDes (for any good
reason) is preferable than ZRTP, maybe we can discuss of a new behavior
such as:
- If both ZRTP and SDes are available and the signaling protocol is
unsecure, then select ZRTP.
- If both ZRTP and SDes are available and the signaling protocol is
secure, then select SDes.

ZRTP is definitely more secure because it's an end-to-end encryption (at least as long as you trust it's heuristic approach). Currently the UI does not indicate an SDES connection as secure if the connection to the server is not protected by TLS. But ideally this should again be a user choice. I guess SDES is most often used in corporate environments where you explicitly don't want end-to-end.

This question (of ZRTP preference) is also raised for XMPP. Although,
SDes for XMPP does not work yet in every case: for example, it will
works for GTalk with the gmail web app, but not with "Talk" (the android
implementation).

I guess this decision is protocol independent.
Do you know why it doesn't work with Talk? AFAIK Talk uses libsrtp and a pretty basic setup. It was at least this way when I created the SIP implementation, but I never tried it.

[snip]

Regards,
Vincent

Regards,
Ingo

Vincent_Lucas · August 20, 2012, 2:28pm

Hi Ingo,

Sorry for the delay.

Great to see the SDES support expanded to Jingle!
I have some comments/questions on your patch:

- CryptoPacketExtension:
Would it help to have some of the code from this class in the SDES-Library?
E.g. to avoid code duplication as the initialize and toSrtpCryptoAttribute
methods particularly look like something that assembles information into/from
strings which are defined by RFC4568.

Indeed, it would help. For example, it would be interesting to add to
the SrtpCryptoAttribute at least the 4 following functions:

Would it be possible for you to create patches for the SDES Library? I currently don't have any more time left than to review some patches that I consider important.
If you wish I can also add your account to the Google Code project directly.

This is on my TODO list, but unfortunately I think that I will not be able to create the patches before next week.

[snip]

- Order of added Crypto Controls in SIP
I can't currently test your modifications, but from memory and looking at
CallPeerMediaHandlerSipImpl (@@ -586,8 +588,10 @@) it seems that ZRTP
now has preference over SDES. The order of updating the media
description matters. The new order causes that SDES is now only enabled
if the default encryption is disabled or if the extreme rare case of
adding the zrtp-hash to the SDP fails.

True, if both ZRTP and SDes are available, then ZRTP is chosen by
default. SDes is only used if at least one peer has disabled the
"Indicate support of ZRTP in signaling protocol".

I made exactly the opposite choice during the original implementation. The reason was that SDES is only enabled by advanced users and should therefore be the preference. Also I think there's not really a point in relying on the zrtp-hash because that can be stripped (you could call that a "downgrade to SDES" attack).
Ideally I think there is a choice of preference for the user, but I postponed that until I get the time to look at the MIKEY stuff again. (Which sadly is probably not in this decade...)

I have no personal preference between ZRTP and SDes. You are right, this choice may be ideally done by the user.

Another reason to put SDES first was the in-band ability of ZRTP: If there is no other encryption in place during the session setup, automatically fall back to ZRTP with its probing.

This is a double edge argument, I see the point and agree with your explication, but at the same time the opposite point of view may also looks coherent:
- As Jitsi has a default fall back to ZRTP if there is no other encryption in place, it may make sense to define ZRTP as the default encryption protocol in order to keep the same kind of behavior if ZRTP and SDes are both available.

I do not have enough knowledge to judge which encryption is more
"robust" than the other. But, the only point I understand is that SDes
security is pointless if the signaling protocol is not encrypted (which
the situation with most of my SIP providers). If SDes (for any good
reason) is preferable than ZRTP, maybe we can discuss of a new behavior
such as:
- If both ZRTP and SDes are available and the signaling protocol is
unsecure, then select ZRTP.
- If both ZRTP and SDes are available and the signaling protocol is
secure, then select SDes.

ZRTP is definitely more secure because it's an end-to-end encryption (at least as long as you trust it's heuristic approach). Currently the UI does not indicate an SDES connection as secure if the connection to the server is not protected by TLS. But ideally this should again be a user choice. I guess SDES is most often used in corporate environments where you explicitly don't want end-to-end.

This question (of ZRTP preference) is also raised for XMPP. Although,
SDes for XMPP does not work yet in every case: for example, it will
works for GTalk with the gmail web app, but not with "Talk" (the android
implementation).

I guess this decision is protocol independent.

True. Moreover, I would like to keep the same behavior between SIP and XMPP.

Do you know why it doesn't work with Talk? AFAIK Talk uses libsrtp and a pretty basic setup. It was at least this way when I created the SIP implementation, but I never tried it.

I do not know why Talk is not working, but here is the behavior observed:
- If Talk is the initiator of the call: the offer (Gingle initiate) is contains the encryption element. The result is that Talk plays sound without decoding it.
- If Jitsi is the initiator of the call: Jitsi sends an offer with the encryption element, but Talk answers without the encryption element. Thus, the call "works correctly", but is unencrypted.

Regards,
Vincent

···

On 08/03/2012 10:58 PM, Ingo Bauersachs wrote:
sent with the encryption element. Jisti answers (Gingle accept) also

[snip]

Regards,
Vincent

Regards,
Ingo

--
Vincent Lucas, Ph.D. Jitsi developer
chenzo@jitsi.org http://jitsi.org

Ingo_Bauersachs · August 27, 2012, 6:39pm

Hey

Thanks for that!
I've committed your patch, but I haven't yet created new packages.

What about CryptoPacketExtension.toSrtpCryptoAttribute? I think this does also some kind of (redundant) parsing which could be solved by the attached patch. Would the call to SrtpCryptoAttribute.create work for you?

Regards,
Ingo

cryptoattribute-from-strings.patch (4.42 KB)

···

-----Original Message-----
From: Vincent Lucas [mailto:chenzo@jitsi.org]
Sent: Montag, 27. August 2012 09:58
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: [jitsi~svn:9769] Adds SDES for XMPP. Works with
Jingle and GTalk (only for gmail web app,

Hi Ingo,

Here you can find a patch to get the key and session params as strings.
If you integrate it to "sdes4j" and thereafter update the sdes4j.jar to
Jitsi, then I will correspondingly modify the CryptoPacketExtension.java.

Regards,
Vincent

On 8/3/12 10:36 AM, Vincent Lucas wrote:
> Hi Ingo,
>
> On 08/02/2012 08:59 PM, Ingo Bauersachs wrote:
>> Hey Vincent
>>
>>> Subject: [jitsi~svn:9769] Adds SDES for XMPP. Works with Jingle and
>>> GTalk
>>> (only for gmail web app,
>>>
>>> Project: jitsi
>>> Repository: svn
>>> Revision: 9769
>>> Author: vincent_lucas
>>> Date: 2012-08-02 08:53:41 UTC
>>> Link:
>>>
>>> Log Message:
>>> ------------
>>> Adds SDES for XMPP. Works with Jingle and GTalk (only for gmail web app,
>>> doesn not work on "talk" for android).
>>
>> Great to see the SDES support expanded to Jingle!
>> I have some comments/questions on your patch:
>>
>> - CryptoPacketExtension:
>> Would it help to have some of the code from this class in the
>> SDES-Library? E.g. to avoid code duplication as the initialize and
>> toSrtpCryptoAttribute methods particularly look like something that
>> assembles information into/from strings which are defined by RFC4568.
>
> Indeed, it would help. For example, it would be interesting to add to
> the SrtpCryptoAttribute at least the 4 following functions:
>
> - public String getKeyParamsString()
> // returns a string representation of the list of key params separated
> // by ";".
>
> - public String getSessionParamsString()
> // returns a string representation of the list of session params
> // separated by " ".
>
> - public void setKeyParamsString(String keyParamsString)
> // Parses the keyParamsString using the ";" separator in order to get
> // a SrtpKeyParam list and store it as the keyParams attribute.
>
>
> - public void setSessionParamsString(String sessionParamsString)
> // Parses the sessionParamsString using the " " separator in order to
> // get a SrtpSessionParam list and store it as the sessionParams
> // attribute.
>
> For the SDes library, these functions can be used inside
> "CryptoAttribute.create" and "CryptoAttribute.encode"
>
>>
>> - Order of added Crypto Controls in SIP
>> I can't currently test your modifications, but from memory and looking
>> at CallPeerMediaHandlerSipImpl (@@ -586,8 +588,10 @@) it seems that
>> ZRTP now has preference over SDES. The order of updating the media
>> description matters. The new order causes that SDES is now only
>> enabled if the default encryption is disabled or if the extreme rare
>> case of adding the zrtp-hash to the SDP fails.
>
> True, if both ZRTP and SDes are available, then ZRTP is chosen by
> default. SDes is only used if at least one peer has disabled the
> "Indicate support of ZRTP in signaling protocol".
>
> I do not have enough knowledge to judge which encryption is more
> "robust" than the other. But, the only point I understand is that SDes
> security is pointless if the signaling protocol is not encrypted (which
> the situation with most of my SIP providers). If SDes (for any good
> reason) is preferable than ZRTP, maybe we can discuss of a new behavior
> such as:
> - If both ZRTP and SDes are available and the signaling protocol is
> unsecure, then select ZRTP.
> - If both ZRTP and SDes are available and the signaling protocol is
> secure, then select SDes.
>
> This question (of ZRTP preference) is also raised for XMPP. Although,
> SDes for XMPP does not work yet in every case: for example, it will
> works for GTalk with the gmail web app, but not with "Talk" (the android
> implementation).
>
>>
>> This is only from looking at the commit, so I'm sorry if I came up
>> with wrong conclusions.
>
> Thank you for this review Ingo. Any question, remark or discussion is
> always welcome.
>
> Regards,
> Vincent
>
>>
>> Regards,
>> Ingo
>>
>

--
Vincent Lucas, Ph.D. Jitsi developer
chenzo@jitsi.org http://jitsi.org

Vincent_Lucas · September 5, 2012, 1:56pm

Hi Ingo,

Hey

Thanks for that!
I've committed your patch, but I haven't yet created new packages.

What about CryptoPacketExtension.toSrtpCryptoAttribute? I think this does also some kind of (redundant) parsing which could be solved by the attached patch. Would the call to SrtpCryptoAttribute.create work for you?

Indeed, your patch helps me to remove the redundant part contained in CryptoPacketExtension.toSrtpCryptoAttribute.

The patch joined to this email is the same as yours, with 3 slightly changes:
- Adds some documentation.
- Sets and gets the sessionParams as "null" if there are none.
- In "build.xml", sets "MANIFEST.MF" in uppercase (my Linux is case sensitive).

I have just committed (svn revision #9831) "sdes4j.jar" (with the new modifications) and the corresponding changes to "CryptoPacketExtension".

Thank you for your help.

Regards,
Vincent

crypto_attributes_from_strings.patch (7.93 KB)

···

On 08/27/2012 08:39 PM, Ingo Bauersachs wrote:

Regards,
Ingo

-----Original Message-----
From: Vincent Lucas [mailto:chenzo@jitsi.org]
Sent: Montag, 27. August 2012 09:58
To: dev@jitsi.java.net
Subject: [jitsi-dev] Re: [jitsi~svn:9769] Adds SDES for XMPP. Works with
Jingle and GTalk (only for gmail web app,

Hi Ingo,

Here you can find a patch to get the key and session params as strings.
If you integrate it to "sdes4j" and thereafter update the sdes4j.jar to
Jitsi, then I will correspondingly modify the CryptoPacketExtension.java.

Regards,
Vincent

On 8/3/12 10:36 AM, Vincent Lucas wrote:

Hi Ingo,

On 08/02/2012 08:59 PM, Ingo Bauersachs wrote:

Hey Vincent

Subject: [jitsi~svn:9769] Adds SDES for XMPP. Works with Jingle and
GTalk
(only for gmail web app,

Project: jitsi
Repository: svn
Revision: 9769
Author: vincent_lucas
Date: 2012-08-02 08:53:41 UTC
Link:

Log Message:
------------
Adds SDES for XMPP. Works with Jingle and GTalk (only for gmail web app,
doesn not work on "talk" for android).

Great to see the SDES support expanded to Jingle!
I have some comments/questions on your patch:

- CryptoPacketExtension:
Would it help to have some of the code from this class in the
SDES-Library? E.g. to avoid code duplication as the initialize and
toSrtpCryptoAttribute methods particularly look like something that
assembles information into/from strings which are defined by RFC4568.

Indeed, it would help. For example, it would be interesting to add to
the SrtpCryptoAttribute at least the 4 following functions:

- public String getKeyParamsString()
// returns a string representation of the list of key params separated
// by ";".

- public String getSessionParamsString()
// returns a string representation of the list of session params
// separated by " ".

- public void setKeyParamsString(String keyParamsString)
// Parses the keyParamsString using the ";" separator in order to get
// a SrtpKeyParam list and store it as the keyParams attribute.

- public void setSessionParamsString(String sessionParamsString)
// Parses the sessionParamsString using the " " separator in order to
// get a SrtpSessionParam list and store it as the sessionParams
// attribute.

For the SDes library, these functions can be used inside
"CryptoAttribute.create" and "CryptoAttribute.encode"

- Order of added Crypto Controls in SIP
I can't currently test your modifications, but from memory and looking
at CallPeerMediaHandlerSipImpl (@@ -586,8 +588,10 @@) it seems that
ZRTP now has preference over SDES. The order of updating the media
description matters. The new order causes that SDES is now only
enabled if the default encryption is disabled or if the extreme rare
case of adding the zrtp-hash to the SDP fails.

True, if both ZRTP and SDes are available, then ZRTP is chosen by
default. SDes is only used if at least one peer has disabled the
"Indicate support of ZRTP in signaling protocol".

I do not have enough knowledge to judge which encryption is more
"robust" than the other. But, the only point I understand is that SDes
security is pointless if the signaling protocol is not encrypted (which
the situation with most of my SIP providers). If SDes (for any good
reason) is preferable than ZRTP, maybe we can discuss of a new behavior
such as:
- If both ZRTP and SDes are available and the signaling protocol is
unsecure, then select ZRTP.
- If both ZRTP and SDes are available and the signaling protocol is
secure, then select SDes.

This question (of ZRTP preference) is also raised for XMPP. Although,
SDes for XMPP does not work yet in every case: for example, it will
works for GTalk with the gmail web app, but not with "Talk" (the android
implementation).

This is only from looking at the commit, so I'm sorry if I came up
with wrong conclusions.

Thank you for this review Ingo. Any question, remark or discussion is
always welcome.

Regards,
Vincent

Regards,
Ingo

--
Vincent Lucas, Ph.D. Jitsi developer
chenzo@jitsi.org http://jitsi.org

--
Vincent Lucas, Ph.D. Jitsi developer
chenzo@jitsi.org http://jitsi.org

Ingo_Bauersachs · August 6, 2012, 8:07pm

Hey Earl

From: Earl [mailto:Large.Files@gmx.net]
I see no reason to prefer SDES over ZRTP.

Certainly not as an automatic detection. But if the user explicitly enables SDES it should be the priority.

- If both ZRTP and SDes are available and the signaling protocol is
secure, then select SDes.

Negative. ALWAYS select ZRTP and give ZRTP priority.

As above, when the user selects SDES it should be used with higher priority. Keep in mind that SDES is by default disabled and can only be enabled by bypassing very distinct warnings.

My opinion.
Regards, Earl

Regards,
Ingo

emcho · August 20, 2012, 10:56pm

Hey folks,

Hi Ingo,

Sorry for the delay.

Great to see the SDES support expanded to Jingle! I have some
comments/questions on your patch:

- CryptoPacketExtension: Would it help to have some of the code
from this class in the SDES-Library? E.g. to avoid code
duplication as the initialize and toSrtpCryptoAttribute methods
particularly look like something that assembles information
into/from strings which are defined by RFC4568.

Indeed, it would help. For example, it would be interesting to
add to the SrtpCryptoAttribute at least the 4 following
functions:

Would it be possible for you to create patches for the SDES
Library? I currently don't have any more time left than to review
some patches that I consider important. If you wish I can also
add your account to the Google Code project directly.

This is on my TODO list, but unfortunately I think that I will not be
able to create the patches before next week.

[snip]

- Order of added Crypto Controls in SIP I can't currently test
your modifications, but from memory and looking at
CallPeerMediaHandlerSipImpl (@@ -586,8 +588,10 @@) it seems
that ZRTP now has preference over SDES. The order of updating
the media description matters. The new order causes that SDES
is now only enabled if the default encryption is disabled or if
the extreme rare case of adding the zrtp-hash to the SDP
fails.

True, if both ZRTP and SDes are available, then ZRTP is chosen
by default. SDes is only used if at least one peer has disabled
the "Indicate support of ZRTP in signaling protocol".

I made exactly the opposite choice during the original
implementation. The reason was that SDES is only enabled by
advanced users and should therefore be the preference.

This does make sense however, I am somewhat uncomfortable about the fact
that it relies on Jitsi's default settings having SDES to off.

It sounds more reasonable to have such preferences based on protocol
robustness, so if we believe that ZRTP is a better choice, we should be
going for that.

Also I think

there's not really a point in relying on the zrtp-hash

I don't think that current trunk behaviour actually relies on the hash
in order to do ZRTP. If I have it right, the intention is to use it as
an indication that there are good chances ZRTP will be available after
call setup, so we don't need to rush on SDES.

because that

can be stripped (you could call that a "downgrade to SDES"
attack).

I don't get this. How is this weaker than simply preferring SDES
whenever it's available, without caring for ZRTP?

Ideally I think there is a choice of preference for the
user, but I postponed that until I get the time to look at the
MIKEY stuff again. (Which sadly is probably not in this decade...)

We definitely agree here. We were actually discussing configuration of
exactly these preferences with Chenzo today ... so "this decade" is
definitely a possibility ... even this quarter

Cheers,
Emil

···

On 20.08.12, 16:28, Vincent Lucas wrote:

On 08/03/2012 10:58 PM, Ingo Bauersachs wrote:

I have no personal preference between ZRTP and SDes. You are right,
this choice may be ideally done by the user.

Another reason to put SDES first was the in-band ability of ZRTP:
If there is no other encryption in place during the session setup,
automatically fall back to ZRTP with its probing.

This is a double edge argument, I see the point and agree with your
explication, but at the same time the opposite point of view may also
looks coherent: - As Jitsi has a default fall back to ZRTP if there
is no other encryption in place, it may make sense to define ZRTP as
the default encryption protocol in order to keep the same kind of
behavior if ZRTP and SDes are both available.

I do not have enough knowledge to judge which encryption is more
"robust" than the other. But, the only point I understand is that
SDes security is pointless if the signaling protocol is not
encrypted (which the situation with most of my SIP providers). If
SDes (for any good reason) is preferable than ZRTP, maybe we can
discuss of a new behavior such as: - If both ZRTP and SDes are
available and the signaling protocol is unsecure, then select
ZRTP. - If both ZRTP and SDes are available and the signaling
protocol is secure, then select SDes.

ZRTP is definitely more secure because it's an end-to-end
encryption (at least as long as you trust it's heuristic approach).
Currently the UI does not indicate an SDES connection as secure if
the connection to the server is not protected by TLS. But ideally
this should again be a user choice. I guess SDES is most often used
in corporate environments where you explicitly don't want
end-to-end.

This question (of ZRTP preference) is also raised for XMPP.
Although, SDes for XMPP does not work yet in every case: for
example, it will works for GTalk with the gmail web app, but not
with "Talk" (the android implementation).

I guess this decision is protocol independent.

True. Moreover, I would like to keep the same behavior between SIP
and XMPP.

Do you know why it doesn't work with Talk? AFAIK Talk uses libsrtp
and a pretty basic setup. It was at least this way when I created
the SIP implementation, but I never tried it.

I do not know why Talk is not working, but here is the behavior
observed: - If Talk is the initiator of the call: the offer (Gingle
initiate) is sent with the encryption element. Jisti answers (Gingle
accept) also contains the encryption element. The result is that Talk
plays sound without decoding it. - If Jitsi is the initiator of the
call: Jitsi sends an offer with the encryption element, but Talk
answers without the encryption element. Thus, the call "works
correctly", but is unencrypted.

Regards, Vincent

[snip]

Regards, Vincent

Regards, Ingo

--
Emil Ivov, Ph.D. 67000 Strasbourg,
Project Lead France
Jitsi
emcho@jitsi.org PHONE: +33.1.77.62.43.30
http://jitsi.org FAX: +33.1.77.62.47.31

Ingo_Bauersachs · August 21, 2012, 8:16pm

Hey

Hi Ingo,
Sorry for the delay.

No problem, I just wasn't sure if it was still on the radar.

Great to see the SDES support expanded to Jingle!
I have some comments/questions on your patch:

- CryptoPacketExtension: Would it help to have some of the code from
this class in the SDES- Library? E.g. to avoid code duplication as
the initialize and toSrtpCryptoAttribute methods particularly look
like something that assembles information into/from strings which are
defined by RFC4568.

Indeed, it would help. For example, it would be interesting to add to
the SrtpCryptoAttribute at least the 4 following functions:

Would it be possible for you to create patches for the SDES Library? I

currently don't have any more time left than to review some patches that I
consider important.

If you wish I can also add your account to the Google Code project

directly.

This is on my TODO list, but unfortunately I think that I will not be
able to create the patches before next week.

I have no hurry on that

[snip]

- Order of added Crypto Controls in SIP
I can't currently test your modifications, but from memory and looking at
CallPeerMediaHandlerSipImpl (@@ -586,8 +588,10 @@) it seems that ZRTP
now has preference over SDES. The order of updating the media
description matters. The new order causes that SDES is now only enabled
if the default encryption is disabled or if the extreme rare case of
adding the zrtp-hash to the SDP fails.

True, if both ZRTP and SDes are available, then ZRTP is chosen by
default. SDes is only used if at least one peer has disabled the
"Indicate support of ZRTP in signaling protocol".

I made exactly the opposite choice during the original implementation. The

reason was that SDES is only enabled by advanced users and should therefore
be the preference. Also I think there's not really a point in relying on the
zrtp-hash because that can be stripped (you could call that a "downgrade to
SDES" attack).

Ideally I think there is a choice of preference for the user, but I

postponed that until I get the time to look at the MIKEY stuff again. (Which
sadly is probably not in this decade...)

I have no personal preference between ZRTP and SDes. You are right, this
choice may be ideally done by the user.

ZRTP is usually considered more secure as it is end-to-end and should therefore be preferred as an automatic option.

I now took a third (or forth?) look at the changes and I think they are okay.
But what is misleading now is the UI-Description and behavior of what the ZRTP-Checkbox does: It doesn't simply hide the Hello-Hash in the SDP. If the setting is enabled it overrides any possibility of using SDES on the receiving side of a call (as updateMediaDescriptionForZrtp returns de-facto always true).

Another reason to put SDES first was the in-band ability of ZRTP: If there
is no other encryption in place during the session setup, automatically fall
back to ZRTP with its probing.

This is a double edge argument, I see the point and agree with your
explication, but at the same time the opposite point of view may also
looks coherent:
- As Jitsi has a default fall back to ZRTP if there is no other
encryption in place, it may make sense to define ZRTP as the default
encryption protocol in order to keep the same kind of behavior if ZRTP
and SDes are both available.

Well okay, we can stick with that. In fact I think it all returns to a matching UI for the current behavior.

[snip]

Do you know why it doesn't work with Talk? AFAIK Talk uses libsrtp and a
pretty basic setup. It was at least this way when I created the SIP
implementation, but I never tried it.

I do not know why Talk is not working, but here is the behavior observed:
- If Talk is the initiator of the call: the offer (Gingle initiate) is
sent with the encryption element. Jisti answers (Gingle accept) also
contains the encryption element. The result is that Talk plays sound
without decoding it.

You mean Talk plays just random noise, right? If so, this interesting because the SRTP packages are integrity protected and should be discarded if they are not considered valid at all.
While developing the SDES stuff for SIP I had an issue with Linksys phones and decryption during a conference call (with Jitsi as the bridge). It turned out that the missing encryption of the RTCP streams was the issue. When using Asterisk with libsrtp it printed a lot of debug messages concerning SRTCP failures. Maybe you can see/obtain a similar output from Talk? (I don't know that thing, so just guessing into the wild).

- If Jitsi is the initiator of the call: Jitsi sends an offer with the
encryption element, but Talk answers without the encryption element.
Thus, the call "works correctly", but is unencrypted.

Do the initially offered encryption schemes differ between Talk and Jitsi? If you have a pcap-trace, may I look at it?

Regards,
Vincent

Ingo

···

On 08/03/2012 10:58 PM, Ingo Bauersachs wrote:

Ingo_Bauersachs · September 5, 2012, 6:10pm

Thanks for that!
I've committed your patch, but I haven't yet created new packages.

What about CryptoPacketExtension.toSrtpCryptoAttribute? I think this does
also some kind of (redundant) parsing which could be solved by the attached
patch. Would the call to SrtpCryptoAttribute.create work for you?

Indeed, your patch helps me to remove the redundant part contained in
CryptoPacketExtension.toSrtpCryptoAttribute.

The patch joined to this email is the same as yours, with 3 slightly
changes: - Adds some documentation. - Sets and gets the sessionParams as
"null" if there are none. - In "build.xml", sets "MANIFEST.MF" in
uppercase (my Linux is case sensitive).

Thanks! I'll apply that soon also to the original project and add the missing unit tests for the new methods.
The patch I sent you was only intended to ask if it's alright that way - as I haven't even compiled the changes in it when I created it...

I have just committed (svn revision #9831) "sdes4j.jar" (with the new
modifications) and the corresponding changes to "CryptoPacketExtension".

Thank you for your help.

Thank you for bringing all the SDES stuff forward!

Ingo

Ingo_Bauersachs · August 21, 2012, 8:21pm

I made exactly the opposite choice during the original
implementation. The reason was that SDES is only enabled by
advanced users and should therefore be the preference.

This does make sense however, I am somewhat uncomfortable about the fact
that it relies on Jitsi's default settings having SDES to off.

It sounds more reasonable to have such preferences based on protocol
robustness, so if we believe that ZRTP is a better choice, we should be
going for that.

See my reply to Vincent, I'm okay with that.

Also I think
there's not really a point in relying on the zrtp-hash

I don't think that current trunk behaviour actually relies on the hash
in order to do ZRTP. If I have it right, the intention is to use it as
an indication that there are good chances ZRTP will be available after
call setup, so we don't need to rush on SDES.

Right, as I pointed out in the other reply, I think it's the UI that is confusing me. (And perhaps that weird language from your country of choice ;-))

because that
can be stripped (you could call that a "downgrade to SDES"
attack).

I don't get this. How is this weaker than simply preferring SDES
whenever it's available, without caring for ZRTP?

It isn't. I had a confused mind

Ideally I think there is a choice of preference for the
user, but I postponed that until I get the time to look at the
MIKEY stuff again. (Which sadly is probably not in this decade...)

We definitely agree here. We were actually discussing configuration of
exactly these preferences with Chenzo today ... so "this decade" is
definitely a possibility ... even this quarter

Ah, good

Cheers,
Emil

Ingo

Vincent_Lucas · September 5, 2012, 2:16pm

Hi Ingo,

[snip]

Ideally I think there is a choice of preference for the
user, but I postponed that until I get the time to look at the
MIKEY stuff again. (Which sadly is probably not in this decade...)

We definitely agree here. We were actually discussing configuration of
exactly these preferences with Chenzo today ... so "this decade" is
definitely a possibility ... even this quarter

Ah, good

Svn revision #9828 and build #4187 allow the user to define the preference between ZRTP end SDes.

As the "callee" always make the last choice, then its configuration is predominant.
I.e. if the preferences are:
- ZRTP 1st, SDes 2nd for the "caller".
- SDes 1st, ZRTP 2nd for the "callee".
Then the call will be encrypted via SDes.

Regards,
Vincent

···

On 08/21/2012 10:21 PM, Ingo Bauersachs wrote:

Cheers,
Emil

Ingo

--
Vincent Lucas, Ph.D. Jitsi developer
chenzo@jitsi.org http://jitsi.org