Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement TLS Sockets #19

Closed
andyholmes opened this issue Aug 30, 2017 · 31 comments
Closed

Implement TLS Sockets #19

andyholmes opened this issue Aug 30, 2017 · 31 comments

Comments

@andyholmes
Copy link
Contributor

Just reporting so it's on the roadmap. TLS/SSL came with protocolVersion 6 and Android client is kept up to date in Play Store and F-Droid. Here's some relevant links to kdeconnect source:

TLS setup:
https://github.com/KDE/kdeconnect-kde/blob/master/core/backends/lan/lanlinkprovider.cpp#L417-L445

Testing for protocolVersion >= 6 (Android client is v7):
https://github.com/KDE/kdeconnect-kde/blob/master/core/backends/lan/lanlinkprovider.cpp#L254-L278

@albertvaka
Copy link

I think this should be prioritized in mconnect. As of now, the KDE Connect Android app still supports the old crypto for compatibility with distributions that don't yet package modern versions of KDE Connect on the desktop, but we would like to drop support for it at some point. Also, the old encryption is unsafe against replay attacks and weak compared to TLS.

@bboozzoo
Copy link
Owner

bboozzoo commented Sep 2, 2017

Makes sense. I hope it will be much easier to implement. From initial investigation I have a feeling that we'll be able to dump openssl as a direct dependency and just stick to Gio.

@andyholmes
Copy link
Contributor Author

I've been playing with this a little bit. It seems like KDE Connect does a bit of a custom handshake which might not be compatible with Gio.SocketClient(), although I'm kind of a socket newb.

First the server connects an SSL socket (in response to UDP Identity packet), then the socket opts are set like before (keepalive etc), then the server sends an Identity packet which the client responds to with a handshake (eg. \u0016\u0003\u0001,68). I've been trying to wrestle Gio.SocketClientEvent's but I think Gio.SocketClient() is really meant to initiate the handshake since it seems to complain:

Error performing TLS handshake: An unexpected TLS handshake packet was received.

Hope that helps a bit.

@andyholmes
Copy link
Contributor Author

andyholmes commented Sep 11, 2017

I haven't had time to learn Vala, but here's my GJS analog to device-channel.vala. After this the connection is ready to receive pair packets. I hope this save you some work at least!

    open: function () {
        log("DeviceChannel.open()");
        
        this._socket = new Gio.Socket({
            family: Gio.SocketFamily.IPV4,
            type: Gio.SocketType.STREAM,
            protocol: Gio.SocketProtocol.TCP
        });
        
        // Setup socket
        this._socket.init(null);
        this._socket.set_keepalive(true);
        this._socket.set_option(6, 4, 10);   // TCP_KEEPIDLE
        this._socket.set_option(6, 5, 5);    // TCP_KEEPINTVL
        this._socket.set_option(6, 6, 3);    // TCP_KEEPCNT
            
        this._connection = new Gio.TcpConnection({ socket: this._socket });
        
        this._connection.connect_async(
            new Gio.InetSocketAddress({
                address: this.device.tcpHost,
                port: this.device.tcpPort
            }),
            null,
            Lang.bind(this, this.opened) // Lang.bind is Javascript scope B.S.
        );
    },
    
    opened: function (connection, res) {
        log("DeviceChannel.opened()");
        
        try {
            if (!connection.connect_finish(res)) {
                throw Error("failed to connect");
            }
        } catch (e) {
            log("Error finishing connection: " + e);
            
            this.emit("disconnected");
            return;
        }
        
        // Send identity packet
        let ident = new Packet.IdentityPacket();
        ident.fromServer(this.device.manager);
        let _out = new Gio.UnixOutputStream({
            fd: this._socket.fd,
            close_fd: false
        });
        _out.write(ident.toString() + "\n", null);
        _out.close(null);
        
        // TLS
        let cert = Gio.TlsCertificate.new_from_files(
            "tls/certificate.pem",
            "tls/privateKey.pem"
        );
        
        // Redefine this._connection to the TLS connection
        this._connection = Gio.TlsServerConnection.new(this._connection, cert);
        this._connection.validation_flags = 0;
        this._connection.authentication_mode = 1;
        this._connection.connect("accept-certificate", (conn, peer_cert, flags) => {
            if (this.device.paired) {
                // If the device is already paired, we can compare the connection
                // certificate to the one we have saved to disk
                let paired_cert = Gio.TlsCertificate.new_from_file(
                    "device_config_path/certificate.pem"
                );
                
                return (paired_cert.verify(null, peer_cert) === 0);
            } else {
                // If the device isn't paired, we store the Gio.TlsCertificate in a variable
                // and later save it to disk if we accept or send a pair request.
                this._peer_cert = peer_cert;
                return true;
            }
        });
        this._connection.handshake(null);
            
        // Streams
        this._in = new Gio.DataInputStream({
            base_stream: this._connection.input_stream,
            newline_type: Gio.DataStreamNewlineType.LF
        });
        
        this._out = new Gio.DataOutputStream({
            base_stream: this._connection.output_stream
        });
        
        // There is no Gio.Socket.create_source() in GJS, but you get it
        this._monitor = this._in.base_stream.create_source(null);
        this._monitor.set_callback(Lang.bind(this, this._io_ready));
        this._monitor.attach(null);
    }

@bboozzoo
Copy link
Owner

Thanks, this looks really helpful.

@andyholmes
Copy link
Contributor Author

andyholmes commented Sep 15, 2017

I updated the code snippet above, so it now includes certificate verification stuff. Apparently:

  • if an unpaired device becomes connected, the connection certificate is accepted unconditionally
  • if an unpaired device becomes paired (either by receiving or sending a pair request) the certificate is saved to disk
  • if a paired device becomes connected, the connection certificate is verified with the certificate saved to disk

So in my test code for pairing (not included above) I just checked if a certificate.pem was written to disk; if true paired, if not unpaired.

@albertvaka
Copy link

Hey Andy,

You got it right :) The idea is to use TOFU (trust on first use), like in SSH or other protocols. If you have any questions feel free to ask here (I'm watching the thread) or on the kdeconnect mailing list (https://mail.kde.org/mailman/listinfo/kdeconnect).

Albert

@andyholmes
Copy link
Contributor Author

Hey Albert,

Thanks for checking in and glad to hear I got it right (I'm kind of any everything newb), I'll let you know if I have any more questions.

@bboozzoo
Copy link
Owner

@albertvaka @andyholmes thanks for your input.

Since we're bound to use GLib.TlsConnection anyway I have spent a couple of hours trying to get rid of explicit dependency on openssl and use GnuTLS instead. GnuTLS is a dependency of glib-networking (a package that delivers Gio TlsConnection classes). Well, tough luck, turns out gnutls.vapi file is outdated, incomplete and generates broken code.

@andyholmes
Copy link
Contributor Author

That's too bad, I seem to recall a similar comment in the kdeconnect code somewhere, lamenting the need for another dependency. FWIW, on Ubuntu/Debian openssl is an indirect dependency of every desktop meta-package available.

@bboozzoo
Copy link
Owner

I'm a bit surprised how inconvenient generating self signed certificates with GnuTLS is. I managed to generate the private key with relative ease, but self signed certificate seems to be a whole new level. Makes me a bit envious of the Qt version. It's nice and clean. Once I fixed gnutls.vapi I tried to at least copy certtool behavior only to decide that it's a waste of time. Instead of doing this programmatically, I'll just use the example openssl command kdeconnect guys left in their code and call it through Process.spawn_sync. A note for the packages, openssl will remain a dependency, but it will no longer be picked up by introspecting library dependencies.

@desiderantes
Copy link

Please share the fixed gnutls.vapi, I know a contributor that wants to fix it and keep it up to date

@bboozzoo
Copy link
Owner

@albertvaka any idea what might be causing this?

09-18 10:16:51.510 24364 25831 E KDE/LanLinkProvider: Handshake failed with mborzecki@comp-thk-006                     
09-18 10:16:51.510 24364 25831 W System.err: javax.net.ssl.SSLHandshakeException: Handshake failed                     
09-18 10:16:51.518 24364 25831 W System.err:    at com.android.org.conscrypt.OpenSSLSocketImpl.startHandshake(OpenSSLSocketImpl.java:429)                                                                                                      
09-18 10:16:51.519 24364 25831 W System.err:    at org.kde.kdeconnect.Backends.LanBackend.LanLinkProvider$4.run(LanLinkProvider.java:247)                                                                                                      
09-18 10:16:51.519 24364 25831 W System.err:    at java.lang.Thread.run(Thread.java:761)                               
09-18 10:16:51.519 24364 25831 W System.err: Caused by: javax.net.ssl.SSLProtocolException: SSL handshake aborted: ssl=0xa8c43900: Failure in SSL library, usually a protocol error                                                            
09-18 10:16:51.519 24364 25831 W System.err: error:04000069:RSA routines:OPENSSL_internal:BLOCK_TYPE_IS_NOT_01 (external/boringssl/src/crypto/rsa/padding.c:114 0xaa09a872:0x00000000)                                                         
09-18 10:16:51.519 24364 25831 W System.err: error:04000083:RSA routines:OPENSSL_internal:PADDING_CHECK_FAILED (external/boringssl/src/crypto/rsa/rsa_impl.c:507 0xaa09a872:0x00000000)                                                        
09-18 10:16:51.519 24364 25831 W System.err: error:10000072:SSL routines:OPENSSL_internal:BAD_SIGNATURE (external/boringssl/src/ssl/s3_clnt.c:1275 0xaa09a872:0x00000000)                                                                      
09-18 10:16:51.519 24364 25831 W System.err:    at com.android.org.conscrypt.NativeCrypto.SSL_do_handshake(Native Method)                                                                                                                      
09-18 10:16:51.519 24364 25831 W System.err:    at com.android.org.conscrypt.OpenSSLSocketImpl.startHandshake(OpenSSLSocketImpl.java:357)                                                                                                      
09-18 10:16:51.519 24364 25831 W System.err:    ... 2 more         

on mconnect side, I get Peer sent fatal TLS alert: Decrypt error so it bails early in the handshake process.

@andyholmes
Copy link
Contributor Author

@bboozzoo This may be caused by Gio.SocketClient, personally I could never get it to work and had to use a bare Gio.Socket. Other than that, my understanding is that after you connect the socket, you send an identity packet over the unencrypted socket, then the device responds by initiating the handshake and you accept by calling handshake() on the TlsConnection wrapped socket.

@bboozzoo
Copy link
Owner

I'm past that step, mconnect is already connected and identity packet was sent. The error happens when the code starts TLS handshake. KDE connect code does a bit of magic:

    QList<QSslCipher> socketCiphers;
    socketCiphers.append(QSslCipher(QStringLiteral("ECDHE-ECDSA-AES256-GCM-SHA384")));
    socketCiphers.append(QSslCipher(QStringLiteral("ECDHE-ECDSA-AES128-GCM-SHA256")));
    socketCiphers.append(QSslCipher(QStringLiteral("ECDHE-RSA-AES128-SHA")));
    socketCiphers.append(QSslCipher(QStringLiteral("RC4-SHA")));
    socketCiphers.append(QSslCipher(QStringLiteral("RC4-MD5")));

    // Configure for ssl
    QSslConfiguration sslConfig;
    sslConfig.setCiphers(socketCiphers);
    sslConfig.setProtocol(QSsl::TlsV1_0);

I tried setting G_TLS_GNUTLS_PRIORITY (only after going through glib-networking source code) to something that's close to what KDE connect does, but with no luck. Digging into boringssl source code, there's a problem with verification of ServerKeyExchange message, specifically some issue with PKCS1 padding.

@desiderantes
Copy link

KDE connect code does a bit of magic

Isn't that represented in gnutls with a priority string ?

@bboozzoo
Copy link
Owner

@bboozzoo
Copy link
Owner

To make things more interesting, my ArchLinux laptop can do TLS handshake just fine:

09-18 19:44:50.995 10911 11017 I KDE/LanLinkProvider: Identity package received from a TCP connection from maciek@corsair                                                 
09-18 19:44:50.995 10911 11017 I KDE/LanLinkProvider: Starting SSL handshake with maciek@corsair trusted:false                                                            
09-18 19:44:51.260 10911 16594 I KDE/LanLinkProvider: Handshake as client successful with maciek@corsair secured with TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA                  
09-18 19:44:51.260 10911 16594 I KDE/LanLinkProvider: Creating a new link for device corsair                                                                              
09-18 19:44:51.263 10911 16594 I KDE/BackgroundService: addLink,unknown device: corsair                                                                                   
09-18 19:44:51.293 10911 16594 I KDE/Device: Got certificate                         
09-18 19:44:51.295 10911 16594 I KDE/Device: addLink LanLinkProvider -> maciek@corsair active links: 1          

OpenSUSE Tumbleweed can not 😒

@andyholmes
Copy link
Contributor Author

andyholmes commented Sep 18, 2017

I don't anything about TLS, but here is my exact Device Channel code in GJS that works reliably for me:

https://gist.github.com/andyholmes/159a6fb628dcc926518d1b129e345f47

I'm on Ubuntu 17.04, GnuTLS 3.5.6-4ubuntu4.2

@bboozzoo
Copy link
Owner

Got it mostly working in bboozzoo/tls-support branch. It is possible to connect, pair and receive data.

Tumbleweed problem is still bugging me though. Thought that maybe it is caused by recent update to GNOME 3.26 in TW, like a regression in glib or glib-networking. I've setup jhbuild and built mconnect with pieces of 3.26 stack, but still got the problem. Tried rebuilding gnutls 3.5.15 from source (same version as in TW repos) with similar results (on a side note, 3.5.15 works just fine in Arch). GnuTLS debug logs indicate that the connection is using RSA-SHA1 for digest verification, but disabling it via priority string caused TLS suite negotiation to fail. I'd appreciate any input on this.

@andyholmes
Copy link
Contributor Author

Cool, I've discovered file transfers can use the same type of connection, minus the identity packet, except downloads need to be wrapped in a Gio.TlsClientConnection.

@bboozzoo
Copy link
Owner

Something totally weird that I stumbled upon. First, I compared GnuTLS handshake logs between Arch and Tumbleweed, no luck here. However, when I regenerated the certificate on the Tumbleweed box, the problem was gone. Copying the old certficate/key to Arch setup I was able to reproduce the problem. So, for some unexplained reason, the certificate/key pair generated by openssl is bad or loading this bundle through GnuTLS causes issues. Comparing openssl x509 -in <cert> -text nothing unexpected stands out, the start/end dates, moduli, keys all differ but there are no 'extra' extensions. I can successfuly openssl verify both the new and the old certificates and the same with GnuTLS' certtool.

@bboozzoo
Copy link
Owner

Pushed some changes that introduce direct dependency on GnuTLS for generating certificate and key. Finally after digging through documentation and crypttool source code I managed to generate a usable key/cert pair in the code. This should also resolve #9 as OpenSSL is no longer needed.

@desiderantes updated gnutls.vapi file is in vapi/ in the source tree, the diff against Vala shipped vapi is right here: https://ptpb.pw/4k7z

@desiderantes
Copy link

@bboozzoo You'd still need to relicense to GPL2+ to fix #9.

@bboozzoo
Copy link
Owner

This should be fairly complete now. Device connection is rather stable, didn't notice anything unusual.

What might be useful for the shell integration is that device certificates are accessible through DBus as Certificate property of org.mconnect.Device interface. Similarly, the daemon's certificate is exposed as Certificate property of org.mconnect.DeviceManager interface (implemented on /org/mconnect/manager object).

@andyholmes
Copy link
Contributor Author

Could be, (current) desktop and android KDE Connect clients present the local and remote SHA fingerprints for verification. Can that be extracted on the backend as well?

@bboozzoo
Copy link
Owner

Yes. Do you know if the fingerprint is SHA1 or SHA256?

@andyholmes
Copy link
Contributor Author

The android app says SHA1

@bboozzoo
Copy link
Owner

Pushed a couple of patches:

$ busctl --user get-property \
    org.mconnect /org/mconnect/device/0 org.mconnect.Device CertificateFingerprint
s "sha1:d7860c6978d714e9e8f9805065602ea14e295df9"

@andyholmes
Copy link
Contributor Author

andyholmes commented Sep 28, 2017

Cool, I've been trying to follow along a bit in GJS just for fun, and just opened this bug here:

Bug 788315 - Implement 'fingerprint()' function on Gio.TlsCertificate

Must be nice having the full GLib and C libraries available ;)

@bboozzoo bboozzoo mentioned this issue Sep 29, 2017
@bboozzoo
Copy link
Owner

The code is now in master, closing this ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants