<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://reviewboard.kde.org/r/4907/">http://reviewboard.kde.org/r/4907/</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 5th, 2010, 5:42 p.m., <b>Ingo Klöcker</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;">> All account usernames on the site are stored as PASSWORD value in the FormData
> folder of Network KWallet with the key:
> accounts_SITE
> where SITE stands for host part of the URL.
I think this is a potential security problem. Let's say there are two completely different websites hosted on the same host like geocities.yahoo.com or as institutions (universities, institutes, etc.) offering personal websites for their members (like example.org/~joeuser). Let's further say both websites have a login form. If you have different usernames for both websites then you always have to double check that you do not select the wrong username because otherwise you would try to login to website A with username _and_ password of website B. Ouch! Moreover, if you use the same username for both websites then you are completely screwed because AFAIU your patch this is not supported resp. without noticing Konqueror might send the password of website A to website B.
Either I misunderstood what your patch does or your patch is IMHO unacceptable because of the above.</pre>
</blockquote>
<p>On August 9th, 2010, 2:32 p.m., <b>Carewolf</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;">Are you objecting to the sharing of username/password across multiple pages within the same host? I can see the potential usability issue if the default isn't remembered separate for each page, but I am not so sure of the security issue. If I remember correctly, when webpages are sharing the same host, domain and IP-adress, they are not protected from cross-scripting either and can also access the same cookies. They are for most purposes the same security domain. Though https will have to be treated differently.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes, that's what I'm objecting to. IMO it is a security issue, but it's true that the patch wouldn't really make it much worse because it's already bad enough due to the possibility of cross-site-scripting, cookies and what not. Ignore security still leaves us with a potential usability issue as you correctly point out.</pre>
<br />
<p>- Ingo</p>
<br />
<p>On August 5th, 2010, 12:33 p.m., Filip Brcic wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By Filip Brcic.</div>
<p style="color: grey;"><i>Updated 2010-08-05 12:33:43</i></p>
<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;">This patch adds support for storing multiple web-site accounts in KWallet when using Khtml-based browser (=Konqueror).
Originally Khtml saved whole forms (containing at least one password field) in KWallet as a map. Support for that behavior is still included, but new approach takes precedence.
Key features of this new approach are the following:
Login page is identified by having 2 form input fields, one that is plain text
field, and the other that is the password field. All other form types are
left to be handled by the original map-based approach.
Login on one site is universal on that site. That means that if you can login using
multiple URLs (like on bugs.kde.org), the login data will be stored only once
and used on all login pages.
All account usernames on the site are stored as PASSWORD value in the FormData
folder of Network KWallet with the key:
accounts_SITE
where SITE stands for host part of the URL.
All passwords are stored as PASSWORD value in the FormData folder of the
Network KWallet with the key:
account_SITE_USERNAME
When you come to the SITE's login page, the system offers you to autocomplete
username. When you select some username from the autocomplete list, the system
autofills the correct password.
No username if prefilled, but that could be easily done (for example, by
selecting the first username from the accounts list or by keeping record which
username was last used).</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;">I made this patch from the current kdelibs trunk checkout, yet I tested it also
on kdelibs 4.3.3 and it applies correctly (actually it doesn't since the
copyright comments don't apply correctly, but if you ignore those few errors as
they only relate to the comments, not the code, the patch does apply). Therefore, it is safe to
assume this patch works on all versions of kdelibs from 4.3.3 to trunk
(possibly with some older versions, but I didn't check that).
Also, I ran the patched kdelibs for two weeks and it works well.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=72317">72317</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdelibs/khtml/html/html_formimpl.h <span style="color: grey">(1159459)</span></li>
<li>/trunk/KDE/kdelibs/khtml/html/html_formimpl.cpp <span style="color: grey">(1159459)</span></li>
<li>/trunk/KDE/kdelibs/khtml/khtml_part.h <span style="color: grey">(1159459)</span></li>
<li>/trunk/KDE/kdelibs/khtml/khtml_part.cpp <span style="color: grey">(1159459)</span></li>
<li>/trunk/KDE/kdelibs/khtml/khtml_wallet_p.h <span style="color: grey">(1159459)</span></li>
<li>/trunk/KDE/kdelibs/khtml/rendering/render_form.cpp <span style="color: grey">(1159459)</span></li>
<li>/trunk/KDE/kdelibs/khtml/ui/passwordbar/storepassbar.h <span style="color: grey">(1159459)</span></li>
<li>/trunk/KDE/kdelibs/khtml/ui/passwordbar/storepassbar.cpp <span style="color: grey">(1159459)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4907/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://reviewboard.kde.org/r/4907/s/471/"><img src="http://reviewboard.kde.org/media/uploaded/images/2010/08/05/kwallet_passwords_400x100.png" style="border: 1px black solid;" alt="Here's how it looks like in KWallet" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>