-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 27/01/11 23:42, Mads Kiilerich wrote:
> On 01/27/2011 04:51 AM, Robert Ancell wrote:
>>
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>>
>> Hi FreeRDPers!
>>
>> I'm currently trying to get Remmina/FreeRDP as the default on
>> the Ubuntu 11.04 CD, but our pesky security team wants the
>> certificate checking to work:
>>
>> RD_BOOL crypto_cert_verify(CryptoCert server_cert, CryptoCert
>> cacert) { /* FIXME: do the actual verification */ return True; }
>
> I assume this is from crypto_openssl.c and that you don't care
> about other crypto backends. Ok.
>
> This function is only used to verify the individual links in the
> x509 certificate chain is correct. That alone is far from enough.
> Note however that this part works with the gnutls backend.
>
> Finally (so far) there is the tls option. libfreerdp/tls.c (which
> so far only works with openssl) is far more complete but still not
> completely finished.
>
>> So the question is: - - Any chance of this working by the end of
>> February? - - Any plans for this? - - If you guys haven't got
>> plans, I'll work on a patch. I'm not an expert at certificate,
>> do I just need to pass the information to the GUI and let the
>> user ACK/NACK it?
>
> AFAIK there are no specific plans and no chance unless somebody do
> something.
>
> I think FreeRDP is quite stable and reliable on local trusted
> networks, but I wouldn't recommend using it on untrusted networks
> or when connecting to untrusted servers. FreeRDP security in these
> (and other) areas is definitely not worse than rdesktop (which I
> assume is the only alternative).
>
> It would be great if you could work on improvements in this area.
>
> A brief description of some aspects of a good solution could be: *
> options for warning/accepting/failing on "Proprietary Certificate"
> * more common handling of certificates for tls and non-tls *
> support more crypto backends for tls (and nla) (but focusing on
> openssl first is fine) * checking that the server certificate
> matches the request hostname * functionality for checking that the
> x509 chain can be validated with the systems CA certificates
> (probably only useful in very few setups) * functionality for using
> other CA certificates (so you can add your local AD CA and
> automatically trust all servers on the domain regarding rdp without
> adding it to the global configuration) * ssh-like "known host"
> functionality, asking "unknown host X shows certificate Y - trust
> it and store it to next time?", adding it to some "known_hosts"
> file and using it next time and failing/prompting if it doesn't
> match next time
>
> It will require changes to both libfreerdp and xfreerdp and will
> thus also require a so version bump.
>
> Not a trivial task ... It might make sense to focus on "known
> host" and ignore the PKI mess. That might bring you most of the way
> to what you want.
>
> /MadsHi Mads, Thanks for the information. Yes, we would be switching from rdesktop (which is a support problem). I've had a first attempt at getting something working. Firstly, I'm using the rdp_mitm server to connect to. I couldn't work out how to get a secure RDP server to work in Linux. Attached patch fixes the build system for this. The second patch is to add a callback in the freerdp API so that RDP clients can prompt the user. Some questions: - - I figure the API needs to provide the security state of the connection (unsecured, secured with unknown/invalid certificate, secured with valid certificate). Please let me know if this is heading in the direction you expect. - - There are two encryption schemes here right? One for the whole channel (TLS) and one inside the RDP protocol (MCS?). Is TLS the newer method and MCS? the legacy method? - - Any thoughts on how to provide certificate contents to the user for them to decide if a certificate is valid / should be added to the list of accepted certificates? This seems difficult to provide due to the number of different crypto backends (i.e. no shared certificate class). Thanks, - --Robert -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk1Q3DAACgkQGOqhiQ98iC4hFgCdFPE8E9uqwMUu4uusxRxs08+Q 7D8AoKxs+aMrfky4XLmjkjwJttJbL2ag =RH45 -----END PGP SIGNATURE-----
>From 3be5a494f1c09ebc25ad6bfc539a31c35a65faf0 Mon Sep 17 00:00:00 2001 From: Robert Ancell <robert.anc...@canonical.com> Date: Tue, 8 Feb 2011 16:16:22 +1100 Subject: [PATCH 1/2] Fix rdp_mitm compile --- rdp_mitm/Makefile | 6 +++--- rdp_mitm/rdp_mitm.c | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/rdp_mitm/Makefile b/rdp_mitm/Makefile index c11eff0..bd9f3a5 100644 --- a/rdp_mitm/Makefile +++ b/rdp_mitm/Makefile @@ -1,8 +1,8 @@ ASN1_PATH = ../asn1 -CFLAGS = -O2 -w -I$(ASN1_PATH) -I.. -LDFLAGS = -lssl -OBJS = rdp_mitm.o $(ASN1_PATH)/*.o ../mem.o +CFLAGS = -O2 -w -I$(ASN1_PATH) -I.. -I../libfreerdp -I../build/include +LDFLAGS = -lssl -lcrypto +OBJS = rdp_mitm.o $(ASN1_PATH)/*.o ../libfreerdp/libfreerdp_la-mem.o all: rdp_mitm diff --git a/rdp_mitm/rdp_mitm.c b/rdp_mitm/rdp_mitm.c index 04a593c..03da5bf 100644 --- a/rdp_mitm/rdp_mitm.c +++ b/rdp_mitm/rdp_mitm.c @@ -30,8 +30,9 @@ real server after. This may require the proxy to know in advance the real password. */ -#include "rdesktop.h" +#include "rdp.h" #include "mem.h" +#include "stream.h" #include <stdio.h> #include <stdlib.h> -- 1.7.2.3
>From c9ada2ddda89c65077841fae5e1c880880411b2d Mon Sep 17 00:00:00 2001
From: Robert Ancell <robert.anc...@canonical.com>
Date: Tue, 8 Feb 2011 16:43:28 +1100
Subject: [PATCH 2/2] Pass certificate check up to UI layer
---
X11/xf_win.c | 8 ++++++++
include/freerdp/freerdp.h | 1 +
libfreerdp/rdp.h | 2 +-
libfreerdp/secure.c | 14 ++++++++++++++
libfreerdp/secure.h | 1 +
libfreerdp/tls.c | 4 +---
libfreerdp/tls.h | 2 ++
7 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/X11/xf_win.c b/X11/xf_win.c
index b3e3d49..5625aa4 100644
--- a/X11/xf_win.c
+++ b/X11/xf_win.c
@@ -188,6 +188,13 @@ xf_set_rop3(xfInfo * xfi, int rop3)
return 0;
}
+static RD_BOOL
+l_ui_check_certificate(struct rdp_inst * inst, const char * text)
+{
+ printf("ui_check_certificate: %s\n", text);
+ return False;
+}
+
static void
l_ui_error(struct rdp_inst * inst, const char * text)
{
@@ -893,6 +900,7 @@ l_ui_channel_data(struct rdp_inst * inst, int chan_id, char * data, int data_siz
static int
xf_assign_callbacks(rdpInst * inst)
{
+ inst->ui_check_certificate = l_ui_check_certificate;
inst->ui_error = l_ui_error;
inst->ui_warning = l_ui_warning;
inst->ui_unimpl = l_ui_unimpl;
diff --git a/include/freerdp/freerdp.h b/include/freerdp/freerdp.h
index 5313c6f..f9c2ad6 100644
--- a/include/freerdp/freerdp.h
+++ b/include/freerdp/freerdp.h
@@ -83,6 +83,7 @@ struct rdp_inst
int (* rdp_channel_data)(rdpInst * inst, int chan_id, char * data, int data_size);
void (* rdp_disconnect)(rdpInst * inst);
/* calls from library to ui */
+ RD_BOOL (* ui_check_certificate)(rdpInst * inst, const char * text);
void (* ui_error)(rdpInst * inst, const char * text);
void (* ui_warning)(rdpInst * inst, const char * text);
void (* ui_unimpl)(rdpInst * inst, const char * text);
diff --git a/libfreerdp/rdp.h b/libfreerdp/rdp.h
index 3890ed1..a0ba245 100644
--- a/libfreerdp/rdp.h
+++ b/libfreerdp/rdp.h
@@ -22,7 +22,7 @@
#define __RDP_H
#include <time.h>
-#include <freerdp/types_ui.h>
+#include <freerdp/freerdp.h>
#include "types.h"
RD_BOOL
diff --git a/libfreerdp/secure.c b/libfreerdp/secure.c
index 6bcdc30..ea4ff9b 100644
--- a/libfreerdp/secure.c
+++ b/libfreerdp/secure.c
@@ -717,6 +717,8 @@ sec_parse_server_security_data(rdpSec * sec, STREAM s, uint32 * encryptionMethod
ui_error(sec->rdp->inst, "TS Certificate not signed with License Certificate\n");
return False;
}
+ if (!sec->rdp->inst->ui_check_certificate || !sec->rdp->inst->ui_check_certificate(sec->rdp->inst, ""))
+ return False;
crypto_cert_free(license_cert);
if (crypto_cert_get_pub_exp_mod(ts_cert, &(sec->server_public_key_len),
@@ -985,6 +987,12 @@ sec_connect(rdpSec * sec, char *server, char *username, int port)
printf("TLS encryption with NLA negotiated\n");
sec->ctx = tls_create_context();
sec->ssl = tls_connect(sec->ctx, sec->mcs->iso->tcp->sock, server);
+ sec->verified = tls_verify(sec->ssl, server);
+ if(!sec->verified)
+ {
+ if (!sec->rdp->inst->ui_check_certificate || !sec->rdp->inst->ui_check_certificate(sec->rdp->inst, ""))
+ return False;
+ }
sec->tls_connected = 1;
ntlm_send_negotiate_message(sec);
credssp_recv(sec);
@@ -997,6 +1005,12 @@ sec_connect(rdpSec * sec, char *server, char *username, int port)
printf("TLS Encryption negotiated\n");
sec->ctx = tls_create_context();
sec->ssl = tls_connect(sec->ctx, sec->mcs->iso->tcp->sock, server);
+ sec->verified = tls_verify(sec->ssl, server);
+ if(!sec->verified)
+ {
+ if (!sec->rdp->inst->ui_check_certificate || !sec->rdp->inst->ui_check_certificate(sec->rdp->inst, ""))
+ return False;
+ }
sec->tls_connected = 1;
sec->rdp->settings->encryption = 0;
success = mcs_connect(sec->mcs);
diff --git a/libfreerdp/secure.h b/libfreerdp/secure.h
index 0649c48..8150e7b 100644
--- a/libfreerdp/secure.h
+++ b/libfreerdp/secure.h
@@ -58,6 +58,7 @@ struct rdp_sec
int tls_connected;
#ifndef DISABLE_TLS
SSL *ssl;
+ RD_BOOL verified;
SSL_CTX *ctx;
struct rdp_nla * nla;
#endif
diff --git a/libfreerdp/tls.c b/libfreerdp/tls.c
index b07ec37..dc23ff9 100644
--- a/libfreerdp/tls.c
+++ b/libfreerdp/tls.c
@@ -103,7 +103,7 @@ exit:
* certificate is signed by a trusted certification authority
*/
-static RD_BOOL
+RD_BOOL
tls_verify(SSL *connection, const char *server)
{
/* TODO: Check for eku extension with server authentication purpose */
@@ -253,8 +253,6 @@ tls_connect(SSL_CTX *ctx, int sockfd, char *server)
return NULL;
}
- tls_verify(ssl, server);
-
printf("TLS connection established\n");
return ssl;
diff --git a/libfreerdp/tls.h b/libfreerdp/tls.h
index 52777fa..18682a6 100644
--- a/libfreerdp/tls.h
+++ b/libfreerdp/tls.h
@@ -37,6 +37,8 @@ void
tls_destroy_context(SSL_CTX *ctx);
SSL*
tls_connect(SSL_CTX *ctx, int sockfd, char *server);
+RD_BOOL
+tls_verify(SSL *connection, const char *server);
void
tls_disconnect(SSL *ssl);
int
--
1.7.2.3
0001-Fix-rdp_mitm-compile.patch.sig
Description: PGP signature
0002-Pass-certificate-check-up-to-UI-layer.patch.sig
Description: PGP signature
------------------------------------------------------------------------------ The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE: Pinpoint memory and threading errors before they happen. Find and fix more than 250 security defects in the development cycle. Locate bottlenecks in serial and parallel code that limit performance. http://p.sf.net/sfu/intel-dev2devfeb
_______________________________________________ Freerdp-devel mailing list Freerdp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freerdp-devel
