{"action":"created","comment":{"url":"https://api.github.com/repos/certbot/certbot/pulls/comments/240865456","pull_request_review_id":182492712,"id":240865456,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDI0MDg2NTQ1Ng==","diff_hunk":"@@ -0,0 +1,307 @@\n+\"\"\"Compat module to handle files security on Windows and Linux\"\"\"\n+from __future__ import absolute_import\n+\n+import os\n+import stat\n+\n+try:\n+    import pwd  # pylint: disable=import-error\n+except ImportError:  # pragma: no cover\n+    pwd = None  # type: ignore","path":"certbot/compat/security.py","position":10,"original_position":10,"commit_id":"0d926623845c65b5d41dd326bb12750ddbb4b95c","original_commit_id":"0d926623845c65b5d41dd326bb12750ddbb4b95c","user":{"login":"ohemorange","id":1227205,"node_id":"MDQ6VXNlcjEyMjcyMDU=","avatar_url":"https://avatars0.githubusercontent.com/u/1227205?v=4","gravatar_id":"","url":"https://api.github.com/users/ohemorange","html_url":"https://github.com/ohemorange","followers_url":"https://api.github.com/users/ohemorange/followers","following_url":"https://api.github.com/users/ohemorange/following{/other_user}","gists_url":"https://api.github.com/users/ohemorange/gists{/gist_id}","starred_url":"https://api.github.com/users/ohemorange/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ohemorange/subscriptions","organizations_url":"https://api.github.com/users/ohemorange/orgs","repos_url":"https://api.github.com/users/ohemorange/repos","events_url":"https://api.github.com/users/ohemorange/events{/privacy}","received_events_url":"https://api.github.com/users/ohemorange/received_events","type":"User","site_admin":false},"body":"mypy has an optional type, does that not work here?","created_at":"2018-12-12T02:41:45Z","updated_at":"2018-12-12T02:43:47Z","html_url":"https://github.com/certbot/certbot/pull/6497#discussion_r240865456","pull_request_url":"https://api.github.com/repos/certbot/certbot/pulls/6497","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/certbot/certbot/pulls/comments/240865456"},"html":{"href":"https://github.com/certbot/certbot/pull/6497#discussion_r240865456"},"pull_request":{"href":"https://api.github.com/repos/certbot/certbot/pulls/6497"}}},"pull_request":{"url":"https://api.github.com/repos/certbot/certbot/pulls/6497","id":230300242,"node_id":"MDExOlB1bGxSZXF1ZXN0MjMwMzAwMjQy","html_url":"https://github.com/certbot/certbot/pull/6497","diff_url":"https://github.com/certbot/certbot/pull/6497.diff","patch_url":"https://github.com/certbot/certbot/pull/6497.patch","issue_url":"https://api.github.com/repos/certbot/certbot/issues/6497","number":6497,"state":"open","locked":false,"title":"[Windows] Security model for files permissions","user":{"login":"adferrand","id":9728851,"node_id":"MDQ6VXNlcjk3Mjg4NTE=","avatar_url":"https://avatars0.githubusercontent.com/u/9728851?v=4","gravatar_id":"","url":"https://api.github.com/users/adferrand","html_url":"https://github.com/adferrand","followers_url":"https://api.github.com/users/adferrand/followers","following_url":"https://api.github.com/users/adferrand/following{/other_user}","gists_url":"https://api.github.com/users/adferrand/gists{/gist_id}","starred_url":"https://api.github.com/users/adferrand/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/adferrand/subscriptions","organizations_url":"https://api.github.com/users/adferrand/orgs","repos_url":"https://api.github.com/users/adferrand/repos","events_url":"https://api.github.com/users/adferrand/events{/privacy}","received_events_url":"https://api.github.com/users/adferrand/received_events","type":"User","site_admin":false},"body":"This PR implements a security model to handle correctly the files permissions on Windows.\r\n\r\nThe complete specification is discussed in #6356, so I will not repeat its details here.\r\n\r\nEssentially, the goal is to provide appropriate DACLs on files generated by Certbot to ensure that sensible files and directories cannot be seen on Windows by everyone, but only by specific users. More generally, the implementation here translates POSIX modes (like 0644 or 0700) into DACLs that make sense for Windows.\r\n\r\nFor instance, 0700 will lead to file that is readable/editable/executable only by the user who runs Certbot and the Administrators.\r\n\r\nWhat I will add here is the impact of this security model for Certbot development.\r\n\r\nIndeed, we are talking about critical matter, as private keys for instance should never be accessible by normal users. However, Python, and in particular its standard `os` module, has a very poor support of Windows file permissions mechanism. The direct usage of `os` may silently make files exposable to anyone. In particular, wrong usages of `os.mkdir` or `os.chmod` could have disastrous consequences on the security model.\r\n\r\nSo I created a module, `certbot.compat.os`, that is intended to replace `os` in order to protect the security model using relevant wrappers around critical functions. Appart that, `certbot.compat.os` exposes any method from `os` and can be used as a valid replacement.\r\n\r\nOf course, any call to `os` has been replaced by `certbot.compat.os` in this PR. But it is not sufficient. Indeed in the future a contributor that is not aware of Windows particularities may use inappropriatly the `os` module, and break the security without the tests or the code reviewers notice it.\r\n\r\nAs a consequence, after discussions with @bmw, I decided to enforce during the CI to never use the standard `os` module in certbot projects (except `acme`). To do that, a new script `tools/enforce-coding-style.py` replaces the current actions on `lint` tox environment. This script will launch `pylint` and fail if errors are detected as usual. But it will also check if `os` is imported in any relevant python file, and fail if this kind of import is detected.\r\n\r\nSo any developer that would import directly `os` will make the tests fail and see with an appropriate report what is wrong.\r\n\r\nAppart that, `pywin32` dependency is added to certbot on Windows. This dependency provides binary builds starting with Python 3.5. Even if it is possible to compile `pywin32` for Python 3.4, it requires a full-fledged .NET environment. So for convenience and security (where `pywin32` is officially supported and tested), we should advertise that Certbot is only available with Python >= 3.5.\r\n\r\nFinally, certbot requires now administrative privileges for any operation except `--help`.","created_at":"2018-11-12T23:26:56Z","updated_at":"2018-12-12T02:43:46Z","closed_at":null,"merged_at":null,"merge_commit_sha":"a478f4827b524ebf1b2f0717e549a6a1495156fc","assignee":{"login":"ohemorange","id":1227205,"node_id":"MDQ6VXNlcjEyMjcyMDU=","avatar_url":"https://avatars0.githubusercontent.com/u/1227205?v=4","gravatar_id":"","url":"https://api.github.com/users/ohemorange","html_url":"https://github.com/ohemorange","followers_url":"https://api.github.com/users/ohemorange/followers","following_url":"https://api.github.com/users/ohemorange/following{/other_user}","gists_url":"https://api.github.com/users/ohemorange/gists{/gist_id}","starred_url":"https://api.github.com/users/ohemorange/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ohemorange/subscriptions","organizations_url":"https://api.github.com/users/ohemorange/orgs","repos_url":"https://api.github.com/users/ohemorange/repos","events_url":"https://api.github.com/users/ohemorange/events{/privacy}","received_events_url":"https://api.github.com/users/ohemorange/received_events","type":"User","site_admin":false},"assignees":[{"login":"ohemorange","id":1227205,"node_id":"MDQ6VXNlcjEyMjcyMDU=","avatar_url":"https://avatars0.githubusercontent.com/u/1227205?v=4","gravatar_id":"","url":"https://api.github.com/users/ohemorange","html_url":"https://github.com/ohemorange","followers_url":"https://api.github.com/users/ohemorange/followers","following_url":"https://api.github.com/users/ohemorange/following{/other_user}","gists_url":"https://api.github.com/users/ohemorange/gists{/gist_id}","starred_url":"https://api.github.com/users/ohemorange/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ohemorange/subscriptions","organizations_url":"https://api.github.com/users/ohemorange/orgs","repos_url":"https://api.github.com/users/ohemorange/repos","events_url":"https://api.github.com/users/ohemorange/events{/privacy}","received_events_url":"https://api.github.com/users/ohemorange/received_events","type":"User","site_admin":false}],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/certbot/certbot/pulls/6497/commits","review_comments_url":"https://api.github.com/repos/certbot/certbot/pulls/6497/comments","review_comment_url":"https://api.github.com/repos/certbot/certbot/pulls/comments{/number}","comments_url":"https://api.github.com/repos/certbot/certbot/issues/6497/comments","statuses_url":"https://api.github.com/repos/certbot/certbot/statuses/0d926623845c65b5d41dd326bb12750ddbb4b95c","head":{"label":"adferrand:windows-files-permissions","ref":"windows-files-permissions","sha":"0d926623845c65b5d41dd326bb12750ddbb4b95c","user":{"login":"adferrand","id":9728851,"node_id":"MDQ6VXNlcjk3Mjg4NTE=","avatar_url":"https://avatars0.githubusercontent.com/u/9728851?v=4","gravatar_id":"","url":"https://api.github.com/users/adferrand","html_url":"https://github.com/adferrand","followers_url":"https://api.github.com/users/adferrand/followers","following_url":"https://api.github.com/users/adferrand/following{/other_user}","gists_url":"https://api.github.com/users/adferrand/gists{/gist_id}","starred_url":"https://api.github.com/users/adferrand/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/adferrand/subscriptions","organizations_url":"https://api.github.com/users/adferrand/orgs","repos_url":"https://api.github.com/users/adferrand/repos","events_url":"https://api.github.com/users/adferrand/events{/privacy}","received_events_url":"https://api.github.com/users/adferrand/received_events","type":"User","site_admin":false},"repo":{"id":144460400,"node_id":"MDEwOlJlcG9zaXRvcnkxNDQ0NjA0MDA=","name":"certbot","full_name":"adferrand/certbot","private":false,"owner":{"login":"adferrand","id":9728851,"node_id":"MDQ6VXNlcjk3Mjg4NTE=","avatar_url":"https://avatars0.githubusercontent.com/u/9728851?v=4","gravatar_id":"","url":"https://api.github.com/users/adferrand","html_url":"https://github.com/adferrand","followers_url":"https://api.github.com/users/adferrand/followers","following_url":"https://api.github.com/users/adferrand/following{/other_user}","gists_url":"https://api.github.com/users/adferrand/gists{/gist_id}","starred_url":"https://api.github.com/users/adferrand/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/adferrand/subscriptions","organizations_url":"https://api.github.com/users/adferrand/orgs","repos_url":"https://api.github.com/users/adferrand/repos","events_url":"https://api.github.com/users/adferrand/events{/privacy}","received_events_url":"https://api.github.com/users/adferrand/received_events","type":"User","site_admin":false},"html_url":"https://github.com/adferrand/certbot","description":"Certbot is EFF's tool to obtain certs from Let's Encrypt and (optionally) auto-enable HTTPS on your server.  It can also act as a client for any other CA that uses the ACME protocol.","fork":true,"url":"https://api.github.com/repos/adferrand/certbot","forks_url":"https://api.github.com/repos/adferrand/certbot/forks","keys_url":"https://api.github.com/repos/adferrand/certbot/keys{/key_id}","collaborators_url":"https://api.github.com/repos/adferrand/certbot/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/adferrand/certbot/teams","hooks_url":"https://api.github.com/repos/adferrand/certbot/hooks","issue_events_url":"https://api.github.com/repos/adferrand/certbot/issues/events{/number}","events_url":"https://api.github.com/repos/adferrand/certbot/events","assignees_url":"https://api.github.com/repos/adferrand/certbot/assignees{/user}","branches_url":"https://api.github.com/repos/adferrand/certbot/branches{/branch}","tags_url":"https://api.github.com/repos/adferrand/certbot/tags","blobs_url":"https://api.github.com/repos/adferrand/certbot/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/adferrand/certbot/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/adferrand/certbot/git/refs{/sha}","trees_url":"https://api.github.com/repos/adferrand/certbot/git/trees{/sha}","statuses_url":"https://api.github.com/repos/adferrand/certbot/statuses/{sha}","languages_url":"https://api.github.com/repos/adferrand/certbot/languages","stargazers_url":"https://api.github.com/repos/adferrand/certbot/stargazers","contributors_url":"https://api.github.com/repos/adferrand/certbot/contributors","subscribers_url":"https://api.github.com/repos/adferrand/certbot/subscribers","subscription_url":"https://api.github.com/repos/adferrand/certbot/subscription","commits_url":"https://api.github.com/repos/adferrand/certbot/commits{/sha}","git_commits_url":"https://api.github.com/repos/adferrand/certbot/git/commits{/sha}","comments_url":"https://api.github.com/repos/adferrand/certbot/comments{/number}","issue_comment_url":"https://api.github.com/repos/adferrand/certbot/issues/comments{/number}","contents_url":"https://api.github.com/repos/adferrand/certbot/contents/{+path}","compare_url":"https://api.github.com/repos/adferrand/certbot/compare/{base}...{head}","merges_url":"https://api.github.com/repos/adferrand/certbot/merges","archive_url":"https://api.github.com/repos/adferrand/certbot/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/adferrand/certbot/downloads","issues_url":"https://api.github.com/repos/adferrand/certbot/issues{/number}","pulls_url":"https://api.github.com/repos/adferrand/certbot/pulls{/number}","milestones_url":"https://api.github.com/repos/adferrand/certbot/milestones{/number}","notifications_url":"https://api.github.com/repos/adferrand/certbot/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/adferrand/certbot/labels{/name}","releases_url":"https://api.github.com/repos/adferrand/certbot/releases{/id}","deployments_url":"https://api.github.com/repos/adferrand/certbot/deployments","created_at":"2018-08-12T11:17:30Z","updated_at":"2018-12-11T17:34:00Z","pushed_at":"2018-12-11T23:27:39Z","git_url":"git://github.com/adferrand/certbot.git","ssh_url":"git@github.com:adferrand/certbot.git","clone_url":"https://github.com/adferrand/certbot.git","svn_url":"https://github.com/adferrand/certbot","homepage":"","size":18270,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"other","name":"Other","spdx_id":"NOASSERTION","url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"certbot:master","ref":"master","sha":"353d09258575a5a8bfcfe7cb145834f787f2b7bb","user":{"login":"certbot","id":17889013,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE3ODg5MDEz","avatar_url":"https://avatars2.githubusercontent.com/u/17889013?v=4","gravatar_id":"","url":"https://api.github.com/users/certbot","html_url":"https://github.com/certbot","followers_url":"https://api.github.com/users/certbot/followers","following_url":"https://api.github.com/users/certbot/following{/other_user}","gists_url":"https://api.github.com/users/certbot/gists{/gist_id}","starred_url":"https://api.github.com/users/certbot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/certbot/subscriptions","organizations_url":"https://api.github.com/users/certbot/orgs","repos_url":"https://api.github.com/users/certbot/repos","events_url":"https://api.github.com/users/certbot/events{/privacy}","received_events_url":"https://api.github.com/users/certbot/received_events","type":"Organization","site_admin":false},"repo":{"id":26516210,"node_id":"MDEwOlJlcG9zaXRvcnkyNjUxNjIxMA==","name":"certbot","full_name":"certbot/certbot","private":false,"owner":{"login":"certbot","id":17889013,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE3ODg5MDEz","avatar_url":"https://avatars2.githubusercontent.com/u/17889013?v=4","gravatar_id":"","url":"https://api.github.com/users/certbot","html_url":"https://github.com/certbot","followers_url":"https://api.github.com/users/certbot/followers","following_url":"https://api.github.com/users/certbot/following{/other_user}","gists_url":"https://api.github.com/users/certbot/gists{/gist_id}","starred_url":"https://api.github.com/users/certbot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/certbot/subscriptions","organizations_url":"https://api.github.com/users/certbot/orgs","repos_url":"https://api.github.com/users/certbot/repos","events_url":"https://api.github.com/users/certbot/events{/privacy}","received_events_url":"https://api.github.com/users/certbot/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/certbot/certbot","description":"Certbot is EFF's tool to obtain certs from Let's Encrypt and (optionally) auto-enable HTTPS on your server.  It can also act as a client for any other CA that uses the ACME protocol.","fork":false,"url":"https://api.github.com/repos/certbot/certbot","forks_url":"https://api.github.com/repos/certbot/certbot/forks","keys_url":"https://api.github.com/repos/certbot/certbot/keys{/key_id}","collaborators_url":"https://api.github.com/repos/certbot/certbot/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/certbot/certbot/teams","hooks_url":"https://api.github.com/repos/certbot/certbot/hooks","issue_events_url":"https://api.github.com/repos/certbot/certbot/issues/events{/number}","events_url":"https://api.github.com/repos/certbot/certbot/events","assignees_url":"https://api.github.com/repos/certbot/certbot/assignees{/user}","branches_url":"https://api.github.com/repos/certbot/certbot/branches{/branch}","tags_url":"https://api.github.com/repos/certbot/certbot/tags","blobs_url":"https://api.github.com/repos/certbot/certbot/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/certbot/certbot/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/certbot/certbot/git/refs{/sha}","trees_url":"https://api.github.com/repos/certbot/certbot/git/trees{/sha}","statuses_url":"https://api.github.com/repos/certbot/certbot/statuses/{sha}","languages_url":"https://api.github.com/repos/certbot/certbot/languages","stargazers_url":"https://api.github.com/repos/certbot/certbot/stargazers","contributors_url":"https://api.github.com/repos/certbot/certbot/contributors","subscribers_url":"https://api.github.com/repos/certbot/certbot/subscribers","subscription_url":"https://api.github.com/repos/certbot/certbot/subscription","commits_url":"https://api.github.com/repos/certbot/certbot/commits{/sha}","git_commits_url":"https://api.github.com/repos/certbot/certbot/git/commits{/sha}","comments_url":"https://api.github.com/repos/certbot/certbot/comments{/number}","issue_comment_url":"https://api.github.com/repos/certbot/certbot/issues/comments{/number}","contents_url":"https://api.github.com/repos/certbot/certbot/contents/{+path}","compare_url":"https://api.github.com/repos/certbot/certbot/compare/{base}...{head}","merges_url":"https://api.github.com/repos/certbot/certbot/merges","archive_url":"https://api.github.com/repos/certbot/certbot/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/certbot/certbot/downloads","issues_url":"https://api.github.com/repos/certbot/certbot/issues{/number}","pulls_url":"https://api.github.com/repos/certbot/certbot/pulls{/number}","milestones_url":"https://api.github.com/repos/certbot/certbot/milestones{/number}","notifications_url":"https://api.github.com/repos/certbot/certbot/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/certbot/certbot/labels{/name}","releases_url":"https://api.github.com/repos/certbot/certbot/releases{/id}","deployments_url":"https://api.github.com/repos/certbot/certbot/deployments","created_at":"2014-11-12T02:52:20Z","updated_at":"2018-12-11T16:41:13Z","pushed_at":"2018-12-12T01:55:34Z","git_url":"git://github.com/certbot/certbot.git","ssh_url":"git@github.com:certbot/certbot.git","clone_url":"https://github.com/certbot/certbot.git","svn_url":"https://github.com/certbot/certbot","homepage":"","size":20480,"stargazers_count":23788,"watchers_count":23788,"language":"Python","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":2373,"mirror_url":null,"archived":false,"open_issues_count":896,"license":{"key":"other","name":"Other","spdx_id":"NOASSERTION","url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":2373,"open_issues":896,"watchers":23788,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/certbot/certbot/pulls/6497"},"html":{"href":"https://github.com/certbot/certbot/pull/6497"},"issue":{"href":"https://api.github.com/repos/certbot/certbot/issues/6497"},"comments":{"href":"https://api.github.com/repos/certbot/certbot/issues/6497/comments"},"review_comments":{"href":"https://api.github.com/repos/certbot/certbot/pulls/6497/comments"},"review_comment":{"href":"https://api.github.com/repos/certbot/certbot/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/certbot/certbot/pulls/6497/commits"},"statuses":{"href":"https://api.github.com/repos/certbot/certbot/statuses/0d926623845c65b5d41dd326bb12750ddbb4b95c"}},"author_association":"COLLABORATOR"}}
{"actor":{"display_login":"ohemorange"}}