<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/124140/">https://git.reviewboard.kde.org/r/124140/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 25th, 2015, 4:09 p.m. UTC, <b>Albert Vaca Cintora</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/124140/diff/6/?file=398262#file398262line567" style="color: black; font-weight: bold; text-decoration: underline;">src/org/kde/kdeconnect/Device.java</a>
<span style="font-weight: normal;">
(Diff revision 6)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> public void onPackageReceived(NetworkPackage np) {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">432</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="o">(</span><span class="n">np</span><span class="o">.</span><span class="na">getType</span><span class="o">().</span><span class="na">equals</span><span class="o">(</span><span class="n">NetworkPackage</span><span class="o">.</span><span class="na">PACKAGE_TYPE_PAIR</span><span class="o">))</span> <span class="o">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">511</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="o">(</span><span class="n">np</span><span class="o">.</span><span class="na">getType</span><span class="o">().</span><span class="na">equals</span><span class="o">(</span><span class="n">NetworkPackage</span><span class="o">.</span><span class="na">PACKAGE_TYPE_PAIR</span><span class="o">))</span> <span class="o">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Here if you receive an unencrypted package, it won't enter this "if" statement and you won't try to decrypt it, but the package will get passed to the Device anyway by this function. This means that your code accepts unencrypted packages, and that's a security problem.</pre>
</blockquote>
<p>On August 25th, 2015, 4:35 p.m. UTC, <b>Vineet Garg</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It is the job of lanlink to decrypt the package if it is received encrypted. Device never received encrypted package, it always receive unencypted packaged. After this it checked by device in these conditions. If if it is pair package then handled by this condition, else passed to plugins only if device is paired. Else unpair package is sent. I am not able to see any security problem here. Can you elaborate further ?</p></pre>
</blockquote>
<p>On August 25th, 2015, 4:40 p.m. UTC, <b>Albert Vaca Cintora</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">When Device.packageReceived gets called, we no longer have knowledge about if the package was originally encrypted or not, so Device can't know if it should trust the package or not. We want to make Device able to trust some unencrypted packages, but not all, and that's not possible right now. Can you add a function that is receivedUnpairedPackage or something like that to Device?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Device knows it should trust the package or not based on pair status. To add support to receive packages when device is not paired, I think this can be done at plugin level where device can ask from respective plugin isUnpairedPackageSupported and if so, it can pass the package to that plugin. Although it will also increase one source of spam</p></pre>
<br />
<p>- Vineet</p>
<br />
<p>On August 25th, 2015, 3:17 p.m. UTC, Vineet Garg wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for kdeconnect and Albert Vaca Cintora.</div>
<div>By Vineet Garg.</div>
<p style="color: grey;"><i>Updated Aug. 25, 2015, 3:17 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdeconnect-android
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">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.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Added support to setup links on SSL</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Links automatically removed if wrong certificate is sent</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Shows keys based on hash of certificate to check right certificates are received</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Added a preference to use SSL, as it is experienced to cause high CPU usage on devices.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Corrected unit tests, LanLinkProvider is removed as it is nearly impossible to write it with current design. Will find a way</li>
</ul></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>build.gradle <span style="color: grey">(b36cf14)</span></li>
<li>libs/netty-handler.jar <span style="color: grey">(PRE-CREATION)</span></li>
<li>proguard-rules.pro <span style="color: grey">(ac9cda5)</span></li>
<li>res/values/strings.xml <span style="color: grey">(c128342)</span></li>
<li>src/org/kde/kdeconnect/Backends/BaseLink.java <span style="color: grey">(53407f0)</span></li>
<li>src/org/kde/kdeconnect/Backends/BasePairingHandler.java <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/org/kde/kdeconnect/Backends/LanBackend/LanLink.java <span style="color: grey">(c070126)</span></li>
<li>src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java <span style="color: grey">(0cb1ee3)</span></li>
<li>src/org/kde/kdeconnect/Backends/LanBackend/LanPairingHandler.java <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackLink.java <span style="color: grey">(add92f8)</span></li>
<li>src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackLinkProvider.java <span style="color: grey">(bd9c41b)</span></li>
<li>src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackPairingHandler.java <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/org/kde/kdeconnect/BackgroundService.java <span style="color: grey">(37baddb)</span></li>
<li>src/org/kde/kdeconnect/Device.java <span style="color: grey">(a0b9392)</span></li>
<li>src/org/kde/kdeconnect/Helpers/SecurityHelpers/RsaHelper.java <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/org/kde/kdeconnect/Helpers/SecurityHelpers/SslHelper.java <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/org/kde/kdeconnect/NetworkPackage.java <span style="color: grey">(a4ef7d0)</span></li>
<li>src/org/kde/kdeconnect/UserInterface/DeviceActivity.java <span style="color: grey">(fe3c470)</span></li>
<li>src/org/kde/kdeconnect/UserInterface/PairActivity.java <span style="color: grey">(7a45751)</span></li>
<li>tests/org/kde/kdeconnect/DeviceTest.java <span style="color: grey">(5d3383d)</span></li>
<li>tests/org/kde/kdeconnect/LanLinkProviderTest.java <span style="color: grey">(0c1eb58)</span></li>
<li>tests/org/kde/kdeconnect/LanLinkTest.java <span style="color: grey">(d3d94c9)</span></li>
<li>tests/org/kde/kdeconnect/NetworkPackageTest.java <span style="color: grey">(a21114e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124140/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>