Review Request 124312: Initial SSL implementation

Vineet Garg grg.vineet at gmail.com
Tue Jul 14 12:15:31 UTC 2015



> On July 11, 2015, midnight, Aleix Pol Gonzalez wrote:
> > core/backends/lan/lanlinkprovider.cpp, line 54
> > <https://git.reviewboard.kde.org/r/124312/diff/1/?file=383919#file383919line54>
> >
> >     Is it leaking?

Since there is one pairing handler per link provider, it should not leak.


> On July 11, 2015, midnight, Aleix Pol Gonzalez wrote:
> > core/backends/lan/lanlinkprovider.cpp, line 125
> > <https://git.reviewboard.kde.org/r/124312/diff/1/?file=383919#file383919line125>
> >
> >     NetworkPackages are usually instantiated using the stack rather than heap. Why's the change?

It is required since the package is needed in "encrypted" is ssl is used. There was a memory leak issue, fixed now.


> On July 11, 2015, midnight, Aleix Pol Gonzalez wrote:
> > core/kdeconnectconfig.cpp, line 120
> > <https://git.reviewboard.kde.org/r/124312/diff/1/?file=383936#file383936line120>
> >
> >     Isn't QSsl able to generate a certificate? It would be cool if we could drop the dependency...

No. There is not even a single method to set something for certificate. I think it is just meant to read certificates.


On July 11, 2015, midnight, Vineet Garg wrote:
> > Maybe you could go a bit further with the testing? :)

Left out due to last minute change in server. Now fixed.


- Vineet


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


On July 14, 2015, 12:15 p.m., Vineet Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124312/
> -----------------------------------------------------------
> 
> (Updated July 14, 2015, 12:15 p.m.)
> 
> 
> Review request for kdeconnect.
> 
> 
> Repository: kdeconnect-kde
> 
> 
> Description
> -------
> 
> * Similar to Android version, this SSL implementation supports old version also.
> * Certificate is generated when public key is generated using QCA, but used as QSslCertificate because QSslSocket don't like QCA certificate.
> * A new server is written overriding QTcpServer for setting socket descriptor to QSslSocket.
> * QSslSocket works as plain socket to transfer identity package and then either uses encrypted mode or normal mode according to the received package.
> * If connetion is to be on SSL, new link is only added after successful handshake.
> * Certificate is embedded into identity package on start of encrypted mode and then set in device using it.
> * PairingHandler is added like in Android version so that each LinkProvider manage pairing according to its own rules.
> 
> P.S. : Uploaded the patch with some debug statements, will comment out them.
> 
> 
> Diffs
> -----
> 
>   core/backends/lan/server.cpp PRE-CREATION 
>   core/backends/lan/server.h PRE-CREATION 
>   core/backends/lan/lanpairinghandler.cpp PRE-CREATION 
>   core/backends/lan/lanpairinghandler.h PRE-CREATION 
>   core/backends/lan/lanlinkprovider.h b379434 
>   core/backends/lan/lanlinkprovider.cpp f5d2b22 
>   core/backends/lan/landevicelink.cpp 0987057 
>   core/backends/lan/landevicelink.h 7d31881 
>   core/backends/lan/downloadjob.cpp bba2e98 
>   core/backends/lan/downloadjob.h eeeab07 
>   core/backends/lan/CMakeLists.txt 7c5be38 
>   core/CMakeLists.txt dd8fedb 
>   core/backends/lan/socketlinereader.h b3be55a 
>   core/backends/lan/socketlinereader.cpp a6bd85e 
>   core/backends/lan/uploadjob.h 12b6f7e 
>   core/backends/lan/uploadjob.cpp b0d35fc 
>   core/backends/linkprovider.h f4fc309 
>   core/backends/pairinghandler.h PRE-CREATION 
>   core/backends/pairinghandler.cpp PRE-CREATION 
>   core/daemon.h 674ca01 
>   core/daemon.cpp af4c67b 
>   core/device.h 03f528c 
>   core/device.cpp c2dc609 
>   core/kdeconnectconfig.h af6c6df 
>   core/kdeconnectconfig.cpp 919505c 
>   core/networkpackage.h 6a0bf9c 
>   core/networkpackage.cpp 64cfab7 
>   tests/CMakeLists.txt b00a574 
>   tests/testsocketlinereader.cpp 77a9b4b 
> 
> Diff: https://git.reviewboard.kde.org/r/124312/diff/
> 
> 
> Testing
> -------
> 
> * Working fine, tested using connection to Android devices.
> * Pairing handler needs to be tested using two LinkProvider, working fine with one, but there be some problems with two LinkProviders simultaneouly.
> 
> 
> Thanks,
> 
> Vineet Garg
> 
>

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


More information about the KDEConnect mailing list