{"thread":{"id":"32953","subject":"[RFH/PATCH] imap-send: support SNI (RFC4366)","startedAt":"2013-02-21T00:18:04Z","lastAt":"2013-02-21T05:48:03Z","messageCount":3,"participants":["Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"209948","messageId":"7vbobey0xv.fsf@alter.siamese.dyndns.org","threadId":"32953","inReplyTo":null,"subject":"[RFH/PATCH] imap-send: support SNI (RFC4366)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-21T00:18:04Z","receivedAt":"2013-02-21T00:18:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"To talk to a site that serves multiple names on a single IP address,\nthe client needs to ask for the specific hostname it wants to talk\nto. Otherwise, the default certificate returned from the IP address\nmay not match that of the host we wanted to talk to.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * I need help from people on this patch in two areas:\n\n (1) I only tested this patch by connecting to https://googlemail.com/ \n     with\n\n     $ git -c imap.host=imaps://googlemail.com -c imap.port=443 imap-send <this-patch.txt\n\n     as it is the only site I knew clients needs to talk SNI to get\n     the right certificate to verify; of course the port does not\n     talk imap, and the only thing that is tested by that approach is\n     we successfully establish an SSL/TLS connection.  Without the\n     patch, we fail to verify the certificate (we get a cert that is\n     for another hostname that is hosted at the same IP address), and\n     with the patch, we successfully get the right one.\n\n     I would appreciate it if somebody knows an imap server that\n     needs SNI and runs an end-to-end test against that server.\n\n (2) I do not know if everybody has SSL_set_tslext_host_name() macro\n     defined, so this patch may be breaking build for people with\n     different versions of OpenSSL.\n\n imap-send.c | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 171c887..d9abd8b 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -370,6 +370,15 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve\n \t\treturn -1;\n \t}\n \n+\t/*\n+\t * SNI (RFC4366)\n+\t * OpenSSL does not document this function, but the implementation\n+\t * returns 1 on success, 0 on failure after calling SSLerr().\n+\t */\n+\tret = SSL_set_tlsext_host_name(sock->ssl, server.host);\n+\tif (ret != 1)\n+\t\twarning(\"SSL_set_tslext_host_name(%s) failed.\\n\", server.host);\n+\n \tret = SSL_connect(sock->ssl);\n \tif (ret <= 0) {\n \t\tsocket_perror(\"SSL_connect\", sock, ret);\n-- \n1.8.2.rc0.106.ga6e4a61\n"},{"id":"209955","messageId":"7vr4kaw7or.fsf@alter.siamese.dyndns.org","threadId":"32953","inReplyTo":"7vbobey0xv.fsf@alter.siamese.dyndns.org","subject":"Re: [RFH/PATCH] imap-send: support SNI (RFC4366)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-21T05:35:16Z","receivedAt":"2013-02-21T05:35:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> To talk to a site that serves multiple names on a single IP address,\n> the client needs to ask for the specific hostname it wants to talk\n> to. Otherwise, the default certificate returned from the IP address\n> may not match that of the host we wanted to talk to.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>\n>  * I need help from people on this patch in two areas:\n>\n>  (1) I only tested this patch by connecting to https://googlemail.com/ \n>      with\n> ...\n>      I would appreciate it if somebody knows an imap server that\n>      needs SNI and runs an end-to-end test against that server.\n> \n>  (2) I do not know if everybody has SSL_set_tslext_host_name() macro\n>      defined, so this patch may be breaking build for people with\n>      different versions of OpenSSL.\n\nWhat I queued for tonight replaces the posted patch with this\nversion in order to address (2) above.\n\n-- >8 --\nSubject: [PATCH] imap-send: support SNI (RFC4366)\n\nTo talk with some sites that serve multiple names on a single IP\naddress, the client needs to ask for the specific host it wants to\ntalk to.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n imap-send.c | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 171c887..ab2098a 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -370,6 +370,17 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve\n \t\treturn -1;\n \t}\n \n+#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME\n+\t/*\n+\t * SNI (RFC4366)\n+\t * OpenSSL does not document this function, but the implementation\n+\t * returns 1 on success, 0 on failure after calling SSLerr().\n+\t */\n+\tret = SSL_set_tlsext_host_name(sock->ssl, server.host);\n+\tif (ret != 1)\n+\t\twarning(\"SSL_set_tslext_host_name(%s) failed.\\n\", server.host);\n+#endif\n+\n \tret = SSL_connect(sock->ssl);\n \tif (ret <= 0) {\n \t\tsocket_perror(\"SSL_connect\", sock, ret);\n-- \n1.8.2.rc0.127.g4d5d7da\n"},{"id":"209956","messageId":"20130221054803.GB25943@sigill.intra.peff.net","threadId":"32953","inReplyTo":"7vr4kaw7or.fsf@alter.siamese.dyndns.org","subject":"Re: [RFH/PATCH] imap-send: support SNI (RFC4366)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-21T05:48:03Z","receivedAt":"2013-02-21T05:48:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 20, 2013 at 09:35:16PM -0800, Junio C Hamano wrote:\n\n>>  (2) I do not know if everybody has SSL_set_tslext_host_name() macro\n>>      defined, so this patch may be breaking build for people with\n>>      different versions of OpenSSL.\n> [...]\n>\n> +#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME\n> +\t/*\n> +\t * SNI (RFC4366)\n> +\t * OpenSSL does not document this function, but the implementation\n> +\t * returns 1 on success, 0 on failure after calling SSLerr().\n> +\t */\n> +\tret = SSL_set_tlsext_host_name(sock->ssl, server.host);\n> +\tif (ret != 1)\n> +\t\twarning(\"SSL_set_tslext_host_name(%s) failed.\\n\", server.host);\n> +#endif\n\nYes, I think this is the right macro to check. According to OpenSSL's\nCHANGES file, it was introduced between 0.9.8n and 1.0.0 (Mar 2010). But\nI note that the use of the same macro in libcurl dates to 2008. Curious.\n\nNote that you have a typo in your warning text (tslext) and an\nextra newline.\n\nAs far as testing goes, I don't have an SNI IMAP server handy, but I\nthink you can simulate one with \"openssl s_server\". It may be a good\nlong-term goal to test any ssl-specific code against that in our test\nsuite (on the other hand, most of the interesting stuff is https, where\nthe details are all handled by curl).\n\n-Peff\n"}]}