Review Request 124312: Initial SSL implementation

[Vineet Garg]

> On Sept. 10, 2015, 10:49 p.m., Siddharth Sharma wrote:
> > QByteArray remoteCertificate = QCryptographicHash::hash(devReply.value(), QCryptographicHash::Sha1).toHex();
> > 
> >   I am looking kde-connect code for first time so I might have some missing links about it, please read the review notes
> >   
> >   Is there any specific reason that SHA-1 is being used ?
> >   
> >   Note: line generated SHA1 Cryptographic Hash, SHA1 is going to deprecate soon, probably next year somewhere in 2016
> >         CA Authorities also suggest to do so. OpenSSL also by default generates SHA1 Hash, but that can be overriden
> >         by specifiying openSSL with -sha256/512 to generate SHA2 Hashesh also used as standard. These seem to be
> >         implemented in Qt5.1
> > 
> >         SHA-1 - provides ~80 security bits ( Attack is possible with the increasing computational power of systems )
> >         SHA-2 - 256 provides  128 Security bits , which is more secure as compared to SHA-1
> > 
> >   Recommendation: Usage >= SHA2 Crypto Hash.

This fingerprint is just meant for user to verify the device manually, SHA2 is longer and is difficult to verify from user's perspective. Although from coding point of view is is very easy to change and can be changed any time.


- Vineet


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


On Sept. 10, 2015, 9:49 p.m., Vineet Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124312/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2015, 9:49 p.m.)
> 
> 
> Review request for kdeconnect and Siddharth Sharma.
> 
> 
> 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
> -----
> 
>   cli/kdeconnect-cli.cpp 536c754 
>   core/CMakeLists.txt dd8fedb 
>   core/backends/devicelink.h 0ae6c00 
>   core/backends/lan/CMakeLists.txt 7c5be38 
>   core/backends/lan/downloadjob.h eeeab07 
>   core/backends/lan/downloadjob.cpp bba2e98 
>   core/backends/lan/landevicelink.h 7d31881 
>   core/backends/lan/landevicelink.cpp 0987057 
>   core/backends/lan/lanlinkprovider.h b379434 
>   core/backends/lan/lanlinkprovider.cpp 2555668 
>   core/backends/lan/lanpairinghandler.h PRE-CREATION 
>   core/backends/lan/lanpairinghandler.cpp PRE-CREATION 
>   core/backends/lan/server.h PRE-CREATION 
>   core/backends/lan/server.cpp PRE-CREATION 
>   core/backends/lan/socketlinereader.h 29b059a 
>   core/backends/lan/socketlinereader.cpp a6bd85e 
>   core/backends/lan/uploadjob.h 40056b3 
>   core/backends/lan/uploadjob.cpp 90978f2 
>   core/backends/linkprovider.h f4fc309 
>   core/backends/loopback/loopbackdevicelink.h 9d1ae14 
>   core/backends/loopback/loopbackdevicelink.cpp 718c5b8 
>   core/backends/pairinghandler.h PRE-CREATION 
>   core/backends/pairinghandler.cpp PRE-CREATION 
>   core/daemon.h d263163 
>   core/daemon.cpp cb6999e 
>   core/device.h 03f528c 
>   core/device.cpp d53b3c5 
>   core/kdeconnectconfig.h af6c6df 
>   core/kdeconnectconfig.cpp 37f33d5 
>   core/networkpackage.cpp 64cfab7 
>   plugins/sftp/mounter.h 76ac860 
>   tests/CMakeLists.txt b00a574 
>   tests/devicetest.cpp PRE-CREATION 
>   tests/kdeconnectconfigtest.cpp PRE-CREATION 
>   tests/lanlinkprovidertest.cpp PRE-CREATION 
>   tests/testsocketlinereader.cpp 77a9b4b 
>   tests/testsslsocketlinereader.cpp PRE-CREATION 
> 
> 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/20150913/438f712d/attachment.html>