{"action":"created","issue":{"url":"https://api.github.com/repos/passff/passff/issues/333","repository_url":"https://api.github.com/repos/passff/passff","labels_url":"https://api.github.com/repos/passff/passff/issues/333/labels{/name}","comments_url":"https://api.github.com/repos/passff/passff/issues/333/comments","events_url":"https://api.github.com/repos/passff/passff/issues/333/events","html_url":"https://github.com/passff/passff/pull/333","id":328366962,"node_id":"MDExOlB1bGxSZXF1ZXN0MTkxOTI0NzQ4","number":333,"title":"Check Domain Before Filling Passwords","user":{"login":"U8NWXD","id":19878639,"node_id":"MDQ6VXNlcjE5ODc4NjM5","avatar_url":"https://avatars3.githubusercontent.com/u/19878639?v=4","gravatar_id":"","url":"https://api.github.com/users/U8NWXD","html_url":"https://github.com/U8NWXD","followers_url":"https://api.github.com/users/U8NWXD/followers","following_url":"https://api.github.com/users/U8NWXD/following{/other_user}","gists_url":"https://api.github.com/users/U8NWXD/gists{/gist_id}","starred_url":"https://api.github.com/users/U8NWXD/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/U8NWXD/subscriptions","organizations_url":"https://api.github.com/users/U8NWXD/orgs","repos_url":"https://api.github.com/users/U8NWXD/repos","events_url":"https://api.github.com/users/U8NWXD/events{/privacy}","received_events_url":"https://api.github.com/users/U8NWXD/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-06-01T03:20:29Z","updated_at":"2018-06-10T15:07:55Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/passff/passff/pulls/333","html_url":"https://github.com/passff/passff/pull/333","diff_url":"https://github.com/passff/passff/pull/333.diff","patch_url":"https://github.com/passff/passff/pull/333.patch"},"body":"# Purpose\r\nOne security benefit of using password managers like `pass` is that, when they\r\nare integrated with the browser through extensions like `passff`, they can make\r\nphishing more difficult by comparing domains before filling passwords. Even if\r\nthe user is fooled into logging in to `examp1e.com` instead of `example.com`,\r\nthe extension can notice the difference and alert the user.\r\n\r\n# `PassFF` Implementation\r\nAs of now, `PassFF` lacks this functionality and will fill any credentials the\r\nuser selects, even if domains do not match. This PR provides a very much\r\nunpolished checking mechanism that warns the user in the following scenarios:\r\n\r\n## When User is Warned\r\n* If the domain of the current site and of the URL in the password database differ\r\n  * Note that the current implementation does not correctly handle\r\n  top-level-domains (TLDs) with multiple dot-separated strings like `.co.uk`.\r\n  Because the TLD is assumed to be only one dot-separated string like `.com`,\r\n  `www.example.co.uk` is interpreted as having a domain of `.co.uk` instead of\r\n  the correct `example.co.uk`.\r\n* If the current page URL is not served over HTTPS\r\n  * This is again imperfect as the current implementation only checks the URL\r\n  of the page, not the configuration of the login form itself, which could still\r\n  be sending passwords in plaintext.\r\n* If the URL in the password database is not HTTPS. This is a softer warning, as\r\nthere is no imminent security threat in this case, but keeping the URLs in the\r\ndatabase explicitly HTTPS helps protect the user in case they follow those links\r\nto access websites that do not automatically redirect to secure pages.\r\n* If there is an error parsing URLs. This can happen if `example.com` is stored\r\nin the database instead of `https://www.example.com` as the URL constructor\r\nexpects. This simply serves to warn the user that anti-phishing protections will\r\nbe bypassed to avoid giving a false sense of security.\r\n\r\n# Audience\r\nThe implementation in this PR is probably too strict for most users, so it can\r\nbe toggled from the extension's preferences and is off by default. This\r\npreserves usability for a wider audience while accommodating users who want more\r\nstrict security settings. In addition, the necessary UI strings have not been\r\ntranslated, making these changes of limited use to non-English speakers. As\r\nsuch, I recognize that these changes may not be ready for deployment. However,\r\nI would appreciate feedback and advice on how they might be improved.\r\n\r\n# Still To Do\r\n* Translate UI strings\r\n* Parse incomplete domains like `google.com`\r\n* Offer to automatically correct stored `http` links\r\n* Make pop-up dialogs more aesthetic than the default `alert()` and `confirm()`\r\ndialogs.\r\n  * In the process, redesign the override confirmation dialogs to discourage\r\n  overrides\r\n\r\n# Potential Future Work\r\n* Handle two-dot TLDs\r\n* Prevent other password exfiltration attacks\r\n  * Forms sending passwords insecurely\r\n  * Forms with different destinations than before\r\n\r\n# Attributions\r\n* [Password Managers: Attacks and Defenses](https://www.usenix.org/conference/usenixsecurity14/technical-sessions/presentation/silver)\r\n  * Authors:\r\n    * Stanford University: David Silver, Suman Jana, and Dan Boneh\r\n    * Carnegie Mellon University: Eric Chen and Collin Jackson\r\n  * Publication: Proceedings of the 23rd USENIX Security Symposium\r\n    * August 20–22, 2014\r\n    * San Diego, CA\r\n  * ISBN 978-1-931971-15-7\r\n"},"comment":{"url":"https://api.github.com/repos/passff/passff/issues/comments/396056396","html_url":"https://github.com/passff/passff/pull/333#issuecomment-396056396","issue_url":"https://api.github.com/repos/passff/passff/issues/333","id":396056396,"node_id":"MDEyOklzc3VlQ29tbWVudDM5NjA1NjM5Ng==","user":{"login":"5bentz","id":10729484,"node_id":"MDQ6VXNlcjEwNzI5NDg0","avatar_url":"https://avatars1.githubusercontent.com/u/10729484?v=4","gravatar_id":"","url":"https://api.github.com/users/5bentz","html_url":"https://github.com/5bentz","followers_url":"https://api.github.com/users/5bentz/followers","following_url":"https://api.github.com/users/5bentz/following{/other_user}","gists_url":"https://api.github.com/users/5bentz/gists{/gist_id}","starred_url":"https://api.github.com/users/5bentz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/5bentz/subscriptions","organizations_url":"https://api.github.com/users/5bentz/orgs","repos_url":"https://api.github.com/users/5bentz/repos","events_url":"https://api.github.com/users/5bentz/events{/privacy}","received_events_url":"https://api.github.com/users/5bentz/received_events","type":"User","site_admin":false},"created_at":"2018-06-10T15:07:55Z","updated_at":"2018-06-10T15:07:55Z","author_association":"CONTRIBUTOR","body":"Thanks for the thorough PR! Well explained and I loved the reference (Password Managers: Attacks and Defenses).\r\n\r\n#### About the PR (functional)\r\n\r\nI understand this is a draft version of a domain check for PassFF, by watching the presentation on Password Managers, I noticed another **potential improvement** for this PR:\r\n\r\n- Following the example of *Secure Filling*, we could check the domain the form sends to (step 4). The implementation seems not as straightforward as the current check though.\r\n\r\n#### About the implementation\r\nJS is new to me but I'll make a review anyway :+1: "}}
{"actor":{"display_login":"5bentz"}}