After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 588189 - TLS support for GSocket*
TLS support for GSocket*
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: network
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 573029 573142 581890 598852 (view as bug list)
Depends on: 634241
Blocks: 526582
 
 
Reported: 2009-07-09 19:16 UTC by Dan Winship
Modified: 2011-02-18 16:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
make GProxyConnection public, as GTcpWrapperConnection (28.08 KB, patch)
2010-11-07 20:02 UTC, Dan Winship
committed Details | Review
Add TLS (SSL) support to gio (148.53 KB, patch)
2010-11-07 20:03 UTC, Dan Winship
none Details | Review

Description Dan Winship 2009-07-09 19:16:28 UTC
OK, need to figure this out.

1. Can we make glib depend on GNUTLS? Assuming the answer is "no", gio extension points make it pretty easy to put the code somewhere else, so that's not that big a deal, other than the fact that we'd be adding another module. (Putting it into gvfs would not really be very good, since the code is needed on Windows as well.) Perhaps it could go into a module with libproxy-based proxy support for GSocket* (bug 580341).

bug 573142 has some code for this, though I haven't looked at it.

2. Do we expose TLS only at the GSocketConnection level, or at the GSocket level? I think GSocket, because (a) I need that in libsoup, (b) to the extent that there's any argument for any app using GSocket rather than GSocketConnection, those arguments apply to TLS connections just as much as plain connections, (c) the SSL libraries all expose a POSIX-like API anyway, so hooking them up to GSocket is much simpler than hooking them up to gio-style async APIs.

3. How fascist are we by default about bad certificates, and how much control over TLS do we expose in the API? bug 573029 discusses this in the context of the existing gnio gtls. 

We could have a compile-time-configured "default system trusted CA list" (eg, /etc/pki/tls/certs/ca-bundle.crt), and by default, trust only certs signed from that list, with valid dates and hostname, etc. And then have properties you can set to override that ("allow-expired", "trusted-ca-file", etc). (Alternatively, we could be like libsoup, and trust everything by default, and require you to set properties if you want to ever *not* trust someone.) For apps that want to present confusing dialogs to the user, we could just have an API that gives them the received certificate as an x509-encoded string, and they can do their own gnutls/openssl/NSS calls on it; this avoids us needing to expose GNUTLS-specific types in the API.

4. What is the general structure of the API? I'm thinking something like:

    tls = g_tls_thingy_new ();
    g_tls_thingy_set_trusted_ca_file (tls, path);
    g_tls_thingy_set_accept_expired (tls, TRUE);

    /* ... */

    g_output_stream_write (socket_out, "STARTTLS\r\n", ...);
    tls_connection = g_tls_socket_connection_new_from_connection (socket_connection, tls);
    g_object_unref (socket_connection);
    socket_out = g_io_connection_get_output_stream (tls_connection);

    /* ... */

    ssl_sock = g_tls_socket_new (tls, G_SOCKET_FAMILY_IPV4, G_SOCKET_TYPE_STREAM, 0, NULL);

ie: there's a "TLS object" that encapsulates generic TLS configuration, which can be used multiple times, and saves us from having to put duplicate TLS options on GSocket and GSocketConnection, and saves us from having to re-parse the CA file for every new connection. GSocket and GSocketConnection are oblivious to TLS, but have TLS-aware subclasses, which can be created either by wrapping an existing socket/connection, or else directly.

Although what happens if you create a plain GSocketConnection from a GTlsSocket? Hm... maybe we don't need GTlsSocketConnection, we can just have g_socket_connection_set_tls(), which would wrap its GSocket. Although maybe it would be confusing that this would cause the GSocketConnection's socket property to change... so maybe we don't want GTlsSocket either; we could just have g_socket_set_tls(), and GSocket would know to use some internal gtls API rather than calling send()/recv() directly when TLS was in use.
Comment 1 Dan Winship 2009-09-14 17:10:36 UTC
been hacking on this on the weekend lately. some more notes

(In reply to comment #0)
> 1. Can we make glib depend on GNUTLS? Assuming the answer is "no", gio
> extension points make it pretty easy to put the code somewhere else, so that's
> not that big a deal, other than the fact that we'd be adding another module.

Alex noted that for all of the other extension points, there is a base implementation in gio, and then additional implementations via the extension point (eg, GLocalFile vs GVfsFile). In this case the base implementation would basically be a no-op. There'd be some function you could call that would tell you whether or not TLS support was available, or if you didn't check that and just started making TLS calls, you'd always get back G_IO_ERROR_NOT_SUPPORTED or something.

> (Putting it into gvfs would not really be very good, since the code is needed
> on Windows as well.)

FWIW, it turns out that contrary to our earlier beliefs, Windows does actually have a built-in TLS API that can be used with winsock; the "Schannel" provider of the CryptoAPI. It is a bit lower level than the UNIX TLS APIs; it doesn't actually provide any transport semantics at all; you read from and write to the sockets yourself, using whatever APIs and I/O paradigms you prefer, and just use CryptoAPI calls to encode/decode the TLS messages. There are code samples on the web of using it with winsock though.

OS X also has a native TLS API, which works more like GNUTLS and OpenSSL; you give it socket-read and socket-write functions to use, and it provides a higher-level I/O API on top of them.

In both cases you have automatic access to the system CA lists, etc.

> 2. Do we expose TLS only at the GSocketConnection level, or at the GSocket
> level?

I'm making g_socket_send() and g_socket_receive() virtual and implementing a GTlsSocket subclass. However, trying to make handshaking be transparent at the GSocket level doesn't seem worth it. In particular, if sources returned from g_socket_create_source() can be reused multiple times (and nothing has said they can't be), then that means that the source created by a GTlsSocket would have to sometimes reverse the condition it was polling on. There doesn't seem to be any easy way to do that generically (meaning, working with both unix and windows GSockets) at the GTlsSocket level, so this would end up requiring hacks at the GSocket level. And even there the hacks are not simple. It's much easier to say that g_socket_receive() on a GTlsSocket will sometimes return G_TLS_ERROR_HANDSHAKE_REQUIRED, and then you have to call g_tls_socket_handshake(), and if the socket is non-blocking that may return G_TLS_ERROR_HANDSHAKE_NEED_READ or G_TLS_ERROR_HANDSHAKE_NEED_WRITE, and the caller has to do the right thing with that.

GTlsConnection, on the other hand, can hide all those details from its caller.

> 3. How fascist are we by default about bad certificates, and how much control
> over TLS do we expose in the API?

I'm thinking basically what I said above: the default should be to use a system-provided list of CAs (and a system-provided CRL if one exists), and to require certificates to validate, but you can call some method to set a weaker default, or implement a signal handler for one-off exceptions. Where needed, certificates are represented as DER-encoded binary blobs, which gio will (at least initially) not provide any methods for dealing with.

>     tls = g_tls_thingy_new ();
>     g_tls_thingy_set_trusted_ca_file (tls, path);
>     g_tls_thingy_set_accept_expired (tls, TRUE);

s/thingy/client/. There will be a base interface GTls with subclasses GTlsClient and GTlsServer, for encompassing client-side and server-side state respectively. (Though I have not thought a whole lot about the server-side stuff yet.)

>     tls_connection = g_tls_socket_connection_new_from_connection
> (socket_connection, tls);
...
>     ssl_sock = g_tls_socket_new (tls, G_SOCKET_FAMILY_IPV4,
> G_SOCKET_TYPE_STREAM, 0, NULL);

"tls" could be either a GTlsClient or a GTlsServer here. For the GTlsConnection case, we actually want to use GInitable and GAsyncInitable, because you want it to do the handshake when you create the connection object. (For GTlsSocket you have to do that by hand yourself.)

There will probably also be "g_socket_client_set_tls(client, TRUE)", to tell your GSocketClient that you want it to wrap sockets in GTlsConnections rather than GTcpConnections automatically. (Also, for the proxy support bug, there will probably end up being "g_socket_client_connect_to_url{,_async}", which could also automatically infer TLS as appropriate.)

(Hm... if we have GSctpConnection in the future then will we need two different GTlsConnection types too? Actually TLS over SCTP sounds "exciting"...)

> Although what happens if you create a plain GSocketConnection from a
> GTlsSocket?

Answer: it mostly works, except that when you g_io_stream_close() it, it doesn't send a TLS Finished message, and if the server requests a rehandshake, then the application will receive a G_TLS_ERROR_HANDSHAKE_REQUIRED that it wasn't expecting. So, don't do that.

> so maybe we don't want GTlsSocket either; we could just
> have g_socket_set_tls(), and GSocket would know to use some internal gtls API
> rather than calling send()/recv() directly when TLS was in use.

I went back and forth on this a few times while I was still trying to make handshaking work transparently; the problem is that you need to have the gtls-instead-of-send/recv exception in both the unix and windows codepaths, which adds a few more places for bugs to sneak in, and then I thought "well what we really need is a GLowLevelSocket, with implementations for unix, windows, and gtls, and then everything will just work", except that that's silly because GSocket is already supposed to be low level, so that's when I decided we probably shouldn't try to make handshaking be transparent at the GSocket level.
Comment 2 Tor Lillqvist 2009-09-19 09:03:46 UTC
> it turns out that contrary to our earlier beliefs, Windows does
> actually have a built-in TLS API that can be used with winsock

Wow, interesting!
Comment 3 Philip Van Hoof 2009-10-18 20:03:36 UTC
Given the usefulness of TLS support in GIO, feel free to publish experimental git repositories, Dan. We might actually experiment a bit with your work :), and that way get you some feedback on the usability of it.
Comment 4 Javier Jardón (IRC: jjardon) 2009-10-22 04:36:47 UTC
*** Bug 598852 has been marked as a duplicate of this bug. ***
Comment 5 Matthew Barnes 2009-11-23 21:02:30 UTC
Highly looking forward to this, Dan!

When this lands I'll be able to give Camel a much-needed upgrade.
Comment 6 Dan Winship 2009-12-19 11:12:46 UTC
ok, this is still incomplete (especially on the server side; the test program, copied from libsoup, uses raw gnutls calls for the server). But at least some parts work now.

danw@x61:tests (tls)> ./tls-test 
SYNCHRONOUS SSL TEST PASSED
ASYNCHRONOUS SSL TEST PASSED

some bits of the code:

  tlsclient = g_tls_client_new ();
  /* The test program uses a self-signed certificate, so we need to
   * not validate that. Hostname validation is currently broken.
   */
  vflags = g_tls_client_get_validation_flags (tlsclient);
  vflags &= ~(G_TLS_VALIDATE_CA | G_TLS_VALIDATE_HOSTNAME);
  g_tls_client_set_validation_flags (tlsclient, vflags);

  client = g_socket_client_new ();
  g_socket_client_set_tls_client (client, tlsclient);
  g_object_unref (tlsclient);

  conn = g_socket_client_connect_to_host (client, "127.0.0.1", port,
					  NULL, &error);
  g_assert_no_error (error);

  g_tls_socket_connection_start_tls (G_TLS_SOCKET_CONNECTION (conn),
				     NULL, &error);
  g_assert_no_error (error);

  ostream = g_io_stream_get_output_stream (G_IO_STREAM (conn));
  ...

and then the ostream works just like an ordinary GSocketOutputStream, etc.

GTlsSocketConnection is a (non-TLS-library-specific) wrapper around GSocketConnection. GTlsClient and GTlsSession (not seen here) are TLS-library-specific; GTlsSocketConnection creates a GTlsSession using the passed-in GTlsClient, and then uses that to do TLS.

I eventually decided against having GTlsSocket, for a few reasons:

    1. GSocket isn't really set up to be subclassed; although we added
       a bunch of private fields to GSocketClass, we'd still have to do
       wacky things like listening for property notifies in the subclass
       etc.

    2. GTlsSocket wouldn't be a "proper" subclass of GSocket anyway.
       Lots of GSocket operations wouldn't be possible on it, and because
       of the handshaking issues, either we'd have to make GTlsSocket very
       complicated to abstract around them, or else some operations would
       have to have more complicated semantics under GTlsSocket, which
       means callers would need to have special GTlsSocket-specific code
       anyway.

    3. It creates weird combinatorial problems as mentioned above (what if
       you make a plain GTcpSocketConnection with a GTlsSocket, etc).

    4. We already have an IO abstraction--G{Input,Output}Stream. We don't
       need a *second* abstraction at the socket layer.

So, instead we have GTlsSession, which works alongside a GSocket without actually wrapping it.

(I have figured out how to implement GTlsSession using NSS as well. We implement our own PRFileDesc "class" wrapping a GSocket and then pass one of those to SSL_ImportFD. Then, when a non-blocking PR_Read/PR_Write returns a PR_WOULD_BLOCK_ERROR, we can know whether we need to poll for readability or writability based on whether the last G_IO_ERROR_WOULD_BLOCK came from a g_socket_send or g_socket_receive.)

The code is currently at http://gnome.org/~danw/glib.git, tls branch. It's not ready to go into a branch on git.gnome.org yet, and I still don't know where I'm going to put the gnutls(/NSS)-specific bits. Comments on the API welcome, but do it soon.
Comment 7 Dan Winship 2009-12-19 11:17:21 UTC
*** Bug 573029 has been marked as a duplicate of this bug. ***
Comment 8 Dan Winship 2009-12-19 11:18:18 UTC
*** Bug 581890 has been marked as a duplicate of this bug. ***
Comment 9 Dan Winship 2009-12-19 11:19:26 UTC
*** Bug 573142 has been marked as a duplicate of this bug. ***
Comment 10 Dan Winship 2009-12-19 11:23:57 UTC
oh, while "TLS" has been the correct name since forever, I'm willing to entertain the possibility of calling it "SSL" if everyone else is going to get horribly confused and think "TLS" means "Thread-Local Storage".
Comment 11 Philip Van Hoof 2009-12-20 00:12:15 UTC
I don't think we should call it SSL. Within the context of networking and I/O is it clear that the acronym TLS doesn't mean Thread Local Storage.
Comment 12 Alexander Larsson 2010-01-06 10:11:49 UTC
glib calls thread local storage "private" (such as in g_private_new or g_private_get), so i don't think confusing these two is any major risk.
Comment 13 Alexander Larsson 2010-01-06 11:50:20 UTC
I'm not really the right person to review this, but from an overal api standpoint it seems good, and I heartily approve using the highlevel abstractions (streams) rather than trying to shoe-horn tls into the socket concept (assuming this is possible, which is seems to be).

About where the implementation code will live... I'm not sure, maybe we could just have it in a gio subdirectory that is conditionally built. Another possibility would be to have the unix ones in gvfs, but that seems a bit weird (its not really a vfs anymore).

Also, this extension point is sort of heavy so it makes it more important to do the work for extension point to cache the plugin info rather than having all processess dlopen all giomodules on startup.
Comment 14 Javier Jardón (IRC: jjardon) 2010-03-24 00:44:08 UTC
punting to 2.26
Comment 15 Eduardo Lima Mitev 2010-03-29 22:49:03 UTC
Adding myself to CC list.
Comment 16 Dan Winship 2010-04-26 14:49:19 UTC
did a (non-fast-forward) push to my private repo (http://gnome.org/~danw/glib.git, tls branch). gtk-docs are more complete now, and there's now a GTlsCertificate type instead of only having GTlsCertificateList like before. There will probably be more operations added to this type later (g_tls_certificate_list_get_hostname, etc).

next steps: add private key support (either as more GTlsCertificate methods, or a separate GTlsPrivateKey type), and then finish implementing the server side, and update tls-test to use that. That should also result in getting client-side certificate support working, since the APIs are nearly identical for the two cases. at that point it should be pretty close to done (at least the gnutls version; as mentioned before, I want to get the NSS version done as well before declaring the API frozen)

There's been some discussion in connection with the proxy bug about whether or not it should be possible to wrap an arbitrary GSocketConnection with a GTlsSocketConnection, or if the current approach, where GTlsSocketConnection cheats and does I/O on the socket directly, is sufficient. Making it able to wrap arbitrary gio streams is obviously more elegant API-wise, but it would suck codewise, because it would need to translate gio-style async ops into unix-style (EAGAIN) async ops, and would probably inevitably end up doing extra copying. However, this code would only need to exist once, not per-tls-library-backend, and it could short-circuit the simple case where the underlying socketconnection was just a plain gsocketconnection.
Comment 17 Eduardo Lima Mitev 2010-05-10 13:00:09 UTC
I have been looking into the tls branch and have a comment:

  * Shouldn't GTlsInputStream and GTlsOutputStream be derived from GFilterInputStream and GFilterOutputStream respectively, instead of just GInputStream and GOutputStream? Some of the reasons I can think of are:

     - TLS is conceptually a stream filter.
     - It could be useful to use a transport other than TCP/SCTP to route the TLS bits (eg. HTTP, SOCKS, ...)
     - It could be useful to have bandwidth/latency throttling before putting the TLS bits on the wire.

I suppose this shouldn't require big changes in current code. Perhaps a GSocketInputStreams and GSocketOutputStreams could be plugged after in the pipeline to achieve same behavior.

  * (nitpicking) First line of gtlsoutputstream.c is "/* GIO - GLib Output, Output and Streaming Library" (notice "output" twice), a search/replace detail I guess :).

Congrats for the great work.
Comment 18 Dan Winship 2010-05-10 13:41:22 UTC
(In reply to comment #17)
>   * Shouldn't GTlsInputStream and GTlsOutputStream be derived from
> GFilterInputStream and GFilterOutputStream respectively, instead of just
> GInputStream and GOutputStream? Some of the reasons I can think of are:

see last paragraph of comment #16. currently it is *not* actually acting as a filter (though your points give additional reasons it might be good if it was).


hoping to do another push later this week with working server support and client-side certificate support
Comment 19 Dan Winship 2010-08-17 18:47:54 UTC
Pushed latest code to git://github.com/danwinship/glib.git, tls branch.

(In reply to comment #16)
> There's been some discussion in connection with the proxy bug about whether or
> not it should be possible to wrap an arbitrary [GIOStream] with a
> GTlsSocketConnection

I ended up doing this. There is now a GIOStreamAdapter that takes a GIOStream and provides an I/O API that looks more like GSocket. It has special short-circuit cases for GSocketConnection and anything that uses GFileDescriptorBased streams, so in the case of TLS-over-GSocketConnection, it doesn't have any more overhead than the old code.

I also drastically simplified the TLS API... in the old API there were both "contexts" (objects that kept track of global state) and "sessions" (objects that kept track of per-connection state). However, I realized that GSocketClient and GSocketService already exist as global-state-keeping-track-of objects, and we can just add multiple TLS-related properties to them.

So in the new API, there are basically 3 types: GTlsCertificate, GTlsClientConnection, and GTlsServerConnection. In the simplest case, using TLS is as easy as calling "g_socket_client_set_tls (client, TRUE);" and then you get back a GTlsClientConnection instead of a GTcpSocketConnection or whatever.

There are two big remaining problems, public-API-wise:

1. GSocketClient typing: GSocketClient only returns GSocketConnections, but GTlsClientConnection is not a GSocketConnection. (In theory, you could wrap a GTlsClientConnection around a GIOStream that wasn't connected to a socket anywhere...) The proxy code also ran into this problem. The solution there was to create a special subclass of GTcpSocketConnection that returns its own input/output streams. We could do the same thing, however this then ties into the next problem:

2. Combinatoric explosion of stream types: if you have a SOCKS-proxied TCP connection to a TLS server, you may want to be able to call SOCKS-related, TCP-related, and TLS-related methods on the stream. We don't want to have a GSocksTcpTlsConnection type. Probably this means we need to more-explicitly "stack" the various layers, and have methods that let you get the next level down in the stack, so you can access the methods you want? ("GFilterIOStream"?)
Comment 20 Dan Winship 2010-11-07 20:02:06 UTC
Created attachment 174020 [details] [review]
make GProxyConnection public, as GTcpWrapperConnection
Comment 21 Dan Winship 2010-11-07 20:03:33 UTC
Created attachment 174021 [details] [review]
Add TLS (SSL) support to gio

These two patches, plus the patches from the depended-on bugs, are now on the "tls" branch in GNOME git
Comment 22 Dan Winship 2010-11-07 20:08:46 UTC
oh, and the actual GNUTLS code is in the "tls" branch of glib-networking
Comment 24 Nicolas Dufresne (ndufresne) 2010-11-11 19:51:16 UTC
Review of attachment 174020 [details] [review]:

I like this rework, ++

p.s. I left un-touched the status assuming other people should agree on API addition, may in next GTK meeting ?

::: gio/gtcpwrapperconnection.c
@@ +19,3 @@
+ *
+ * Authors: Nicolas Dufresne <nicolas.dufresne@colllabora.co.uk>
+ */

You may add you name here if you want ;),
btw, I like the change you did to store Socket and Stream, instead of Connection and Stream.
Comment 25 Nicolas Dufresne (ndufresne) 2010-11-11 20:21:15 UTC
Review of attachment 174021 [details] [review]:

This is just small issues I've came across while reading the new API.

::: gio/gdummytlsbackend.h
@@ +1,3 @@
+/* GIO - GLib Input, Output and Streaming Library
+ *
+ * Copyright (C) 2010 Collabora, Ltd.

Not sure Red Hat will agree ;)

@@ +18,3 @@
+ * Boston, MA 02111-1307, USA.
+ *
+ * Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>

I don't remember writing this ;)

::: gio/gsocketclient.c
@@ +604,3 @@
+ * Returns: the TLS validation flags
+ *
+ * Since: 2.26

Should be 2.28

@@ +620,3 @@
+ * via @client. The default value is %G_TLS_VALIDATE_ALL.
+ *
+ * Since: 2.26

Should be 2.28

::: gio/gsocketclient.h
@@ +94,3 @@
+									 gboolean              tls);
+gboolean                g_socket_client_get_tls_handshake               (GSocketClient        *client);
+void                    g_socket_client_set_tls_handshake               (GSocketClient        *client,

Those two functions are not implemented. I'm not sure if we really need those, is it that much work to use TLS from a normal GTcpConnection ?

::: gio/gtlsbackend.h
@@ +1,3 @@
+/* GIO - GLib Input, Output and Streaming Library
+ *
+ * Copyright (C) 2009 Red Hat, Inc.

2010

::: gio/gtlscertificate.c
@@ +1,3 @@
+/* GIO - GLib Input, Output and Certificateing Library
+ *
+ * Copyright (C) 2009 Red Hat, Inc.

2010

::: gio/gtlscertificate.h
@@ +1,3 @@
+/* GIO - GLib Input, Output and Streaming Library
+ *
+ * Copyright (C) 2009 Red Hat, Inc.

2010

::: gio/gtlsclientconnection.h
@@ +1,3 @@
+/* GIO - GLib Input, Output and Streaming Library
+ *
+ * Copyright (C) 2009 Red Hat, Inc.

2010 ;)

::: gio/gtlsconnection.h
@@ +1,3 @@
+/* GIO - GLib Input, Output and Streaming Library
+ *
+ * Copyright (C) 2009 Red Hat, Inc.

2010 ;)

::: gio/gtlsserverconnection.h
@@ +54,3 @@
+
+GTlsServerConnection *g_tls_server_connection_new                      (GError                 **error,
+									...) G_GNUC_NULL_TERMINATED;

Spacing is weird, might be bugzilla differ though ...
Comment 26 David Zeuthen (not reading bugmail) 2010-11-12 21:36:55 UTC
Quick comment about http://people.gnome.org/~danw/tls-docs/gio/gio-TLS-Overview.html#GTlsValidationFlags as mentioned on IRC:

 - Would expect G_TLS_VALIDATION_FLAGS_ as the prefix instead of
   G_TLS_VALIDATE_
   - I'm surprised that it works with e.g. gobject-introspection this way

 - Should use (1<<N) instead of (2<<N)

 - Traditionally there's a _NONE = 0 member as the first member (mostly
   to discourage app programmers from passing a literal 0 in their user
   code).. but since it's weird to use _NONE here then maybe it's not
   necessary. Would be nice to have, regardless

 - G_TLS_VALIDATE_ALL is problematic

   - it's currently wrong (should be 0x7f)

   - because of the (2<<N) bug it's even more wrong - right
     now it doesn't select REVOCATION and ALGORITHM

   - if you grow the enum but don't recompile your apps they will
     not select the new features - I argue that they should...

   - suggest to do this instead

   #define G_TLS_VALIDATE_ALL (g_tls_validation_flags_get_supported())

   where g_tls_validation_flags_get_supported() is defined to return
   all supported validation flags. 

 - It's a bit weird to see G_TLS_VALIDATE_GENERIC_ERROR as
   part of this enumeration - is it possible to get rid of that?
Comment 27 Dan Winship 2010-11-12 21:58:02 UTC
(In reply to comment #26)
>  - Would expect G_TLS_VALIDATION_FLAGS_ as the prefix instead of
>    G_TLS_VALIDATE_
>    - I'm surprised that it works with e.g. gobject-introspection this way

You apparently haven't looked at many GTK enum types...

The naming was to suggest "G_TLS_VALIDATE_IDENTITY" = "Please validate the identity". But it does end up seeming weird in lots of contexts (especially when it's used as the set of validations that *failed* in the accept-certificate signal handler). I should rename them.

>  - Should use (1<<N) instead of (2<<N)

Seriously no clue how that happened... :)

>    code).. but since it's weird to use _NONE here then maybe it's not
>    necessary. Would be nice to have, regardless

Not weird, no. There are definitely cases where you want to say "don't validate anything".

>    - suggest to do this instead
> 
>    #define G_TLS_VALIDATE_ALL (g_tls_validation_flags_get_supported())

I want to just define it to 0xFFFFFFFF or G_MAXUINT or ~0 or something... but there doesn't seem to be much precedent for that...

>  - It's a bit weird to see G_TLS_VALIDATE_GENERIC_ERROR as
>    part of this enumeration - is it possible to get rid of that?

meh, see, that's the "here's what I want you do validate" vs "here's what errors I found" distinction again. The backend may choose to reject a certificate for some reason that isn't any of the other GTlsValidationFlags values, in which case accept-certificate will see that as G_TLS_VALIDATE_GENERIC_ERROR.
Comment 28 David Zeuthen (not reading bugmail) 2010-11-12 23:13:05 UTC
(In reply to comment #27)
> meh, see, that's the "here's what I want you do validate" vs "here's what
> errors I found" distinction again. The backend may choose to reject a
> certificate for some reason that isn't any of the other GTlsValidationFlags
> values, in which case accept-certificate will see that as
> G_TLS_VALIDATE_GENERIC_ERROR.

Maybe split the "here's what I want you to validate" and "here's what errors I found" into two separate enumeration types? I mean, it's already kind of confusing with G_TLS_VALIDATE_EXPIRATION - is that in the former or the latter - you kinda have to look up the docs to tell (ideally the name should be enough to tell).

So maybe two enumeration types, say GTlsValidationRequestFlags (with G_TLS_VALIDATE_REQUEST_* values) and GTlsValidationResponseFlags (with G_TLS_VALIDATE_RESPONSE_* values), instead of just one?
Comment 29 Dan Winship 2010-11-12 23:24:41 UTC
(In reply to comment #28)
> Maybe split the "here's what I want you to validate" and "here's what errors I
> found" into two separate enumeration types? I mean, it's already kind of
> confusing with G_TLS_VALIDATE_EXPIRATION - is that in the former or the latter

They're all in both. If you specify G_TLS_VALIDATE_EXPIRATION in validation-flags, then it will check that certificates are not expired. If you see G_TLS_VALIDATE_EXPIRATION in accept-certificate's validation_errors parameter, then it means that the certificate was expired. (The mask that you pass in as validation-flags equals the set of validation_errors you could possibly get out.)
Comment 30 Simon McVittie 2010-11-18 15:40:35 UTC
(Please let me know if this is the wrong bug or if I should open another.)

In telepathy-gabble bug https://bugs.freedesktop.org/show_bug.cgi?id=31474 and Empathy Bug #634197, we were asked to copy in a configure check from GIO so that packagers could use the same configure option to set the system cert path for all three projects.

However, we've realised this configure check has a couple of problems:

Adam Conran points out that the configure check fails if the CA file isn't present on the build system. In a minimal build environment, or when cross-compiling, or whatever, we don't want to require that - it's fine for the auto-detection to fail in these cases, but if the user specifies a location, we should believe that they will arrange for it to be present on the host system.

I propose to fix this for telepathy-gabble with
http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=commitdiff;h=f6468dd3e47958de879708ea83caaefa2e2fd3f5 and it'd be great if you could comment on this and apply it to GIO.

Meanwhile, Sjoerd Simons points out that when using the OpenSSL backend, it's
conventional and more efficient if the CA location is a directory
(/etc/ssl/certs on Debian) containing fingerprint-based symlinks (e.g.
/etc/ssl/certs/00673b5b.0 -> thawte_Primary_Root_CA.pem). Again, it's OK if
auto-detection doesn't handle this case, but if the user forces it, that should
be respected.

This makes the name --with-ca-file misleading, so I reverted to
--with-ca-certificates in
http://git.collabora.co.uk/?p=user/smcv/telepathy-gabble-smcv.git;a=commitdiff;h=3badae31eb4c05015b17a7f759b7281f46a814bf - again, it'd be great to get this change into GIO.

See the patches' commit messages for more details.
Comment 31 Dan Winship 2010-11-18 16:59:39 UTC
(In reply to comment #30)
> Adam Conran points out that the configure check fails if the CA file isn't
> present on the build system.

Yeah, that was reported against epiphany too. It's already fixed in glib-networking (tls branch). If you don't specify --with-ca-file, it will error out if it can't find anything, but if you do specify a file, and that file doesn't exist, it will just give a warning.

> Meanwhile, Sjoerd Simons points out that when using the OpenSSL backend, it's
> conventional and more efficient if the CA location is a directory
> (/etc/ssl/certs on Debian) containing fingerprint-based symlinks

Ah, that's definitely nicer than parsing a 10000-line file... stefw had already pointed out that we need to rework the API to only look up the specific CAs it needs, rather than constructing a list of all of them ahead of time. Of course if you only have the single-file CA list then you have to parse it anyway to be able to do the lookup, but in the Debian case, or in the eventual gnome-keyring/PKCS#11 case, it will be possible to avoid that.
Comment 32 Simon McVittie 2010-11-19 12:29:57 UTC
Thanks, I've resynced our check with the one from glib-networking (in a branch).

Are you going to rename --with-ca-file to --with-ca-certificates in glib-networking as we suggest above, or keep the shorter but less correct name? I prefer --with-ca-certificates myself, but I think it's more important that we use the same option name that glib-networking does, so please decide one way or the other, so I can merge my patch or not :-)
Comment 33 Dan Winship 2010-11-19 13:57:26 UTC
--with-ca-certificates sounds better, but eventually you shouldn't need it in telepathy-gabble anyway because you'll just use GTlsCertificateDatabase (qv http://git.gnome.org/browse/glib/tree/gio/TLS-NOTES.txt?h=tls)
Comment 34 Simon McVittie 2010-11-19 15:45:53 UTC
(In reply to comment #33)
> --with-ca-certificates sounds better

Thanks, I'll go back to that name.
Comment 35 Dan Winship 2010-11-26 21:02:49 UTC
Initial implementation committed. This bug has gotten too big, so further
discussion can happen on new bugs. (I'll fill in bugs for the existing
known issues later this weekend.)

Attachment 174020 [details] pushed as a169033 - make GProxyConnection public, as GTcpWrapperConnection