Review Request 124140: Ported over to Netty and added SSL support

Vineet Garg grg.vineet at gmail.com
Wed Jul 1 12:07:57 UTC 2015



> On July 1, 2015, 5:33 a.m., Albert Vaca Cintora wrote:
> > src/org/kde/kdeconnect/Helpers/SecurityHelpers/SslHelper.java, line 66
> > <https://git.reviewboard.kde.org/r/124140/diff/2-3/?file=381391#file381391line66>
> >
> >     Why this security provider is not conscrypt?

Because conscrypt cannot generate certificates, only bouncy castle can. Bouncycastle cannot handle TLS connections like conscrypt.


> On July 1, 2015, 5:33 a.m., Albert Vaca Cintora wrote:
> > src/org/kde/kdeconnect/Helpers/SecurityHelpers/SslHelper.java, line 29
> > <https://git.reviewboard.kde.org/r/124140/diff/2-3/?file=381391#file381391line29>
> >
> >     What's the difference between bouncycastle and spongycastle?

"The Android platform unfortunately ships with a cut-down version of Bouncy Castle - as well as being crippled, it also makes installing an updated version of the libraries difficult due to classloader conflicts.

Spongy Castle is the stock Bouncy Castle libraries with a couple of small changes to make it work on Android"

This happened on old android devices, where certificate generation failed due class not found error from bouncycastle.


> On July 1, 2015, 5:33 a.m., Albert Vaca Cintora wrote:
> > src/org/kde/kdeconnect/Helpers/SecurityHelpers/SslHelper.java, line 174
> > <https://git.reviewboard.kde.org/r/124140/diff/2-3/?file=381391#file381391line174>
> >
> >     TLSv1.2 is only supported on Android API 16+, according to this link: http://developer.android.com/reference/javax/net/ssl/SSLSocket.html

I am using SSLEngine[1] and it says that supported on API 20+. With new provider this should not be an issue, tested on device with api level 17 and 18.

[1]http://developer.android.com/reference/javax/net/ssl/SSLEngine.html


> On July 1, 2015, 5:33 a.m., Albert Vaca Cintora wrote:
> > src/org/kde/kdeconnect/Device.java, line 330
> > <https://git.reviewboard.kde.org/r/124140/diff/2-3/?file=381389#file381389line330>
> >
> >     How come you had to add this? Weren't we saving the name before?

I moved that to pairing handler, later it is added back to device.


- Vineet


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


On June 30, 2015, 10:12 p.m., Vineet Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124140/
> -----------------------------------------------------------
> 
> (Updated June 30, 2015, 10:12 p.m.)
> 
> 
> Review request for kdeconnect and Albert Vaca Cintora.
> 
> 
> Repository: kdeconnect-android
> 
> 
> Description
> -------
> 
> * Since MINA was tying hands in adding SSL, ported over Netty which has a good, clean and easy interface and a larger user and developer base.
> * Added support to setup links on SSL
> * Links automatically removed if wrong certificate is sent
> * Shows keys based on hash of certificate to check right certificates are received
> * Added a preference to use SSL, as it is experienced to cause high CPU usage on devices.
> * Corrected unit tests, LanLinkProvider is removed as it is nearly impossible to write it with current design. Will find a way
> 
> 
> Diffs
> -----
> 
>   AndroidManifest.xml 0da70ba 
>   build.gradle 6520c1c 
>   libs/conscrypt.jar PRE-CREATION 
>   libs/conscrypt.readme PRE-CREATION 
>   libs/conscrypt_jni.jar PRE-CREATION 
>   proguard-rules.pro ac9cda5 
>   res/values/strings.xml c128342 
>   src/org/kde/kdeconnect/Backends/BaseLink.java 579a7af 
>   src/org/kde/kdeconnect/Backends/BaseLinkProvider.java cfaf621 
>   src/org/kde/kdeconnect/Backends/BasePairingHandler.java PRE-CREATION 
>   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/BackgroundService.java 5e3d8c2 
>   src/org/kde/kdeconnect/Device.java 3262aef 
>   src/org/kde/kdeconnect/Helpers/SecurityHelpers/RsaHelper.java PRE-CREATION 
>   src/org/kde/kdeconnect/Helpers/SecurityHelpers/SslHelper.java PRE-CREATION 
>   src/org/kde/kdeconnect/NetworkPackage.java f62b5d0 
>   src/org/kde/kdeconnect/UserInterface/PairActivity.java 7a45751 
>   tests/org/kde/kdeconnect/DeviceTest.java 5d3383d 
>   tests/org/kde/kdeconnect/LanLinkProviderTest.java fe7863a 
>   tests/org/kde/kdeconnect/LanLinkTest.java d3d94c9 
>   tests/org/kde/kdeconnect/NetworkPackageTest.java a21114e 
> 
> Diff: https://git.reviewboard.kde.org/r/124140/diff/
> 
> 
> Testing
> -------
> 
> Tesed on some device with where both supports SSL, also with PC where ssl is not supported, working fine. Need a little bit more testing with more devices.
> 
> 
> Thanks,
> 
> Vineet Garg
> 
>

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


More information about the KDEConnect mailing list