{"action":"created","comment":{"url":"https://api.github.com/repos/krb5/krb5/pulls/comments/51348544","id":51348544,"diff_hunk":"@@ -336,6 +336,31 @@ def ldap_add(dn, objectclass, attrs=[]):\n realm.kinit('canon', password('canon'))\n realm.kinit('alias', password('canon'), ['-C'])\n \n+# Test password history\n+def test_pwhist(nhist):\n+    output('Testing password history of size ' + `nhist`)\n+    princ = 'pwhistprinc' + `nhist`\n+    pol = 'pwhistpol'+`nhist`\n+    realm.run([kadminl, 'addpol', '-history', `nhist`, pol])\n+    realm.run([kadminl, 'addprinc', '-policy', pol, '-nokey', princ])\n+    # Test nhist number of passwords and make sure that all previous passwords\n+    # fail when trying to be changed to them.\n+    cpw = lambda x,y: realm.run([kadminl, 'cpw', '-pw', `x`, princ],\n+                                expected_code=y)\n+\n+    # Creates a combination of password attempts, with (x, 0) and ([0..x], 1)\n+    cmb = lambda x: izip(chain([x], islice(count(), x+1)),\n+                             chain([0], repeat(1)))\n+    # Tries passwords, with one password change that should succeed followed by\n+    # all previous passwords that should fail\n+    # (0, 0) (0, 1) (1, 0) (0, 1), (1, 1)...\n+    list(starmap(cpw, chain.from_iterable(imap(cmb, islice(count(), nhist)))))\n+\n+    # Test that keys are rotated out correctly\n+    cpw(nhist, 0)\n+    cpw(0, 0)\n+map(test_pwhist, imap(pow, repeat(2), range(5)))","path":"src/tests/t_kdb.py","position":36,"original_position":36,"commit_id":"7323f9f0e428ca7195a4be03e52ba97a340175ee","original_commit_id":"4dd9bad5a56db6b9dad29cbb1c53f60461df80b3","user":{"login":"greghudson","id":1131421,"avatar_url":"https://avatars.githubusercontent.com/u/1131421?v=3","gravatar_id":"","url":"https://api.github.com/users/greghudson","html_url":"https://github.com/greghudson","followers_url":"https://api.github.com/users/greghudson/followers","following_url":"https://api.github.com/users/greghudson/following{/other_user}","gists_url":"https://api.github.com/users/greghudson/gists{/gist_id}","starred_url":"https://api.github.com/users/greghudson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/greghudson/subscriptions","organizations_url":"https://api.github.com/users/greghudson/orgs","repos_url":"https://api.github.com/users/greghudson/repos","events_url":"https://api.github.com/users/greghudson/events{/privacy}","received_events_url":"https://api.github.com/users/greghudson/received_events","type":"User","site_admin":false},"body":"Here is what I would consider a more reasonable version of the same tests, in the same number of lines of Python code:\r\n\r\n    # Test password history.                                                    \r\n    def test_pwhist(nhist):\r\n        def cpw(n):\r\n            realm.run([kadminl, 'cpw', '-pw', str(n), princ])\r\n        def cpw_fail(n):\r\n            realm.run([kadminl, 'cpw', '-pw', str(n), princ], expected_code=1)\r\n        output('*** Testing password history of size %d\\n' % nhist)\r\n        princ = 'pwhistprinc' + str(nhist)\r\n        pol = 'pwhistpol' + str(nhist)\r\n        realm.run([kadminl, 'addpol', '-history', str(nhist), pol])\r\n        realm.run([kadminl, 'addprinc', '-policy', pol, '-nokey', princ])\r\n        for i in range(nhist):\r\n            # Set a password, then check that all previous passwords fail.      \r\n            cpw(i)\r\n            for j in range(i + 1):\r\n                cpw_fail(i)\r\n        # Set one more new password, and make sure the oldest key is            \r\n        # rotated out.                                                          \r\n        cpw(nhist)\r\n        # I would add: cpw_fail(1)\r\n        cpw(0)\r\n\r\n    for n in (1, 2, 4, 8, 16):\r\n        test_pwhist(n)\r\n\r\nI can understand being concerned by the complexity of add_to_history(); that function is grossly overcomplicated because it uses a circular queue for no reason.  (The old keys are marshalled and unmarshalled on each password change, which is O(n) in the number of old keys, so trying to keep the insertion cost to O(1) is pointless.)  I would be happy to see it rewritten to be simpler.    But for its current incarnation, running a big quadratic test doesn't hit any more of its edge cases than a small quadratic test.  To cover that function properly you would need to do things like change the password policy after loading up the principal with some old keys.","created_at":"2016-01-30T18:14:37Z","updated_at":"2016-01-30T18:14:37Z","html_url":"https://github.com/krb5/krb5/pull/399#discussion_r51348544","pull_request_url":"https://api.github.com/repos/krb5/krb5/pulls/399","_links":{"self":{"href":"https://api.github.com/repos/krb5/krb5/pulls/comments/51348544"},"html":{"href":"https://github.com/krb5/krb5/pull/399#discussion_r51348544"},"pull_request":{"href":"https://api.github.com/repos/krb5/krb5/pulls/399"}}},"pull_request":{"url":"https://api.github.com/repos/krb5/krb5/pulls/399","id":57255253,"html_url":"https://github.com/krb5/krb5/pull/399","diff_url":"https://github.com/krb5/krb5/pull/399.diff","patch_url":"https://github.com/krb5/krb5/pull/399.patch","issue_url":"https://api.github.com/repos/krb5/krb5/issues/399","number":399,"state":"open","locked":false,"title":"Implement password history in LDAP plugin [KRB-49]","user":{"login":"SaharahSarah","id":14116259,"avatar_url":"https://avatars.githubusercontent.com/u/14116259?v=3","gravatar_id":"","url":"https://api.github.com/users/SaharahSarah","html_url":"https://github.com/SaharahSarah","followers_url":"https://api.github.com/users/SaharahSarah/followers","following_url":"https://api.github.com/users/SaharahSarah/following{/other_user}","gists_url":"https://api.github.com/users/SaharahSarah/gists{/gist_id}","starred_url":"https://api.github.com/users/SaharahSarah/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SaharahSarah/subscriptions","organizations_url":"https://api.github.com/users/SaharahSarah/orgs","repos_url":"https://api.github.com/users/SaharahSarah/repos","events_url":"https://api.github.com/users/SaharahSarah/events{/privacy}","received_events_url":"https://api.github.com/users/SaharahSarah/received_events","type":"User","site_admin":false},"body":"The password history is stored in the kerberos LDAP schema attribute 'krbPwdHistory', with one history entry per attribute.  When the history is decoded the history entries are sorted by kvno in descending order, with the next replacement key set to the last key entry, or the key history entry with the smallest kvno.\r\n\r\nThis commit is based on code commited by Tomas Kuthan <tkuthan@gmail.com>.\r\n \r\nticket: [5889](https://krbdev.mit.edu/rt/Ticket/Display.html?id=5889)","created_at":"2016-01-26T19:00:44Z","updated_at":"2016-01-30T18:14:37Z","closed_at":null,"merged_at":null,"merge_commit_sha":"0727b78d706a7582a641fb931048ac5e677fbbb8","assignee":{"login":"SaharahSarah","id":14116259,"avatar_url":"https://avatars.githubusercontent.com/u/14116259?v=3","gravatar_id":"","url":"https://api.github.com/users/SaharahSarah","html_url":"https://github.com/SaharahSarah","followers_url":"https://api.github.com/users/SaharahSarah/followers","following_url":"https://api.github.com/users/SaharahSarah/following{/other_user}","gists_url":"https://api.github.com/users/SaharahSarah/gists{/gist_id}","starred_url":"https://api.github.com/users/SaharahSarah/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SaharahSarah/subscriptions","organizations_url":"https://api.github.com/users/SaharahSarah/orgs","repos_url":"https://api.github.com/users/SaharahSarah/repos","events_url":"https://api.github.com/users/SaharahSarah/events{/privacy}","received_events_url":"https://api.github.com/users/SaharahSarah/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/krb5/krb5/pulls/399/commits","review_comments_url":"https://api.github.com/repos/krb5/krb5/pulls/399/comments","review_comment_url":"https://api.github.com/repos/krb5/krb5/pulls/comments{/number}","comments_url":"https://api.github.com/repos/krb5/krb5/issues/399/comments","statuses_url":"https://api.github.com/repos/krb5/krb5/statuses/7323f9f0e428ca7195a4be03e52ba97a340175ee","head":{"label":"SaharahSarah:sarahday/ldap-pw-hist","ref":"sarahday/ldap-pw-hist","sha":"7323f9f0e428ca7195a4be03e52ba97a340175ee","user":{"login":"SaharahSarah","id":14116259,"avatar_url":"https://avatars.githubusercontent.com/u/14116259?v=3","gravatar_id":"","url":"https://api.github.com/users/SaharahSarah","html_url":"https://github.com/SaharahSarah","followers_url":"https://api.github.com/users/SaharahSarah/followers","following_url":"https://api.github.com/users/SaharahSarah/following{/other_user}","gists_url":"https://api.github.com/users/SaharahSarah/gists{/gist_id}","starred_url":"https://api.github.com/users/SaharahSarah/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SaharahSarah/subscriptions","organizations_url":"https://api.github.com/users/SaharahSarah/orgs","repos_url":"https://api.github.com/users/SaharahSarah/repos","events_url":"https://api.github.com/users/SaharahSarah/events{/privacy}","received_events_url":"https://api.github.com/users/SaharahSarah/received_events","type":"User","site_admin":false},"repo":{"id":47211184,"name":"krb5","full_name":"SaharahSarah/krb5","owner":{"login":"SaharahSarah","id":14116259,"avatar_url":"https://avatars.githubusercontent.com/u/14116259?v=3","gravatar_id":"","url":"https://api.github.com/users/SaharahSarah","html_url":"https://github.com/SaharahSarah","followers_url":"https://api.github.com/users/SaharahSarah/followers","following_url":"https://api.github.com/users/SaharahSarah/following{/other_user}","gists_url":"https://api.github.com/users/SaharahSarah/gists{/gist_id}","starred_url":"https://api.github.com/users/SaharahSarah/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SaharahSarah/subscriptions","organizations_url":"https://api.github.com/users/SaharahSarah/orgs","repos_url":"https://api.github.com/users/SaharahSarah/repos","events_url":"https://api.github.com/users/SaharahSarah/events{/privacy}","received_events_url":"https://api.github.com/users/SaharahSarah/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/SaharahSarah/krb5","description":"mirror of MIT krb5 repository","fork":true,"url":"https://api.github.com/repos/SaharahSarah/krb5","forks_url":"https://api.github.com/repos/SaharahSarah/krb5/forks","keys_url":"https://api.github.com/repos/SaharahSarah/krb5/keys{/key_id}","collaborators_url":"https://api.github.com/repos/SaharahSarah/krb5/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/SaharahSarah/krb5/teams","hooks_url":"https://api.github.com/repos/SaharahSarah/krb5/hooks","issue_events_url":"https://api.github.com/repos/SaharahSarah/krb5/issues/events{/number}","events_url":"https://api.github.com/repos/SaharahSarah/krb5/events","assignees_url":"https://api.github.com/repos/SaharahSarah/krb5/assignees{/user}","branches_url":"https://api.github.com/repos/SaharahSarah/krb5/branches{/branch}","tags_url":"https://api.github.com/repos/SaharahSarah/krb5/tags","blobs_url":"https://api.github.com/repos/SaharahSarah/krb5/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/SaharahSarah/krb5/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/SaharahSarah/krb5/git/refs{/sha}","trees_url":"https://api.github.com/repos/SaharahSarah/krb5/git/trees{/sha}","statuses_url":"https://api.github.com/repos/SaharahSarah/krb5/statuses/{sha}","languages_url":"https://api.github.com/repos/SaharahSarah/krb5/languages","stargazers_url":"https://api.github.com/repos/SaharahSarah/krb5/stargazers","contributors_url":"https://api.github.com/repos/SaharahSarah/krb5/contributors","subscribers_url":"https://api.github.com/repos/SaharahSarah/krb5/subscribers","subscription_url":"https://api.github.com/repos/SaharahSarah/krb5/subscription","commits_url":"https://api.github.com/repos/SaharahSarah/krb5/commits{/sha}","git_commits_url":"https://api.github.com/repos/SaharahSarah/krb5/git/commits{/sha}","comments_url":"https://api.github.com/repos/SaharahSarah/krb5/comments{/number}","issue_comment_url":"https://api.github.com/repos/SaharahSarah/krb5/issues/comments{/number}","contents_url":"https://api.github.com/repos/SaharahSarah/krb5/contents/{+path}","compare_url":"https://api.github.com/repos/SaharahSarah/krb5/compare/{base}...{head}","merges_url":"https://api.github.com/repos/SaharahSarah/krb5/merges","archive_url":"https://api.github.com/repos/SaharahSarah/krb5/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/SaharahSarah/krb5/downloads","issues_url":"https://api.github.com/repos/SaharahSarah/krb5/issues{/number}","pulls_url":"https://api.github.com/repos/SaharahSarah/krb5/pulls{/number}","milestones_url":"https://api.github.com/repos/SaharahSarah/krb5/milestones{/number}","notifications_url":"https://api.github.com/repos/SaharahSarah/krb5/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/SaharahSarah/krb5/labels{/name}","releases_url":"https://api.github.com/repos/SaharahSarah/krb5/releases{/id}","deployments_url":"https://api.github.com/repos/SaharahSarah/krb5/deployments","created_at":"2015-12-01T19:07:48Z","updated_at":"2016-01-14T17:22:46Z","pushed_at":"2016-01-29T19:47:10Z","git_url":"git://github.com/SaharahSarah/krb5.git","ssh_url":"git@github.com:SaharahSarah/krb5.git","clone_url":"https://github.com/SaharahSarah/krb5.git","svn_url":"https://github.com/SaharahSarah/krb5","homepage":null,"size":66032,"stargazers_count":0,"watchers_count":0,"language":"C","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":0,"default_branch":"master"}},"base":{"label":"krb5:master","ref":"master","sha":"83ed75feba32e46f736fcce0d96a0445f29b96c2","user":{"login":"krb5","id":523797,"avatar_url":"https://avatars.githubusercontent.com/u/523797?v=3","gravatar_id":"","url":"https://api.github.com/users/krb5","html_url":"https://github.com/krb5","followers_url":"https://api.github.com/users/krb5/followers","following_url":"https://api.github.com/users/krb5/following{/other_user}","gists_url":"https://api.github.com/users/krb5/gists{/gist_id}","starred_url":"https://api.github.com/users/krb5/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/krb5/subscriptions","organizations_url":"https://api.github.com/users/krb5/orgs","repos_url":"https://api.github.com/users/krb5/repos","events_url":"https://api.github.com/users/krb5/events{/privacy}","received_events_url":"https://api.github.com/users/krb5/received_events","type":"Organization","site_admin":false},"repo":{"id":4307702,"name":"krb5","full_name":"krb5/krb5","owner":{"login":"krb5","id":523797,"avatar_url":"https://avatars.githubusercontent.com/u/523797?v=3","gravatar_id":"","url":"https://api.github.com/users/krb5","html_url":"https://github.com/krb5","followers_url":"https://api.github.com/users/krb5/followers","following_url":"https://api.github.com/users/krb5/following{/other_user}","gists_url":"https://api.github.com/users/krb5/gists{/gist_id}","starred_url":"https://api.github.com/users/krb5/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/krb5/subscriptions","organizations_url":"https://api.github.com/users/krb5/orgs","repos_url":"https://api.github.com/users/krb5/repos","events_url":"https://api.github.com/users/krb5/events{/privacy}","received_events_url":"https://api.github.com/users/krb5/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/krb5/krb5","description":"mirror of MIT krb5 repository","fork":false,"url":"https://api.github.com/repos/krb5/krb5","forks_url":"https://api.github.com/repos/krb5/krb5/forks","keys_url":"https://api.github.com/repos/krb5/krb5/keys{/key_id}","collaborators_url":"https://api.github.com/repos/krb5/krb5/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/krb5/krb5/teams","hooks_url":"https://api.github.com/repos/krb5/krb5/hooks","issue_events_url":"https://api.github.com/repos/krb5/krb5/issues/events{/number}","events_url":"https://api.github.com/repos/krb5/krb5/events","assignees_url":"https://api.github.com/repos/krb5/krb5/assignees{/user}","branches_url":"https://api.github.com/repos/krb5/krb5/branches{/branch}","tags_url":"https://api.github.com/repos/krb5/krb5/tags","blobs_url":"https://api.github.com/repos/krb5/krb5/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/krb5/krb5/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/krb5/krb5/git/refs{/sha}","trees_url":"https://api.github.com/repos/krb5/krb5/git/trees{/sha}","statuses_url":"https://api.github.com/repos/krb5/krb5/statuses/{sha}","languages_url":"https://api.github.com/repos/krb5/krb5/languages","stargazers_url":"https://api.github.com/repos/krb5/krb5/stargazers","contributors_url":"https://api.github.com/repos/krb5/krb5/contributors","subscribers_url":"https://api.github.com/repos/krb5/krb5/subscribers","subscription_url":"https://api.github.com/repos/krb5/krb5/subscription","commits_url":"https://api.github.com/repos/krb5/krb5/commits{/sha}","git_commits_url":"https://api.github.com/repos/krb5/krb5/git/commits{/sha}","comments_url":"https://api.github.com/repos/krb5/krb5/comments{/number}","issue_comment_url":"https://api.github.com/repos/krb5/krb5/issues/comments{/number}","contents_url":"https://api.github.com/repos/krb5/krb5/contents/{+path}","compare_url":"https://api.github.com/repos/krb5/krb5/compare/{base}...{head}","merges_url":"https://api.github.com/repos/krb5/krb5/merges","archive_url":"https://api.github.com/repos/krb5/krb5/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/krb5/krb5/downloads","issues_url":"https://api.github.com/repos/krb5/krb5/issues{/number}","pulls_url":"https://api.github.com/repos/krb5/krb5/pulls{/number}","milestones_url":"https://api.github.com/repos/krb5/krb5/milestones{/number}","notifications_url":"https://api.github.com/repos/krb5/krb5/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/krb5/krb5/labels{/name}","releases_url":"https://api.github.com/repos/krb5/krb5/releases{/id}","deployments_url":"https://api.github.com/repos/krb5/krb5/deployments","created_at":"2012-05-12T16:01:32Z","updated_at":"2016-01-25T17:26:06Z","pushed_at":"2016-01-29T19:47:11Z","git_url":"git://github.com/krb5/krb5.git","ssh_url":"git@github.com:krb5/krb5.git","clone_url":"https://github.com/krb5/krb5.git","svn_url":"https://github.com/krb5/krb5","homepage":null,"size":66981,"stargazers_count":85,"watchers_count":85,"language":"C","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":109,"mirror_url":null,"open_issues_count":17,"forks":109,"open_issues":17,"watchers":85,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/krb5/krb5/pulls/399"},"html":{"href":"https://github.com/krb5/krb5/pull/399"},"issue":{"href":"https://api.github.com/repos/krb5/krb5/issues/399"},"comments":{"href":"https://api.github.com/repos/krb5/krb5/issues/399/comments"},"review_comments":{"href":"https://api.github.com/repos/krb5/krb5/pulls/399/comments"},"review_comment":{"href":"https://api.github.com/repos/krb5/krb5/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/krb5/krb5/pulls/399/commits"},"statuses":{"href":"https://api.github.com/repos/krb5/krb5/statuses/7323f9f0e428ca7195a4be03e52ba97a340175ee"}}}}