{"action":"created","comment":{"url":"https://api.github.com/repos/pyca/tls/pulls/comments/70563301","id":70563301,"diff_hunk":"@@ -137,21 +144,24 @@\n \n ASN1Cert = Struct(\n     \"ASN1Cert\",\n-    UBInt32(\"length\"),   # TODO: Reject packets with length not in 1..2^24-1\n+    SizeWithin(UBInt32(\"length\"),\n+               min_size=1, max_size=2 ** 24 - 1),\n     Bytes(\"asn1_cert\", lambda ctx: ctx.length),\n )\n \n Certificate = Struct(\n-    \"Certificate\",  # TODO: Reject packets with length > 2 ** 24 - 1\n-    UBInt32(\"certificates_length\"),\n+    \"Certificate\",\n+    SizeWithin(UBInt32(\"certificates_length\"),\n+               min_size=1, max_size=2 ** 24 - 1),\n     Bytes(\"certificates_bytes\", lambda ctx: ctx.certificates_length),\n )\n \n Handshake = Struct(\n     \"Handshake\",\n     UBInt8(\"msg_type\"),\n-    UBInt24(\"length\"),  # TODO: Reject packets with length > 2 ** 24","path":"tls/_constructs.py","position":114,"original_position":121,"commit_id":"e7125c6be9d89913f241de00e41c9d98472ed9e6","original_commit_id":"3c992b304aa00c855085694945c15c694334fb34","user":{"login":"markrwilliams","id":704247,"avatar_url":"https://avatars.githubusercontent.com/u/704247?v=3","gravatar_id":"","url":"https://api.github.com/users/markrwilliams","html_url":"https://github.com/markrwilliams","followers_url":"https://api.github.com/users/markrwilliams/followers","following_url":"https://api.github.com/users/markrwilliams/following{/other_user}","gists_url":"https://api.github.com/users/markrwilliams/gists{/gist_id}","starred_url":"https://api.github.com/users/markrwilliams/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/markrwilliams/subscriptions","organizations_url":"https://api.github.com/users/markrwilliams/orgs","repos_url":"https://api.github.com/users/markrwilliams/repos","events_url":"https://api.github.com/users/markrwilliams/events{/privacy}","received_events_url":"https://api.github.com/users/markrwilliams/received_events","type":"User","site_admin":false},"body":"OK! I will do some digging!","created_at":"2016-07-13T04:33:48Z","updated_at":"2016-07-13T04:33:48Z","html_url":"https://github.com/pyca/tls/pull/88#discussion_r70563301","pull_request_url":"https://api.github.com/repos/pyca/tls/pulls/88","_links":{"self":{"href":"https://api.github.com/repos/pyca/tls/pulls/comments/70563301"},"html":{"href":"https://github.com/pyca/tls/pull/88#discussion_r70563301"},"pull_request":{"href":"https://api.github.com/repos/pyca/tls/pulls/88"}}},"pull_request":{"url":"https://api.github.com/repos/pyca/tls/pulls/88","id":77060970,"html_url":"https://github.com/pyca/tls/pull/88","diff_url":"https://github.com/pyca/tls/pull/88.diff","patch_url":"https://github.com/pyca/tls/pull/88.patch","issue_url":"https://api.github.com/repos/pyca/tls/issues/88","number":88,"state":"open","locked":false,"title":"Length validation","user":{"login":"markrwilliams","id":704247,"avatar_url":"https://avatars.githubusercontent.com/u/704247?v=3","gravatar_id":"","url":"https://api.github.com/users/markrwilliams","html_url":"https://github.com/markrwilliams","followers_url":"https://api.github.com/users/markrwilliams/followers","following_url":"https://api.github.com/users/markrwilliams/following{/other_user}","gists_url":"https://api.github.com/users/markrwilliams/gists{/gist_id}","starred_url":"https://api.github.com/users/markrwilliams/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/markrwilliams/subscriptions","organizations_url":"https://api.github.com/users/markrwilliams/orgs","repos_url":"https://api.github.com/users/markrwilliams/repos","events_url":"https://api.github.com/users/markrwilliams/events{/privacy}","received_events_url":"https://api.github.com/users/markrwilliams/received_events","type":"User","site_admin":false},"body":"This introduces `SizeAtMost`, `SizeAtLeast`, and `SizeWithin` validators.  They validate numeric fields.\r\n\r\nThe idea is that the the length field of any given TLS message would be wrapped in one of these validators.  There's a little song and dance to make this work with `TLSPrefixedArray`.\r\n\r\nA good thing about this approach is that an over-long message won't be parsed, as it will be rejected on the basis of its prefixed length (though the tests do use very long messages as a defensive measure.  This seems to have slowed them down and is worth discussing!).\r\n\r\nA bad thing about this approach is that the `ValidationErrors` include only the lengths that are wrong, not the contents.  This may make debugging harder.  I think the right solution will still validate actual size of the length fields, not the contents of what they measure, but provide more context about which construct the error occurred within.","created_at":"2016-07-12T04:48:19Z","updated_at":"2016-07-13T04:33:48Z","closed_at":null,"merged_at":null,"merge_commit_sha":"64a65aa61bbcf82b43cbb8f5a5c89dd13556ce5a","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/pyca/tls/pulls/88/commits","review_comments_url":"https://api.github.com/repos/pyca/tls/pulls/88/comments","review_comment_url":"https://api.github.com/repos/pyca/tls/pulls/comments{/number}","comments_url":"https://api.github.com/repos/pyca/tls/issues/88/comments","statuses_url":"https://api.github.com/repos/pyca/tls/statuses/e7125c6be9d89913f241de00e41c9d98472ed9e6","head":{"label":"markrwilliams:length-validation","ref":"length-validation","sha":"e7125c6be9d89913f241de00e41c9d98472ed9e6","user":{"login":"markrwilliams","id":704247,"avatar_url":"https://avatars.githubusercontent.com/u/704247?v=3","gravatar_id":"","url":"https://api.github.com/users/markrwilliams","html_url":"https://github.com/markrwilliams","followers_url":"https://api.github.com/users/markrwilliams/followers","following_url":"https://api.github.com/users/markrwilliams/following{/other_user}","gists_url":"https://api.github.com/users/markrwilliams/gists{/gist_id}","starred_url":"https://api.github.com/users/markrwilliams/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/markrwilliams/subscriptions","organizations_url":"https://api.github.com/users/markrwilliams/orgs","repos_url":"https://api.github.com/users/markrwilliams/repos","events_url":"https://api.github.com/users/markrwilliams/events{/privacy}","received_events_url":"https://api.github.com/users/markrwilliams/received_events","type":"User","site_admin":false},"repo":{"id":62269849,"name":"tls","full_name":"markrwilliams/tls","owner":{"login":"markrwilliams","id":704247,"avatar_url":"https://avatars.githubusercontent.com/u/704247?v=3","gravatar_id":"","url":"https://api.github.com/users/markrwilliams","html_url":"https://github.com/markrwilliams","followers_url":"https://api.github.com/users/markrwilliams/followers","following_url":"https://api.github.com/users/markrwilliams/following{/other_user}","gists_url":"https://api.github.com/users/markrwilliams/gists{/gist_id}","starred_url":"https://api.github.com/users/markrwilliams/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/markrwilliams/subscriptions","organizations_url":"https://api.github.com/users/markrwilliams/orgs","repos_url":"https://api.github.com/users/markrwilliams/repos","events_url":"https://api.github.com/users/markrwilliams/events{/privacy}","received_events_url":"https://api.github.com/users/markrwilliams/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/markrwilliams/tls","description":"A pure Python implementation of the Transport Layer Security protocol version 1.2, using existing libraries for crypto math.","fork":true,"url":"https://api.github.com/repos/markrwilliams/tls","forks_url":"https://api.github.com/repos/markrwilliams/tls/forks","keys_url":"https://api.github.com/repos/markrwilliams/tls/keys{/key_id}","collaborators_url":"https://api.github.com/repos/markrwilliams/tls/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/markrwilliams/tls/teams","hooks_url":"https://api.github.com/repos/markrwilliams/tls/hooks","issue_events_url":"https://api.github.com/repos/markrwilliams/tls/issues/events{/number}","events_url":"https://api.github.com/repos/markrwilliams/tls/events","assignees_url":"https://api.github.com/repos/markrwilliams/tls/assignees{/user}","branches_url":"https://api.github.com/repos/markrwilliams/tls/branches{/branch}","tags_url":"https://api.github.com/repos/markrwilliams/tls/tags","blobs_url":"https://api.github.com/repos/markrwilliams/tls/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/markrwilliams/tls/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/markrwilliams/tls/git/refs{/sha}","trees_url":"https://api.github.com/repos/markrwilliams/tls/git/trees{/sha}","statuses_url":"https://api.github.com/repos/markrwilliams/tls/statuses/{sha}","languages_url":"https://api.github.com/repos/markrwilliams/tls/languages","stargazers_url":"https://api.github.com/repos/markrwilliams/tls/stargazers","contributors_url":"https://api.github.com/repos/markrwilliams/tls/contributors","subscribers_url":"https://api.github.com/repos/markrwilliams/tls/subscribers","subscription_url":"https://api.github.com/repos/markrwilliams/tls/subscription","commits_url":"https://api.github.com/repos/markrwilliams/tls/commits{/sha}","git_commits_url":"https://api.github.com/repos/markrwilliams/tls/git/commits{/sha}","comments_url":"https://api.github.com/repos/markrwilliams/tls/comments{/number}","issue_comment_url":"https://api.github.com/repos/markrwilliams/tls/issues/comments{/number}","contents_url":"https://api.github.com/repos/markrwilliams/tls/contents/{+path}","compare_url":"https://api.github.com/repos/markrwilliams/tls/compare/{base}...{head}","merges_url":"https://api.github.com/repos/markrwilliams/tls/merges","archive_url":"https://api.github.com/repos/markrwilliams/tls/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/markrwilliams/tls/downloads","issues_url":"https://api.github.com/repos/markrwilliams/tls/issues{/number}","pulls_url":"https://api.github.com/repos/markrwilliams/tls/pulls{/number}","milestones_url":"https://api.github.com/repos/markrwilliams/tls/milestones{/number}","notifications_url":"https://api.github.com/repos/markrwilliams/tls/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/markrwilliams/tls/labels{/name}","releases_url":"https://api.github.com/repos/markrwilliams/tls/releases{/id}","deployments_url":"https://api.github.com/repos/markrwilliams/tls/deployments","created_at":"2016-06-30T01:37:16Z","updated_at":"2016-06-30T07:58:40Z","pushed_at":"2016-07-13T04:32:22Z","git_url":"git://github.com/markrwilliams/tls.git","ssh_url":"git@github.com:markrwilliams/tls.git","clone_url":"https://github.com/markrwilliams/tls.git","svn_url":"https://github.com/markrwilliams/tls","homepage":null,"size":209,"stargazers_count":1,"watchers_count":1,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":1,"default_branch":"master"}},"base":{"label":"pyca:master","ref":"master","sha":"76c5df8251fea4fd1939ead8574eeb9a9ab309fc","user":{"login":"pyca","id":5615737,"avatar_url":"https://avatars.githubusercontent.com/u/5615737?v=3","gravatar_id":"","url":"https://api.github.com/users/pyca","html_url":"https://github.com/pyca","followers_url":"https://api.github.com/users/pyca/followers","following_url":"https://api.github.com/users/pyca/following{/other_user}","gists_url":"https://api.github.com/users/pyca/gists{/gist_id}","starred_url":"https://api.github.com/users/pyca/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pyca/subscriptions","organizations_url":"https://api.github.com/users/pyca/orgs","repos_url":"https://api.github.com/users/pyca/repos","events_url":"https://api.github.com/users/pyca/events{/privacy}","received_events_url":"https://api.github.com/users/pyca/received_events","type":"Organization","site_admin":false},"repo":{"id":20925004,"name":"tls","full_name":"pyca/tls","owner":{"login":"pyca","id":5615737,"avatar_url":"https://avatars.githubusercontent.com/u/5615737?v=3","gravatar_id":"","url":"https://api.github.com/users/pyca","html_url":"https://github.com/pyca","followers_url":"https://api.github.com/users/pyca/followers","following_url":"https://api.github.com/users/pyca/following{/other_user}","gists_url":"https://api.github.com/users/pyca/gists{/gist_id}","starred_url":"https://api.github.com/users/pyca/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pyca/subscriptions","organizations_url":"https://api.github.com/users/pyca/orgs","repos_url":"https://api.github.com/users/pyca/repos","events_url":"https://api.github.com/users/pyca/events{/privacy}","received_events_url":"https://api.github.com/users/pyca/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/pyca/tls","description":"A pure Python implementation of the Transport Layer Security protocol version 1.2, using existing libraries for crypto math.","fork":false,"url":"https://api.github.com/repos/pyca/tls","forks_url":"https://api.github.com/repos/pyca/tls/forks","keys_url":"https://api.github.com/repos/pyca/tls/keys{/key_id}","collaborators_url":"https://api.github.com/repos/pyca/tls/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/pyca/tls/teams","hooks_url":"https://api.github.com/repos/pyca/tls/hooks","issue_events_url":"https://api.github.com/repos/pyca/tls/issues/events{/number}","events_url":"https://api.github.com/repos/pyca/tls/events","assignees_url":"https://api.github.com/repos/pyca/tls/assignees{/user}","branches_url":"https://api.github.com/repos/pyca/tls/branches{/branch}","tags_url":"https://api.github.com/repos/pyca/tls/tags","blobs_url":"https://api.github.com/repos/pyca/tls/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/pyca/tls/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/pyca/tls/git/refs{/sha}","trees_url":"https://api.github.com/repos/pyca/tls/git/trees{/sha}","statuses_url":"https://api.github.com/repos/pyca/tls/statuses/{sha}","languages_url":"https://api.github.com/repos/pyca/tls/languages","stargazers_url":"https://api.github.com/repos/pyca/tls/stargazers","contributors_url":"https://api.github.com/repos/pyca/tls/contributors","subscribers_url":"https://api.github.com/repos/pyca/tls/subscribers","subscription_url":"https://api.github.com/repos/pyca/tls/subscription","commits_url":"https://api.github.com/repos/pyca/tls/commits{/sha}","git_commits_url":"https://api.github.com/repos/pyca/tls/git/commits{/sha}","comments_url":"https://api.github.com/repos/pyca/tls/comments{/number}","issue_comment_url":"https://api.github.com/repos/pyca/tls/issues/comments{/number}","contents_url":"https://api.github.com/repos/pyca/tls/contents/{+path}","compare_url":"https://api.github.com/repos/pyca/tls/compare/{base}...{head}","merges_url":"https://api.github.com/repos/pyca/tls/merges","archive_url":"https://api.github.com/repos/pyca/tls/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/pyca/tls/downloads","issues_url":"https://api.github.com/repos/pyca/tls/issues{/number}","pulls_url":"https://api.github.com/repos/pyca/tls/pulls{/number}","milestones_url":"https://api.github.com/repos/pyca/tls/milestones{/number}","notifications_url":"https://api.github.com/repos/pyca/tls/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/pyca/tls/labels{/name}","releases_url":"https://api.github.com/repos/pyca/tls/releases{/id}","deployments_url":"https://api.github.com/repos/pyca/tls/deployments","created_at":"2014-06-17T13:44:42Z","updated_at":"2016-07-05T15:18:25Z","pushed_at":"2016-07-13T04:32:22Z","git_url":"git://github.com/pyca/tls.git","ssh_url":"git@github.com:pyca/tls.git","clone_url":"https://github.com/pyca/tls.git","svn_url":"https://github.com/pyca/tls","homepage":null,"size":223,"stargazers_count":66,"watchers_count":66,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":23,"mirror_url":null,"open_issues_count":17,"forks":23,"open_issues":17,"watchers":66,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/pyca/tls/pulls/88"},"html":{"href":"https://github.com/pyca/tls/pull/88"},"issue":{"href":"https://api.github.com/repos/pyca/tls/issues/88"},"comments":{"href":"https://api.github.com/repos/pyca/tls/issues/88/comments"},"review_comments":{"href":"https://api.github.com/repos/pyca/tls/pulls/88/comments"},"review_comment":{"href":"https://api.github.com/repos/pyca/tls/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/pyca/tls/pulls/88/commits"},"statuses":{"href":"https://api.github.com/repos/pyca/tls/statuses/e7125c6be9d89913f241de00e41c9d98472ed9e6"}}}}
{"actor":{"display_login":"markrwilliams"}}