Re: Re: GSoC Mid-Term Evaluation

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. :p

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

Other comments inline.

On 7/11/07, Romain KUNTZ <r.kuntzgmail.com> wrote:
> 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:
>
> On 2007/07/09, at 12:50, Bing Su wrote:
> > 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.


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. 

> > 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   I may
have some more questions in the next days.
>
> > Some of my code is derived from Werner's ccRtp.
Many thanks to
> > him. 
>
> 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. 

> Werner, no need to say that you are free to take a look
at Su's code
> if you feel so 
>
> 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 

Cheers,
Su

>
> Cheers,
> --
> Romain KUNTZ
> kuntzlsiit.u-strasbg.fr
> Louis Pasteur University - Networks and Protocols Team
>
>
>

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesip-communicator.dev.java.net
For additional commands, e-mail: dev-helpsip-communicator.dev.java.net