unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] gnutls: Add SNI support
@ 2014-11-24 12:17 Toke Høiland-Jørgensen
  2014-11-24 12:28 ` Thien-Thi Nguyen
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2014-11-24 12:17 UTC (permalink / raw)
  To: emacs-devel

Currently, Emacs does not support Server Name Identification. This means
that servers that host multiple SSL sites on the same IP will send the
wrong certificate to Emacs. In addition, some servers refuse connections
entirely if the SNI extension is not included in the client handshake.

This patch adds what I think is the required call into GnuTLS to add the
hostname information on the handshake. Unfortunately I have been unable
to actually test the patch, since I can't get the git trunk to compile.
However, I thought I'd post it anyway to maybe get someone else to have
a look.

Oh, and there's a bit of a hack in trying to detect whether the hostname
is an IPv4 or IPv6 literal (in which case the hostname shouldn't be
included in the handshake). Not sure if omitting that check entirely
will work, or if there's a better way to detect this case.

-Toke



diff --git a/src/gnutls.c b/src/gnutls.c
index 22e3aec..e8a6966 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -187,6 +187,9 @@ DEF_GNUTLS_FN (int, gnutls_x509_crt_get_key_id,
 DEF_GNUTLS_FN (const char*, gnutls_sec_param_get_name, (gnutls_sec_param_t));
 DEF_GNUTLS_FN (const char*, gnutls_sign_algorithm_get_name,
 	       (gnutls_sign_algorithm_t));
+DEF_GNUTLS_FN (int, gnutls_server_name_set, (gnutls_session_t,
+					     gnutls_server_name_type_t,
+					     const void *, size_t));
 
 static bool
 init_gnutls_functions (void)
@@ -263,6 +266,7 @@ init_gnutls_functions (void)
   LOAD_GNUTLS_FN (library, gnutls_x509_crt_get_key_id);
   LOAD_GNUTLS_FN (library, gnutls_sec_param_get_name);
   LOAD_GNUTLS_FN (library, gnutls_sign_algorithm_get_name);
+  LOAD_GNUTLS_FN (library, gnutls_server_name_set);
 
   max_log_level = global_gnutls_log_level;
 
@@ -335,6 +339,7 @@ init_gnutls_functions (void)
 #define fn_gnutls_x509_crt_get_key_id           gnutls_x509_crt_get_key_id
 #define fn_gnutls_sec_param_get_name            gnutls_sec_param_get_name
 #define fn_gnutls_sign_algorithm_get_name       gnutls_sign_algorithm_get_name
+#define fn_gnutls_server_name_set		gnutls_server_name_set
 
 #endif /* !WINDOWSNT */
 
@@ -1137,6 +1142,7 @@ one trustfile (usually a CA bundle).  */)
   char const *priority_string_ptr = "NORMAL"; /* default priority string.  */
   unsigned int peer_verification;
   char *c_hostname;
+  char *c; bool send_hostname = 0;
 
   /* Placeholders for the property list elements.  */
   Lisp_Object priority_string;
@@ -1375,6 +1381,22 @@ one trustfile (usually a CA bundle).  */)
   if (ret < GNUTLS_E_SUCCESS)
     return gnutls_make_error (ret);
 
+  /* Quick and dirty test of the hostname; shouldn't be an IP. If it
+     contains letters, we assume it's a hostname, unless it contains a
+     : in which case we assume it's a literal IPv6 address. */
+  for(c = c_hostname; c; c++) {
+    if(c >= 'a') send_hostname = 1;
+    if(c == ':') {send_hostname = 0; break;}
+  }
+
+  if(send_hostname) {
+    GNUTLS_LOG (1, max_log_level, "setting TLS hostname");
+    ret = fn_gnutls_server_name_set(state, GNUTLS_NAME_DNS, c_hostname, strlen(c_hostname));
+    if (ret < GNUTLS_E_SUCCESS)
+      return gnutls_make_error (ret);
+  }
+
+
   GNUTLS_INITSTAGE (proc) = GNUTLS_STAGE_CRED_SET;
   ret = emacs_gnutls_handshake (XPROCESS (proc));
   if (ret < GNUTLS_E_SUCCESS)



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] gnutls: Add SNI support
  2014-11-24 12:17 [PATCH] gnutls: Add SNI support Toke Høiland-Jørgensen
@ 2014-11-24 12:28 ` Thien-Thi Nguyen
  2014-11-24 13:08   ` Toke Høiland-Jørgensen
  2014-11-24 20:56   ` Florian Weimer
  2014-11-24 12:33 ` Jérémie Courrèges-Anglas
  2014-11-26 21:52 ` Lars Magne Ingebrigtsen
  2 siblings, 2 replies; 9+ messages in thread
From: Thien-Thi Nguyen @ 2014-11-24 12:28 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1105 bytes --]

() Toke Høiland-Jørgensen <toke@toke.dk>
() Mon, 24 Nov 2014 13:17:22 +0100

   +  char *c; bool send_hostname = 0;

      /* Placeholders for the property list elements.  */
      Lisp_Object priority_string;
   @@ -1375,6 +1381,22 @@ one trustfile (usually a CA bundle).  */)
      if (ret < GNUTLS_E_SUCCESS)
        return gnutls_make_error (ret);

   +  /* Quick and dirty test of the hostname; shouldn't be an IP. If it
   +     contains letters, we assume it's a hostname, unless it contains a
   +     : in which case we assume it's a literal IPv6 address. */
   +  for(c = c_hostname; c; c++) {
   +    if(c >= 'a') send_hostname = 1;
   +    if(c == ':') {send_hostname = 0; break;}
   +  }

Aside from the whitespace and brace placement, this code is buggy: ‘c’
is of type ‘char *’ and cannot be meaningfully compared directly to 'a'.

-- 
Thien-Thi Nguyen
   GPG key: 4C807502
   (if you're human and you know it)
      read my lisp: (responsep (questions 'technical)
                               (not (via 'mailing-list)))
                     => nil

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] gnutls: Add SNI support
  2014-11-24 12:17 [PATCH] gnutls: Add SNI support Toke Høiland-Jørgensen
  2014-11-24 12:28 ` Thien-Thi Nguyen
@ 2014-11-24 12:33 ` Jérémie Courrèges-Anglas
  2014-11-26 21:52 ` Lars Magne Ingebrigtsen
  2 siblings, 0 replies; 9+ messages in thread
From: Jérémie Courrèges-Anglas @ 2014-11-24 12:33 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: emacs-devel

Toke Høiland-Jørgensen <toke@toke.dk> writes:
[...]
> Oh, and there's a bit of a hack in trying to detect whether the hostname
> is an IPv4 or IPv6 literal (in which case the hostname shouldn't be
> included in the handshake). Not sure if omitting that check entirely
> will work, or if there's a better way to detect this case.

Better not omit the check.  The common trick is to use inet_pton(3) to
see if the param can be converted into an address.  Now this raises
portability issues, which may be solved by the inet_pton gnulib
module[1].

[1] https://www.gnu.org/software/gnulib/manual/html_node/inet_005fpton.html
-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] gnutls: Add SNI support
  2014-11-24 12:28 ` Thien-Thi Nguyen
@ 2014-11-24 13:08   ` Toke Høiland-Jørgensen
  2014-11-24 14:50     ` Lars Magne Ingebrigtsen
  2014-11-24 20:56   ` Florian Weimer
  1 sibling, 1 reply; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2014-11-24 13:08 UTC (permalink / raw)
  To: emacs-devel

Thien-Thi Nguyen <ttn@gnu.org> writes:

> Aside from the whitespace and brace placement, this code is buggy: ‘c’
> is of type ‘char *’ and cannot be meaningfully compared directly to
> 'a'.

Ah yes, was missing a pointer deref I guess. But as I noted, there's
probably a better way to check this.

Not sure I'm the right person to integrate this probably; can make it a
feature request instead? Please support SNI... :)

-Toke



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] gnutls: Add SNI support
  2014-11-24 13:08   ` Toke Høiland-Jørgensen
@ 2014-11-24 14:50     ` Lars Magne Ingebrigtsen
  0 siblings, 0 replies; 9+ messages in thread
From: Lars Magne Ingebrigtsen @ 2014-11-24 14:50 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: Paul Eggert, emacs-devel

Toke Høiland-Jørgensen <toke@toke.dk> writes:

> Not sure I'm the right person to integrate this probably; can make it a
> feature request instead? Please support SNI... :)

This is bug#18208.  Calling inet_pton sounds like a good idea to me, but
might require the gnulib module, as noted:

-------

* This function is missing on some platforms: HP-UX 11.00, OSF/1 4.0,
  Solaris 2.5.1, mingw, MSVC 9, Interix 3.5, BeOS. 
* This function is declared in <netdb.h> instead of <arpa/inet.h> on
  some platforms: NonStop Kernel. 
* This function is declared in <ws2tcpip.h>, with a POSIX incompatible
  declaration, on some platforms: MSVC 9 on Windows >= Vista. 

-------

Paul, any opinions here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] gnutls: Add SNI support
  2014-11-24 12:28 ` Thien-Thi Nguyen
  2014-11-24 13:08   ` Toke Høiland-Jørgensen
@ 2014-11-24 20:56   ` Florian Weimer
  1 sibling, 0 replies; 9+ messages in thread
From: Florian Weimer @ 2014-11-24 20:56 UTC (permalink / raw)
  To: emacs-devel

* Thien-Thi Nguyen:

>    +  /* Quick and dirty test of the hostname; shouldn't be an IP. If it
>    +     contains letters, we assume it's a hostname, unless it contains a
>    +     : in which case we assume it's a literal IPv6 address. */
>    +  for(c = c_hostname; c; c++) {
>    +    if(c >= 'a') send_hostname = 1;
>    +    if(c == ':') {send_hostname = 0; break;}
>    +  }
>
> Aside from the whitespace and brace placement, this code is buggy: ‘c’
> is of type ‘char *’ and cannot be meaningfully compared directly to 'a'.

And what about uppercase names?  Or domain names which do not contain
letters?

(I think to be absolutely correct, you need to track the kind of name
resolution you have performed on the name.)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] gnutls: Add SNI support
  2014-11-24 12:17 [PATCH] gnutls: Add SNI support Toke Høiland-Jørgensen
  2014-11-24 12:28 ` Thien-Thi Nguyen
  2014-11-24 12:33 ` Jérémie Courrèges-Anglas
@ 2014-11-26 21:52 ` Lars Magne Ingebrigtsen
  2014-11-26 22:12   ` Lars Magne Ingebrigtsen
  2014-11-27  9:01   ` Toke Høiland-Jørgensen
  2 siblings, 2 replies; 9+ messages in thread
From: Lars Magne Ingebrigtsen @ 2014-11-26 21:52 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: emacs-devel

Toke Høiland-Jørgensen <toke@toke.dk> writes:

> This patch adds what I think is the required call into GnuTLS to add the
> hostname information on the handshake. Unfortunately I have been unable
> to actually test the patch, since I can't get the git trunk to compile.
> However, I thought I'd post it anyway to maybe get someone else to have
> a look.

Thanks; I'm applying a version of your patch...

> Oh, and there's a bit of a hack in trying to detect whether the hostname
> is an IPv4 or IPv6 literal (in which case the hostname shouldn't be
> included in the handshake). Not sure if omitting that check entirely
> will work, or if there's a better way to detect this case.

... but I left this out.  Does it matter that we send the host name if
we've been given the URL https://01.02.03.04/ or whatever?  First of
all, you'd think that would be kinda rare.  Second of all, does the
library mind getting an IP address as the SNI?

If not, we could just do the SNI always without checking.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] gnutls: Add SNI support
  2014-11-26 21:52 ` Lars Magne Ingebrigtsen
@ 2014-11-26 22:12   ` Lars Magne Ingebrigtsen
  2014-11-27  9:01   ` Toke Høiland-Jørgensen
  1 sibling, 0 replies; 9+ messages in thread
From: Lars Magne Ingebrigtsen @ 2014-11-26 22:12 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: emacs-devel

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Second of all, does the library mind getting an IP address as the SNI?

Well, it's documented to not want IP addresses.  So I added a function
to check for that.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] gnutls: Add SNI support
  2014-11-26 21:52 ` Lars Magne Ingebrigtsen
  2014-11-26 22:12   ` Lars Magne Ingebrigtsen
@ 2014-11-27  9:01   ` Toke Høiland-Jørgensen
  1 sibling, 0 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2014-11-27  9:01 UTC (permalink / raw)
  To: Lars Magne Ingebrigtsen; +Cc: emacs-devel

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> Thanks; I'm applying a version of your patch...

Cool!

> Well, it's documented to not want IP addresses. So I added a function
> to check for that.

Yep, probably a good call.

Would test the change, but emacs trunk still won't compile on my box and
I'm stumped on debugging it. Code looks good to me, though :)

-Toke



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-11-27  9:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-24 12:17 [PATCH] gnutls: Add SNI support Toke Høiland-Jørgensen
2014-11-24 12:28 ` Thien-Thi Nguyen
2014-11-24 13:08   ` Toke Høiland-Jørgensen
2014-11-24 14:50     ` Lars Magne Ingebrigtsen
2014-11-24 20:56   ` Florian Weimer
2014-11-24 12:33 ` Jérémie Courrèges-Anglas
2014-11-26 21:52 ` Lars Magne Ingebrigtsen
2014-11-26 22:12   ` Lars Magne Ingebrigtsen
2014-11-27  9:01   ` Toke Høiland-Jørgensen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).