File transfers

[Dimitur Kirov]

The initial work on filetransfers for gossip is attached in gnome
bugzilla. There are some things that I want do discuss before going on.

LmBsSession is the public class. It is designed to keep track of all
transfers among all accounts. Currently libgossip creates one instance
of this class per account. I suggest to change this and put LmBsSession
instance in gossip-session, so that we have one LmBsSession per program.

LmBsClient class is used for all socket io actions. In the future it
will be replaced by lm-socket.c , because they overlap in
functionality. This will require extending the lm-socket class with
some functionality for synchronous communication:
 - callback for data_written event
 - ability to add and remove socket watches for reading

Function "lm_bs_transfer_send_success_reply" from lb-bs-transfer.c
sends a streamhost activation reply. It could be better suited in
libgossip, instead of loudmouth, but at the same time this will make
LmBsSession more complicated.

xep 0096 requires that clients with FT support should advertise their
FT profile as a result of disco info request. I added the following
features and I'm not sure if they cover everything that gossip has:
 <feature var="http://jabber.org/protocol/disco#info"></feature>
 <feature var="http://jabber.org/protocol/si"></feature>
 <feature
var="http://jabber.org/protocol/si/profile/file-transfer"></feature>
<feature var="http://jabber.org/protocol/bytestreams"></feature>
<feature var="http://jabber.org/protocol/muc"></feature>

Currently file transfer session ids are generated with a simple
incremental rule GOSSIPSID1,GOSSIPSID2 .. etc. This is not good from
security point of view, because every internet user will be able to
steal a file if he knows that certain user runs gossip.

Proxy support helps users behind masqueraded networks to send files.
However it is a bad practice to hardcode a predefined set of proxies
and at the same time users should not be bothered to enter FT proxy in
gossip settings. Do you think it is worth to implement proxy support
now, or better leave it for later versions ?

File request dialog  (ui/file_transfer.glade) looks old-fashioned. IMO
it will be better to have this request notification in chat windows,
similar to muc invitations.

Finally, we can enable the xep-0047 support with a simple rule like:
if file is <= 20Kb and the sender supports ibb, prefer ibb over
bytestreams. It is not appropriate to use this method for large files,
because user will be banned from server, due to high traffic.

Regards,
D.Kirov

_______________________________________________
Gossip-dev mailing list
Gossip-dev@...
http://lists.imendio.com/mailman/listinfo/gossip-dev

[Martyn Russell-3]

Dimitur Kirov wrote:

Hi Dimitur, thanks for the work on this! and sorry for the late reply.

> The initial work on filetransfers for gossip is attached in gnome
> bugzilla. There are some things that I want do discuss before going on.

Woohoo, I have yet to take a look, but I will do as soon as possible.

> LmBsSession is the public class. It is designed to keep track of all
> transfers among all accounts. Currently libgossip creates one instance
> of this class per account. I suggest to change this and put LmBsSession
> instance in gossip-session, so that we have one LmBsSession per program.

That makes sense to me too yes.

> LmBsClient class is used for all socket io actions. In the future it
> will be replaced by lm-socket.c , because they overlap in
> functionality. This will require extending the lm-socket class with
> some functionality for synchronous communication:
>  - callback for data_written event
>  - ability to add and remove socket watches for reading

Hmm, are you just extending the synchronous functionality here and also
have asynchronous functionality available?

> Function "lm_bs_transfer_send_success_reply" from lb-bs-transfer.c
> sends a streamhost activation reply. It could be better suited in
> libgossip, instead of loudmouth, but at the same time this will make
> LmBsSession more complicated.

I will take a look at the patch shortly and get back to you on this.

> xep 0096 requires that clients with FT support should advertise their
> FT profile as a result of disco info request. I added the following
> features and I'm not sure if they cover everything that gossip has:
>  <feature var="http://jabber.org/protocol/disco#info"></feature>
>  <feature var="http://jabber.org/protocol/si"></feature>
>  <feature
> var="http://jabber.org/protocol/si/profile/file-transfer"></feature>
> <feature var="http://jabber.org/protocol/bytestreams"></feature>
> <feature var="http://jabber.org/protocol/muc"></feature>

Yes, we should be able to do that with little trouble. We have
protocols/jabber/gossip-disco.[ch] to handle that sort of thing, I am
not sure of the state I left it in :) it might need some work.

> Currently file transfer session ids are generated with a simple
> incremental rule GOSSIPSID1,GOSSIPSID2 .. etc. This is not good from
> security point of view, because every internet user will be able to
> steal a file if he knows that certain user runs gossip.

OK. Actually Micke, isn't this something you changed in Loudmouth? So
the message IDs are now a generated number?

> Proxy support helps users behind masqueraded networks to send files.
> However it is a bad practice to hardcode a predefined set of proxies
> and at the same time users should not be bothered to enter FT proxy in
> gossip settings. Do you think it is worth to implement proxy support
> now, or better leave it for later versions ?

Hmm, I think perhaps it is better to implement that later so we have
something working.

> File request dialog  (ui/file_transfer.glade) looks old-fashioned. IMO
> it will be better to have this request notification in chat windows,
> similar to muc invitations.

That's a good idea, less dialogs, etc.

> Finally, we can enable the xep-0047 support with a simple rule like:
> if file is <= 20Kb and the sender supports ibb, prefer ibb over
> bytestreams. It is not appropriate to use this method for large files,
> because user will be banned from server, due to high traffic.

Yes, this makes sense.

--
Regards,
Martyn
_______________________________________________
Gossip-dev mailing list
Gossip-dev@...
http://lists.imendio.com/mailman/listinfo/gossip-dev

[Mikael Hallendal]

1 apr 2007 kl. 12.55 skrev Martyn Russell:

Hi,

> Dimitur Kirov wrote:
>
> Hi Dimitur, thanks for the work on this! and sorry for the late reply.

Indeed, very nice!

>> Currently file transfer session ids are generated with a simple
>> incremental rule GOSSIPSID1,GOSSIPSID2 .. etc. This is not good from
>> security point of view, because every internet user will be able to
>> steal a file if he knows that certain user runs gossip.
>
> OK. Actually Micke, isn't this something you changed in Loudmouth? So
> the message IDs are now a generated number?

Loudmouth currently generates entirely random numbers, too random in  
fact as it makes tracking the numbers hard :)

There is an issue open about adding an incremented value to the ID  
string in order to make it easier to follow:

http://developer.imendio.com/issues/browse/LM-75

Best Regards,
   Mikael Hallendal

--
Imendio AB, http://www.imendio.com


_______________________________________________
Gossip-dev mailing list
Gossip-dev@...
http://lists.imendio.com/mailman/listinfo/gossip-dev

[Martyn Russell-3]

Dimitur Kirov wrote:

> The initial work on filetransfers for gossip is attached in gnome
> bugzilla. There are some things that I want do discuss before going on.
>
> LmBsSession is the public class. It is designed to keep track of all
> transfers among all accounts. Currently libgossip creates one instance
> of this class per account. I suggest to change this and put LmBsSession
> instance in gossip-session, so that we have one LmBsSession per program.
>
> LmBsClient class is used for all socket io actions. In the future it
> will be replaced by lm-socket.c , because they overlap in
> functionality. This will require extending the lm-socket class with
> some functionality for synchronous communication:
>  - callback for data_written event
>  - ability to add and remove socket watches for reading
>
> Function "lm_bs_transfer_send_success_reply" from lb-bs-transfer.c
> sends a streamhost activation reply. It could be better suited in
> libgossip, instead of loudmouth, but at the same time this will make
> LmBsSession more complicated.
>
> xep 0096 requires that clients with FT support should advertise their
> FT profile as a result of disco info request. I added the following
> features and I'm not sure if they cover everything that gossip has:
>  <feature var="http://jabber.org/protocol/disco#info"></feature>
>  <feature var="http://jabber.org/protocol/si"></feature>
>  <feature
> var="http://jabber.org/protocol/si/profile/file-transfer"></feature>
> <feature var="http://jabber.org/protocol/bytestreams"></feature>
> <feature var="http://jabber.org/protocol/muc"></feature>
>
> Currently file transfer session ids are generated with a simple
> incremental rule GOSSIPSID1,GOSSIPSID2 .. etc. This is not good from
> security point of view, because every internet user will be able to
> steal a file if he knows that certain user runs gossip.
>
> Proxy support helps users behind masqueraded networks to send files.
> However it is a bad practice to hardcode a predefined set of proxies
> and at the same time users should not be bothered to enter FT proxy in
> gossip settings. Do you think it is worth to implement proxy support
> now, or better leave it for later versions ?
>
> File request dialog  (ui/file_transfer.glade) looks old-fashioned. IMO
> it will be better to have this request notification in chat windows,
> similar to muc invitations.
>
> Finally, we can enable the xep-0047 support with a simple rule like:
> if file is <= 20Kb and the sender supports ibb, prefer ibb over
> bytestreams. It is not appropriate to use this method for large files,
> because user will be banned from server, due to high traffic.

I have just tried out the patches and it works very nicely indeed.
Micke, I have commented on the bug, any chance we can get this new API
into Loudmouth?

--
Regards,
Martyn
_______________________________________________
Gossip-dev mailing list
Gossip-dev@...
http://lists.imendio.com/mailman/listinfo/gossip-dev

[Dimitur Kirov]

@Martyn Russell <martyn@...> wrote:
> > LmBsSession is the public class. It is designed to keep track of all
> > transfers among all accounts. Currently libgossip creates one
> > instance of this class per account. I suggest to change this and
> > put LmBsSession instance in gossip-session, so that we have one
> > LmBsSession per program.
>
> That makes sense to me too yes.
I couldn't figure out how to change it. gossip-session is part of the ui
and doesn't deal with lm-* api. At the same time all protocol related
classes are per account. Generally, we needed:
 
bs_session = lm_bs_session_get_default (NULL);
lm_bs_session_set_failure_function (bs_session, faiure_function, ...)
lm_bs_session_set_complete_function (bs_session, complete_function, ...)
lm_bs_session_set_progress_function (bs_session, progress_function, ...)

These three functions must be called once per program, not once per
account, or transfer. Can you point me to where should I put these
callbacks and their registration ?

>
> > LmBsClient class is used for all socket io actions. In the future it
> > will be replaced by lm-socket.c , because they overlap in
> > functionality. This will require extending the lm-socket class with
> > some functionality for synchronous communication:
> >  - callback for data_written event
> >  - ability to add and remove socket watches for reading
>
> Hmm, are you just extending the synchronous functionality here and
> also have asynchronous functionality available?

Just extending the synchronous functionality, by adding one more callback in
LmSocketFuncs and ability to set io watches by condition (read/write).

> > xep 0096 requires that clients with FT support should advertise
> > their FT profile as a result of disco info request. I added the
> > following features and I'm not sure if they cover everything that
> > gossip has: <feature
> > var="http://jabber.org/protocol/disco#info"></feature> <feature
> > var="http://jabber.org/protocol/si"></feature> <feature
> > var="http://jabber.org/protocol/si/profile/file-transfer"></feature>
> > <feature var="http://jabber.org/protocol/bytestreams"></feature>
> > <feature var="http://jabber.org/protocol/muc"></feature>
>
> Yes, we should be able to do that with little trouble. We have
> protocols/jabber/gossip-disco.[ch] to handle that sort of thing, I am
> not sure of the state I left it in :) it might need some work.

It is part of the patch already. Apparently the message handler needs to
stay alive all the time.


@Mikael Hallendal <micke@...>:

> >> Currently file transfer session ids are generated with a simple
> >> incremental rule GOSSIPSID1,GOSSIPSID2 .. etc. This is not good from
> >> security point of view, because every internet user will be able to
> >> steal a file if he knows that certain user runs gossip.
> >
> > OK. Actually Micke, isn't this something you changed in Loudmouth? So
> > the message IDs are now a generated number?
>
> Loudmouth currently generates entirely random numbers, too random in
> fact as it makes tracking the numbers hard :)

So it is fine then.


Regards,
D.Kirov
_______________________________________________
Gossip-dev mailing list
Gossip-dev@...
http://lists.imendio.com/mailman/listinfo/gossip-dev

[Mikael Hallendal]

4 apr 2007 kl. 20.21 skrev Martyn Russell:

Hi,

> I have just tried out the patches and it works very nicely indeed.
> Micke, I have commented on the bug, any chance we can get this new API
> into Loudmouth?

I have not had the time to look into this patch closely yet and as it  
currently stands it might take a while (I'm currently focusing on  
getting the HEAD version ready for a 1.3.2 release).

As I mentioned in the bug is that I might be willing to include this  
under a huge unstable-flag :) I will also look at the possibility to  
ship this outside of Loudmouth for the moment and what I would need  
to do in order to make that possible.

Best Regards,
   Mikael Hallendal

--
Imendio AB, http://www.imendio.com


_______________________________________________
Gossip-dev mailing list
Gossip-dev@...
http://lists.imendio.com/mailman/listinfo/gossip-dev