Review Request 123593: [WIP] Refactored code to implement ssl and provide backward compatibility

Albert Vaca Cintora albertvaka at gmail.com
Mon May 4 20:46:01 UTC 2015 


> On May 3, 2015, 5:42 p.m., Albert Vaca Cintora wrote:
> > src/org/kde/kdeconnect/Device.java, lines 164-173
> > <https://git.reviewboard.kde.org/r/123593/diff/1/?file=365417#file365417line164>
> >
> >     This does not allow us to have more than one link per device, please change it. The device should not know what types of link to use: it should try them all until one works.
> 
> Vineet Garg wrote:
>     When we recieve an identity package, we test if a previous link is present or not. So I am not getting how is it possible to device to have more than one link per device(it only happens for a short period of time and then link is removed). The device can have multiple persistent links only when each link provider creates a new link. I was planning that we create a field "type" in identity package, and each link provider will create identity package by passing "type" in createIdentitiyPackage method. That's why I used this type of code.
>     If all three types of links are availaible i.e. normal, ssl, bluetooth, we will choose ssl between normal and ssl, this way we will get two device instances for a device, one with bluetooth and other with ssl/normal. We can mark them in UI with bluetooth sign or with no sign so that user can use whichever he wants.
>     Currenlty I am little bit unsure to show different devices for each link in UI, or set the priority by my self.

"The device can have multiple persistent links only when each link provider creates a new link." Exactly, that's what we want to make happen. For example, the BluetoothLinkProvider will detect a device that is already connected through the SslLinkProvider, so you will have two Links to the same Device. 

If the question is "Which one to use then?": Each link will have a priority, so you will prefer "local" (low-latency) links over links that have more bounces in the route. That would be: bluetooth > ssl > internet. Then, when the device needs to send something, it will try the link with the highest priority. Also, if it fails, it will try the next link in the list, and so on.


> On May 3, 2015, 5:42 p.m., Albert Vaca Cintora wrote:
> > src/org/kde/kdeconnect/Device.java, line 522
> > <https://git.reviewboard.kde.org/r/123593/diff/1/?file=365417#file365417line522>
> >
> >     Right now the LinkProvider receives a pairing package, passes it to the Device, and the Device passes it back to the PairingHandler. What if the LinkProvider passes identity packages directly to the PairingHandler instead of the device? This way the Device won't need to know about the different PairingHandlers of the different LinkProviders. (Because every LinkProvider knows its own PairingHandler).
> 
> Vineet Garg wrote:
>     The reverse is not true. While we are sending packages, we are sending them directly via link (no role of link provider). This will make sending unpair packages difficult.

The device could pass the unpair packages to the link provider, or the link provider could intercept all the packages. In any case, I think it's cleaner that every linkprovider manages its own pairing, its own encryption, and its own everything possible. 

That makes even more sense due to the different types of links: the loopback link doesn't need encryption, the bluetooth link might need encryption, but will not need pairing (because you will do the pairing through Bluetooth).

So, the more self-contained a link type is, the better. And, in any case, Device should not know about the different types of links, that's bad design!


- Albert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123593/#review79824
-----------------------------------------------------------


On May 2, 2015, 6:31 a.m., Vineet Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123593/
> -----------------------------------------------------------
> 
> (Updated May 2, 2015, 6:31 a.m.)
> 
> 
> Review request for kdeconnect and Albert Vaca Cintora.
> 
> 
> Repository: kdeconnect-android
> 
> 
> Description
> -------
> 
> * Moved private key and public key from Device to Link
> * Encrypt and decrypt functions moved from NetworkPackage class to LanLink class
> * Added a new ParingHandler class so that each type of link can handle its own pairing process
> * Added a field of "type" in device so that we can differentiate between devices based on types
> * Now device need to send which type of link they will prefer in identity package, if nothing is send, the type is considered "normal". No change is done in createIdentityPackage now.
> 
> ISSUES
> * SFTP plugin is not working now, because it uses public key from device which is not present now
> * There is an instance of Public Key in pairing handler also, which needs to be removed, this instance is only used to save public key when pairing is done
> 
> 
> Diffs
> -----
> 
>   src/org/kde/kdeconnect/Backends/BaseLink.java 579a7af 
>   src/org/kde/kdeconnect/Backends/BaseLinkProvider.java cfaf621 
>   src/org/kde/kdeconnect/Backends/LanBackend/LanLink.java 5994142 
>   src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java ae26958 
>   src/org/kde/kdeconnect/Backends/LanBackend/LanPairingHandler.java PRE-CREATION 
>   src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackLink.java add92f8 
>   src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackLinkProvider.java bd9c41b 
>   src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackPairingHandler.java PRE-CREATION 
>   src/org/kde/kdeconnect/Backends/PairingHandler.java PRE-CREATION 
>   src/org/kde/kdeconnect/BackgroundService.java 855a52d 
>   src/org/kde/kdeconnect/Device.java 9fe583c 
>   src/org/kde/kdeconnect/NetworkPackage.java e5a777e 
>   src/org/kde/kdeconnect/Plugins/SftpPlugin/SftpImpl.java ec41060 
>   src/org/kde/kdeconnect/Plugins/SftpPlugin/SftpPlugin.java af7c66d 
>   src/org/kde/kdeconnect/UserInterface/PairActivity.java 7a45751 
> 
> Diff: https://git.reviewboard.kde.org/r/123593/diff/
> 
> 
> Testing
> -------
> 
> Testing done with pairing and unpairing both pc and mobile, working fine
> Tested using Ping plugin and Touchpad plugin, working fine.
> 
> 
> Thanks,
> 
> Vineet Garg
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20150504/e5dfccbc/attachment.html>

More information about the KDEConnect mailing list