[sip-comm-dev] Re: GSoC Mid-Term Evaluation


#1

Hi Romain / Jean,

Long time no talk :slight_smile:
I was just about to send my code for mid-term evaluation and now I see
your mail.

The file attached with this mail is a "minimal" working patch for SC-SRTP.
I created this patch using eclipse with latest cvs source (07/09/2007).
And did a "two sip communicator test". There is a flag (usingSRTP) in
CallSessionImpl controlling whether shall we
use SRTP encryption. If both sides are on or off, then we will get
correct voice.
If one side is on and the other side is off, then we will hear only
noisy sounds.

Because we don't have a key management protocol yet, I can't test the
impl with other clients. And the master key / salt is hard coded in
the program.

I used jdk1.4.2 for compilation but it failed at somewhere else, so I
used jdk1.6.0.

Bouncycastle's jar file is included in the patch. It's manifest file
is very big and will cause wired class not found exception. So I
removed it from the jar file.

I've encountered some other wired class not found problems with
eclipse, but rebuilding it with ant has fixed them.

Because I think the code is still unstable, I wrote no java comments.
They will be added later when the code is stable enough.

Still there are many places need to be polished and many work need to done.
For now, I think we are on the correct way of SRTP implementation.

Some of my code is derived from Werner's ccRtp. Many thanks to him. :slight_smile:

If you have any question, please let me know.

Thanks,
Su

sc-srtp-impl-v0.1.patch.gz (1.01 MB)

路路路

On 7/9/07, Romain KUNTZ <r.kuntz@gmail.com> wrote:

Hi Su,

How are you doing?
Today starts the GSoC middle term evaluations. Note that the
evaluation is not a goal in itself, but we would need to see what you
have achieved so far to check that we are in the right way in the
project. Could you submit us your code (as a patch or a tarball) as
soon as possible, so that we can start your evaluation?

Also, do not forget to fill your midterm survey as explained in
http://groups.google.com/group/google-summer-of-code-announce/web/
midterm-survey-information

Cheers,
Romain & Jean


#2

Hi Su,

Thank you very much for your contribution. I'm now reviewing it and will further look at the code in the following days. But it looks like you have done some good piece of job already! I have some minor comments inline:

The file attached with this mail is a "minimal" working patch for SC-SRTP.
I created this patch using eclipse with latest cvs source (07/09/2007).

Regarding the patch creation, could you avoid to include the Eclipse .settings diifs in the patch? Also, do not include the bouncy castle jar in the patch, but as a separate file.

And did a "two sip communicator test". There is a flag (usingSRTP) in
CallSessionImpl controlling whether shall we
use SRTP encryption. If both sides are on or off, then we will get
correct voice.

I could not see the usingSRTP flag anywhere in the code. Did you forget to include some files in the patch?
Also, I remember you sent some service definition few weeks ago, would you mind include it in the patch too? Or is it deprecated?
About the interfaces, should'nt they be located in service/media instead of impl/media?

I used jdk1.4.2 for compilation but it failed at somewhere else, so I
used jdk1.6.0.

We try to remain 1.4 compatible, so could you try to solve the problems you have when using 1.4? Maybe you use some of the 1.6 functionalities? In that case, you'll have to find some equivalent compatible with 1.4.

Bouncycastle's jar file is included in the patch. It's manifest file
is very big and will cause wired class not found exception. So I
removed it from the jar file.

The one you use seems to be bcprov-jdk16-136.jar. In order to be 1.4 compatible, could you use bcprov-jdk14-137.jar instead?

Because I think the code is still unstable, I wrote no java comments.
They will be added later when the code is stable enough.

Ok thank you. As a side note regarding code convention, could you also put your "implements" and "extends" clauses on their own lines with a single class/interface per line? Also, when your code exceeds 80 columns, continue on a new line.

Still there are many places need to be polished and many work need to done.
For now, I think we are on the correct way of SRTP implementation.

Looks like, yes :slight_smile: I may have some more questions in the next days.

Some of my code is derived from Werner's ccRtp. Many thanks to him. :slight_smile:

Could you also acknowledge your sources when necessary? As an example, you check the header from src/net/java/sip/communicator/util/xml/DOMElementWriter.java

Werner, no need to say that you are free to take a look at Su's code if you feel so :slight_smile:

Once again Su, thank you for your contribution. Do not forget to fill your mid-term survey before the deadline, and keep up the good work for the next steps of the project!

Cheers,

路路路

On 2007/07/09, at 12:50, Bing Su wrote:
--
Romain KUNTZ
kuntz@lsiit.u-strasbg.fr
Louis Pasteur University - Networks and Protocols Team

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#3

Hey Su,

Thanks for submitting!

I just noticed that you have included all class dependencies in the
patch. Do you think you could take them out of the patch and submit them
apart (e.g. in a jar of their own)? This would make the patch easier to
review.

Thanks
Emil

Bing Su wrote:

路路路

Hi Romain / Jean,

Long time no talk :slight_smile:
I was just about to send my code for mid-term evaluation and now I see
your mail.

The file attached with this mail is a "minimal" working patch for SC-SRTP.
I created this patch using eclipse with latest cvs source (07/09/2007).
And did a "two sip communicator test". There is a flag (usingSRTP) in
CallSessionImpl controlling whether shall we
use SRTP encryption. If both sides are on or off, then we will get
correct voice.
If one side is on and the other side is off, then we will hear only
noisy sounds.

Because we don't have a key management protocol yet, I can't test the
impl with other clients. And the master key / salt is hard coded in
the program.

I used jdk1.4.2 for compilation but it failed at somewhere else, so I
used jdk1.6.0.

Bouncycastle's jar file is included in the patch. It's manifest file
is very big and will cause wired class not found exception. So I
removed it from the jar file.

I've encountered some other wired class not found problems with
eclipse, but rebuilding it with ant has fixed them.

Because I think the code is still unstable, I wrote no java comments.
They will be added later when the code is stable enough.

Still there are many places need to be polished and many work need to done.
For now, I think we are on the correct way of SRTP implementation.

Some of my code is derived from Werner's ccRtp. Many thanks to him. :slight_smile:

If you have any question, please let me know.

Thanks,
Su

On 7/9/07, Romain KUNTZ <r.kuntz@gmail.com> wrote:

Hi Su,

How are you doing?
Today starts the GSoC middle term evaluations. Note that the
evaluation is not a goal in itself, but we would need to see what you
have achieved so far to check that we are in the right way in the
project. Could you submit us your code (as a patch or a tarball) as
soon as possible, so that we can start your evaluation?

Also, do not forget to fill your midterm survey as explained in
http://groups.google.com/group/google-summer-of-code-announce/web/
midterm-survey-information

Cheers,
Romain & Jean

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#4

Hi Romain,

Thank you for your reviewing. I created the patch using eclipse, but I
didn't double check the result and it doesn't contain the
CallSessionImpl's modification. Sorry for this inconvenience. :stuck_out_tongue:

Now I'm sending my code in a zip file, because eclipse's patch doesn't
seem to work correctly.

Other comments inline.

Hi Su,

Thank you very much for your contribution. I'm now reviewing it and
will further look at the code in the following days. But it looks
like you have done some good piece of job already! I have some minor
comments inline:

> The file attached with this mail is a "minimal" working patch for
> SC-SRTP.
> I created this patch using eclipse with latest cvs source
> (07/09/2007).

Regarding the patch creation, could you avoid to include the
Eclipse .settings diifs in the patch? Also, do not include the bouncy
castle jar in the patch, but as a separate file.

Of course, no problem.

> And did a "two sip communicator test". There is a flag (usingSRTP) in
> CallSessionImpl controlling whether shall we
> use SRTP encryption. If both sides are on or off, then we will get
> correct voice.

I could not see the usingSRTP flag anywhere in the code. Did you
forget to include some files in the patch?
Also, I remember you sent some service definition few weeks ago,
would you mind include it in the patch too? Or is it deprecated?
About the interfaces, should'nt they be located in service/media
instead of impl/media?

The crypto service interface I posted earlier is now deprecated for
mid-term evaluation (We can discuss this later). Because at last I get
to know the encryption algorithms of SRTP are not general encryption
methods, they are defined by RFC3711 (the Counter Mode and F8 Mode).
Other common working modes of AES cipher are not used by SRTP. The
dependency on bouncy castle is the basic AES block cipher which
encrypts / decrypts data only in 128bit block. And I don't think we
need such a service interface for this basic AES block cipher, because
it is very simple and primitive. If we want to use such a cipher, we
should use the JCE / bouncy castle's native interface instead. Now the
cipher related code are in SRTPCipher class.
Of course, if we want a common yet easy to use cipher interface, I can
provide one easily based on the crypto service code I posted before.
:slight_smile:

And the SRTP's interface, I remember Emil said that they will just be
the implementation detail of media service, and should not be visible
to other components. In my understanding, when we have the key
management protocol, we can tell if SRTP will be enabled based on the
negotiation result.

> I used jdk1.4.2 for compilation but it failed at somewhere else, so I
> used jdk1.6.0.

We try to remain 1.4 compatible, so could you try to solve the
problems you have when using 1.4? Maybe you use some of the 1.6
functionalities? In that case, you'll have to find some equivalent
compatible with 1.4.

The JDK1.4 problem is not in SRTP's implementation. It in some other
packages. For the SRTP related class, I believe they are jdk1.4
compatible. :slight_smile:

> Bouncycastle's jar file is included in the patch. It's manifest file
> is very big and will cause wired class not found exception. So I
> removed it from the jar file.

The one you use seems to be bcprov-jdk16-136.jar. In order to be 1.4
compatible, could you use bcprov-jdk14-137.jar instead?

I will attach the jdk1.4 version of bouncy castle with this email.

> Because I think the code is still unstable, I wrote no java comments.
> They will be added later when the code is stable enough.

Ok thank you. As a side note regarding code convention, could you
also put your "implements" and "extends" clauses on their own lines
with a single class/interface per line? Also, when your code exceeds
80 columns, continue on a new line.

Sure they are fixed now.

> Still there are many places need to be polished and many work need
> to done.
> For now, I think we are on the correct way of SRTP implementation.

Looks like, yes :slight_smile: I may have some more questions in the next days.

> Some of my code is derived from Werner's ccRtp. Many thanks to
> him. :slight_smile:

Could you also acknowledge your sources when necessary? As an
example, you check the header from src/net/java/sip/communicator/util/
xml/DOMElementWriter.java

Done. :slight_smile:

Werner, no need to say that you are free to take a look at Su's code
if you feel so :slight_smile:

Once again Su, thank you for your contribution. Do not forget to fill
your mid-term survey before the deadline, and keep up the good work
for the next steps of the project!

You and the community are always very warm hearted and very nice to me.
I've learnt a lot of things through this GSoC program. It's really a honor to
have a mentoring organization as SIP Communicator's and a honor to have a mentor
like you, Jean and Emil. Thank you all :slight_smile:

Cheers,
Su

sc-srtp-impl-v0.1.zip (27.5 KB)

bcprov-jdk14-137.jar (1.4 MB)

路路路

On 7/11/07, Romain KUNTZ <r.kuntz@gmail.com> wrote:

On 2007/07/09, at 12:50, Bing Su wrote:

Cheers,
--
Romain KUNTZ
kuntz@lsiit.u-strasbg.fr
Louis Pasteur University - Networks and Protocols Team


#5

Hi Emil,

I've separated them in my last mail to dev, please check it out. :slight_smile:

Thanks,
Su

路路路

On 7/12/07, Emil Ivov <emcho@emcho.com> wrote:

Hey Su,

Thanks for submitting!

I just noticed that you have included all class dependencies in the
patch. Do you think you could take them out of the patch and submit them
apart (e.g. in a jar of their own)? This would make the patch easier to
review.

Thanks
Emil

Bing Su wrote:
> Hi Romain / Jean,
>
> Long time no talk :slight_smile:
> I was just about to send my code for mid-term evaluation and now I see
> your mail.
>
> The file attached with this mail is a "minimal" working patch for SC-SRTP.
> I created this patch using eclipse with latest cvs source (07/09/2007).
> And did a "two sip communicator test". There is a flag (usingSRTP) in
> CallSessionImpl controlling whether shall we
> use SRTP encryption. If both sides are on or off, then we will get
> correct voice.
> If one side is on and the other side is off, then we will hear only
> noisy sounds.
>
> Because we don't have a key management protocol yet, I can't test the
> impl with other clients. And the master key / salt is hard coded in
> the program.
>
> I used jdk1.4.2 for compilation but it failed at somewhere else, so I
> used jdk1.6.0.
>
> Bouncycastle's jar file is included in the patch. It's manifest file
> is very big and will cause wired class not found exception. So I
> removed it from the jar file.
>
> I've encountered some other wired class not found problems with
> eclipse, but rebuilding it with ant has fixed them.
>
> Because I think the code is still unstable, I wrote no java comments.
> They will be added later when the code is stable enough.
>
> Still there are many places need to be polished and many work need to done.
> For now, I think we are on the correct way of SRTP implementation.
>
> Some of my code is derived from Werner's ccRtp. Many thanks to him. :slight_smile:
>
> If you have any question, please let me know.
>
> Thanks,
> Su
>
> On 7/9/07, Romain KUNTZ <r.kuntz@gmail.com> wrote:
>> Hi Su,
>>
>> How are you doing?
>> Today starts the GSoC middle term evaluations. Note that the
>> evaluation is not a goal in itself, but we would need to see what you
>> have achieved so far to check that we are in the right way in the
>> project. Could you submit us your code (as a patch or a tarball) as
>> soon as possible, so that we can start your evaluation?
>>
>> Also, do not forget to fill your midterm survey as explained in
>> http://groups.google.com/group/google-summer-of-code-announce/web/
>> midterm-survey-information
>>
>> Cheers,
>> Romain & Jean
>>
>> ------------------------------------------------------------------------
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
>> For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net