{"action":"created","issue":{"url":"https://api.github.com/repos/theupdateframework/tuf/issues/781","repository_url":"https://api.github.com/repos/theupdateframework/tuf","labels_url":"https://api.github.com/repos/theupdateframework/tuf/issues/781/labels{/name}","comments_url":"https://api.github.com/repos/theupdateframework/tuf/issues/781/comments","events_url":"https://api.github.com/repos/theupdateframework/tuf/issues/781/events","html_url":"https://github.com/theupdateframework/tuf/pull/781","id":355793694,"node_id":"MDExOlB1bGxSZXF1ZXN0MjEyMjQxMzU5","number":781,"title":"Update TUF to handle HTTPS proxies","user":{"login":"trishankatdatadog","id":33133073,"node_id":"MDQ6VXNlcjMzMTMzMDcz","avatar_url":"https://avatars1.githubusercontent.com/u/33133073?v=4","gravatar_id":"","url":"https://api.github.com/users/trishankatdatadog","html_url":"https://github.com/trishankatdatadog","followers_url":"https://api.github.com/users/trishankatdatadog/followers","following_url":"https://api.github.com/users/trishankatdatadog/following{/other_user}","gists_url":"https://api.github.com/users/trishankatdatadog/gists{/gist_id}","starred_url":"https://api.github.com/users/trishankatdatadog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/trishankatdatadog/subscriptions","organizations_url":"https://api.github.com/users/trishankatdatadog/orgs","repos_url":"https://api.github.com/users/trishankatdatadog/repos","events_url":"https://api.github.com/users/trishankatdatadog/events{/privacy}","received_events_url":"https://api.github.com/users/trishankatdatadog/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":53,"created_at":"2018-08-30T23:53:42Z","updated_at":"2018-09-12T18:31:58Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/theupdateframework/tuf/pulls/781","html_url":"https://github.com/theupdateframework/tuf/pull/781","diff_url":"https://github.com/theupdateframework/tuf/pull/781.diff","patch_url":"https://github.com/theupdateframework/tuf/pull/781.patch"},"body":"**Fixes issue #**:\r\n\r\nUnfortunately, TUF fails to work via HTTPS proxies, whereas the `requests` library does.\r\n\r\nThe problem is this [well-intentioned but outdated code](https://github.com/theupdateframework/tuf/blob/98622521a3a6632d6e44f01f3158d308d0416296/tuf/download.py#L739).\r\n\r\nAs the comment hints, I had borrowed that VerifiedHTTPSConnection code from pip back then, because I knew that python2 back then was not verifying hostnames on SSL certs. Unfortunately, this code is now severely outdated.\r\n\r\n**Description of the changes being introduced by the pull request**:\r\n\r\nThe tuf.download module now uses the `requests` library. I have manually tested that it handles HTTPS proxies.\r\n\r\nChanges:\r\n- Install `requests` and its dependencies by default.\r\n- Change the networking code to use `requests` instead of custom, outdated networking code.\r\n- Remove option to specify custom SSL certs via a TUF setting. Instead, we depend on clients exporting the `REQUESTS_CA_BUNDLE` environment variable to `requests`.\r\n- Remove `tuf.settings.SLOW_START_GRACE_PERIOD`, as this no longer seems terribly useful, because `requests` allow for setting connect and / or read timeouts.\r\n- We no longer explicitly check for supported schemes (e.g., `http` and `https` only). This was originally done to prevent potentially unsafe requests to files on disk, but `requests` does not handle opening local files anyway.\r\n\r\nTODO:\r\n- [x] Unfortunately, a few slow retrieval attack tests are failing. I'm too tired right now to figure out what is going on.\r\n- [x] Use a separate `requests.Session` per repository mirror to avoid sharing state, and accidentally falling for unknown / unforeseen security issues.\r\n- [ ] Add unit tests to see whether: (1) hostnames are verified on SSL certs, and (2) HTTPS proxies are handled transparently\r\n- [x] Add version number to the TUF user agent\r\n- [ ] Add documentation about: (1) exporting the `REQUESTS_CA_BUNDLE` and / or `HTTPS_PROXY` variables in order to handle HTTPS proxies, and (2) ensuring that different sessions are used per hostname\r\n- [ ] Add coverage tests, if necessary\r\n\r\n**Please verify and check that the pull request fulfills the following\r\nrequirements**:\r\n\r\n- [x] The code follows the [Code Style Guidelines](https://github.com/secure-systems-lab/code-style-guidelines#code-style-guidelines)\r\n- [ ] Tests have been added for the bug fix or new feature\r\n- [ ] Docs have been added for the bug fix or new feature\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/theupdateframework/tuf/issues/comments/420752228","html_url":"https://github.com/theupdateframework/tuf/pull/781#issuecomment-420752228","issue_url":"https://api.github.com/repos/theupdateframework/tuf/issues/781","id":420752228,"node_id":"MDEyOklzc3VlQ29tbWVudDQyMDc1MjIyOA==","user":{"login":"awwad","id":13263928,"node_id":"MDQ6VXNlcjEzMjYzOTI4","avatar_url":"https://avatars3.githubusercontent.com/u/13263928?v=4","gravatar_id":"","url":"https://api.github.com/users/awwad","html_url":"https://github.com/awwad","followers_url":"https://api.github.com/users/awwad/followers","following_url":"https://api.github.com/users/awwad/following{/other_user}","gists_url":"https://api.github.com/users/awwad/gists{/gist_id}","starred_url":"https://api.github.com/users/awwad/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/awwad/subscriptions","organizations_url":"https://api.github.com/users/awwad/orgs","repos_url":"https://api.github.com/users/awwad/repos","events_url":"https://api.github.com/users/awwad/events{/privacy}","received_events_url":"https://api.github.com/users/awwad/received_events","type":"User","site_admin":false},"created_at":"2018-09-12T18:31:58Z","updated_at":"2018-09-12T18:31:58Z","author_association":"CONTRIBUTOR","body":"I added a test for SSL hostname verification and untrusted certs.\r\n\r\nAssuming all this passes now, I think I'm ready to merge. There have been some changes, so I could use a final eyeball on it in tandem with my own last check. @trishankatdatadog?\r\n\r\n(I also added an extra bit of commented-out remote testing, in case someone wants to do some diagnosis, say. No other TUF testing fails on a machine with no net access, so that's why this one is commented out.)\r\n\r\nNote that there is not testing to ensure that the behavior using a transparent HTTPS proxy is correct. That might be a little involved. I gather that the new behavior works for you, Trishank?"}}
{"actor":{"display_login":"awwad"}}