{"action":"created","issue":{"url":"https://api.github.com/repos/eidheim/Simple-Web-Server/issues/89","repository_url":"https://api.github.com/repos/eidheim/Simple-Web-Server","labels_url":"https://api.github.com/repos/eidheim/Simple-Web-Server/issues/89/labels{/name}","comments_url":"https://api.github.com/repos/eidheim/Simple-Web-Server/issues/89/comments","events_url":"https://api.github.com/repos/eidheim/Simple-Web-Server/issues/89/events","html_url":"https://github.com/eidheim/Simple-Web-Server/pull/89","id":192678197,"number":89,"title":"Added forced client certificate validation","user":{"login":"sebt3","id":814870,"avatar_url":"https://avatars.githubusercontent.com/u/814870?v=3","gravatar_id":"","url":"https://api.github.com/users/sebt3","html_url":"https://github.com/sebt3","followers_url":"https://api.github.com/users/sebt3/followers","following_url":"https://api.github.com/users/sebt3/following{/other_user}","gists_url":"https://api.github.com/users/sebt3/gists{/gist_id}","starred_url":"https://api.github.com/users/sebt3/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sebt3/subscriptions","organizations_url":"https://api.github.com/users/sebt3/orgs","repos_url":"https://api.github.com/users/sebt3/repos","events_url":"https://api.github.com/users/sebt3/events{/privacy}","received_events_url":"https://api.github.com/users/sebt3/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":16,"created_at":"2016-11-30T21:01:06Z","updated_at":"2016-12-18T11:48:42Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/eidheim/Simple-Web-Server/pulls/89","html_url":"https://github.com/eidheim/Simple-Web-Server/pull/89","diff_url":"https://github.com/eidheim/Simple-Web-Server/pull/89.diff","patch_url":"https://github.com/eidheim/Simple-Web-Server/pull/89.patch"},"body":"Hi,\r\n\r\nA few notes about this pull request.\r\nThe added else block is very usefull for debuging any ssl issues. It should probably either come in a #ifdef block or a configurable \"if\".\r\nssl_session_ctx_id could be randomized, although as far as I know there's no security issue at having a const this way.\r\nthe SSL_CTX_set_session_id_context line enable SSL session caching. It is mandatory for client certification. Performance wise it sound wise to enable it even if not using client cert (so the long ssl handshake is skipped)\r\nyour verify_file is then used to match the client certificate."},"comment":{"url":"https://api.github.com/repos/eidheim/Simple-Web-Server/issues/comments/267816881","html_url":"https://github.com/eidheim/Simple-Web-Server/pull/89#issuecomment-267816881","issue_url":"https://api.github.com/repos/eidheim/Simple-Web-Server/issues/89","id":267816881,"user":{"login":"eidheim","id":5269318,"avatar_url":"https://avatars.githubusercontent.com/u/5269318?v=3","gravatar_id":"","url":"https://api.github.com/users/eidheim","html_url":"https://github.com/eidheim","followers_url":"https://api.github.com/users/eidheim/followers","following_url":"https://api.github.com/users/eidheim/following{/other_user}","gists_url":"https://api.github.com/users/eidheim/gists{/gist_id}","starred_url":"https://api.github.com/users/eidheim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eidheim/subscriptions","organizations_url":"https://api.github.com/users/eidheim/orgs","repos_url":"https://api.github.com/users/eidheim/repos","events_url":"https://api.github.com/users/eidheim/events{/privacy}","received_events_url":"https://api.github.com/users/eidheim/received_events","type":"User","site_admin":false},"created_at":"2016-12-18T11:48:42Z","updated_at":"2016-12-18T11:48:42Z","body":"I'll work on the error reporting later, but setting the session_id_context should maybe be done somewhat like this:\r\n```diff\r\ndiff --git a/server_https.hpp b/server_https.hpp\r\nindex bbd7f79..4416b3c 100644\r\n--- a/server_https.hpp\r\n+++ b/server_https.hpp\r\n@@ -3,12 +3,15 @@\r\n \r\n #include \"server_http.hpp\"\r\n #include <boost/asio/ssl.hpp>\r\n+#include <random>\r\n+#include <openssl/ssl.h>\r\n \r\n namespace SimpleWeb {\r\n     typedef boost::asio::ssl::stream<boost::asio::ip::tcp::socket> HTTPS;    \r\n     \r\n     template<>\r\n     class Server<HTTPS> : public ServerBase<HTTPS> {\r\n+        unsigned char session_id_context[SSL_MAX_SSL_SESSION_ID_LENGTH];\r\n     public:\r\n         Server(unsigned short port, size_t num_threads, const std::string& cert_file, const std::string& private_key_file,\r\n                 long timeout_request=5, long timeout_content=300,\r\n@@ -18,8 +21,22 @@ namespace SimpleWeb {\r\n             context.use_certificate_chain_file(cert_file);\r\n             context.use_private_key_file(private_key_file, boost::asio::ssl::context::pem);\r\n             \r\n-            if(verify_file.size()>0)\r\n+            if(verify_file.size()>0) {\r\n                 context.load_verify_file(verify_file);\r\n+                context.set_verify_mode(boost::asio::ssl::verify_peer | boost::asio::ssl::verify_fail_if_no_peer_cert);\r\n+                \r\n+                // From https://wiki.openssl.org/index.php/Manual:SSL_CTX_set_session_id_context(3):\r\n+                // \"... each application must set its own session id context sid_ctx which is used to distinguish the contexts and is stored in exported sessions\"\r\n+                auto port_str=std::to_string(port)+' ';\r\n+                std::random_device rd;\r\n+                for(size_t c=0;c<SSL_MAX_SSL_SESSION_ID_LENGTH;++c) {\r\n+                    if(c<port_str.size())\r\n+                        session_id_context[c]=static_cast<unsigned char>(port_str[c]);\r\n+                    else\r\n+                        session_id_context[c]=static_cast<unsigned char>(rd());\r\n+                }\r\n+                SSL_CTX_set_session_id_context(context.native_handle(), session_id_context, SSL_MAX_SSL_SESSION_ID_LENGTH);\r\n+            }\r\n         }\r\n \r\n     protected:\r\n```\r\n\r\nWhat do you think?"}}
{"actor":{"display_login":"eidheim"}}