{"action":"created","issue":{"url":"https://api.github.com/repos/mholt/caddy/issues/1821","repository_url":"https://api.github.com/repos/mholt/caddy","labels_url":"https://api.github.com/repos/mholt/caddy/issues/1821/labels{/name}","comments_url":"https://api.github.com/repos/mholt/caddy/issues/1821/comments","events_url":"https://api.github.com/repos/mholt/caddy/issues/1821/events","html_url":"https://github.com/mholt/caddy/pull/1821","id":249792251,"number":1821,"title":"tls: Fix OCSP stapling bug when certificate names overlap other certs","user":{"login":"mholt","id":1128849,"avatar_url":"https://avatars2.githubusercontent.com/u/1128849?v=4","gravatar_id":"","url":"https://api.github.com/users/mholt","html_url":"https://github.com/mholt","followers_url":"https://api.github.com/users/mholt/followers","following_url":"https://api.github.com/users/mholt/following{/other_user}","gists_url":"https://api.github.com/users/mholt/gists{/gist_id}","starred_url":"https://api.github.com/users/mholt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mholt/subscriptions","organizations_url":"https://api.github.com/users/mholt/orgs","repos_url":"https://api.github.com/users/mholt/repos","events_url":"https://api.github.com/users/mholt/events{/privacy}","received_events_url":"https://api.github.com/users/mholt/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/mholt/caddy/milestones/11","html_url":"https://github.com/mholt/caddy/milestone/11","labels_url":"https://api.github.com/repos/mholt/caddy/milestones/11/labels","id":2667792,"number":11,"title":"0.10.7","description":"","creator":{"login":"mholt","id":1128849,"avatar_url":"https://avatars2.githubusercontent.com/u/1128849?v=4","gravatar_id":"","url":"https://api.github.com/users/mholt","html_url":"https://github.com/mholt","followers_url":"https://api.github.com/users/mholt/followers","following_url":"https://api.github.com/users/mholt/following{/other_user}","gists_url":"https://api.github.com/users/mholt/gists{/gist_id}","starred_url":"https://api.github.com/users/mholt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mholt/subscriptions","organizations_url":"https://api.github.com/users/mholt/orgs","repos_url":"https://api.github.com/users/mholt/repos","events_url":"https://api.github.com/users/mholt/events{/privacy}","received_events_url":"https://api.github.com/users/mholt/received_events","type":"User","site_admin":false},"open_issues":5,"closed_issues":10,"state":"open","created_at":"2017-07-28T15:42:39Z","updated_at":"2017-08-12T21:17:29Z","due_on":null,"closed_at":null},"comments":0,"created_at":"2017-08-12T06:19:37Z","updated_at":"2017-08-23T18:25:53Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mholt/caddy/pulls/1821","html_url":"https://github.com/mholt/caddy/pull/1821","diff_url":"https://github.com/mholt/caddy/pull/1821.diff","patch_url":"https://github.com/mholt/caddy/pull/1821.patch"},"body":"https://caddy.community/t/random-ocsp-response-errors-for-random-clients/2473?u=matt\r\n\r\nCertificates are keyed by name in the cache, optimized for fast lookups\r\nduring TLS handshakes using SNI. A more \"correct\" way that is truly a\r\n1:1 would be to cache certificates by a hash of the leaf's DER bytes,\r\nbut this involves an extra index to maintain. So instead of that, we\r\nsimply choose to prevent overlap when keying certificates by server\r\nname. This avoids the ambiguity when updating OCSP staples, for instance.\r\n\r\n(The fillCertFromLeaf changes are just cleanup, not actually related to the bug.)"},"comment":{"url":"https://api.github.com/repos/mholt/caddy/issues/comments/324421539","html_url":"https://github.com/mholt/caddy/pull/1821#issuecomment-324421539","issue_url":"https://api.github.com/repos/mholt/caddy/issues/1821","id":324421539,"user":{"login":"mholt","id":1128849,"avatar_url":"https://avatars2.githubusercontent.com/u/1128849?v=4","gravatar_id":"","url":"https://api.github.com/users/mholt","html_url":"https://github.com/mholt","followers_url":"https://api.github.com/users/mholt/followers","following_url":"https://api.github.com/users/mholt/following{/other_user}","gists_url":"https://api.github.com/users/mholt/gists{/gist_id}","starred_url":"https://api.github.com/users/mholt/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mholt/subscriptions","organizations_url":"https://api.github.com/users/mholt/orgs","repos_url":"https://api.github.com/users/mholt/repos","events_url":"https://api.github.com/users/mholt/events{/privacy}","received_events_url":"https://api.github.com/users/mholt/received_events","type":"User","site_admin":false},"created_at":"2017-08-23T18:25:53Z","updated_at":"2017-08-23T18:25:53Z","body":"@slightfoot Thanks for the review!\r\n\r\n> I wonder though about overlap in regards to wildcard certificates.\r\n\r\nI don't think this will be a problem; only one certificate per name is mapped in the cache. Per my testing the wildcards were OK. Certificates are chosen based on most specific to least specific match, so wildcards will only be used if there's not another more specific matching certificate.\r\n\r\n> Also, about the use of certCacheMu without defer. In the odd situation where we attempt to recover but then the lock is still held.\r\n\r\nGood thing to notice, for sure. Defer doesn't run until the end of the function, I'd have to refactor to make that happen. But the loop is correct; that code shouldn't panic (famous last words, I know) and it doesn't return. Good opportunity for cleanup in the future perhaps.\r\n\r\nWith that, merging this change now!"}}
{"actor":{"display_login":"mholt"}}