* bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 @ 2017-08-03 22:05 Leo Famulari 2017-08-03 23:22 ` Marius Bakke 0 siblings, 1 reply; 8+ messages in thread From: Leo Famulari @ 2017-08-03 22:05 UTC (permalink / raw) To: 27939; +Cc: Thomas Danckaert [-- Attachment #1: Type: text/plain, Size: 488 bytes --] The bugs corresponding to CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 were recently fixed in the FreeRDP Git repo: https://github.com/FreeRDP/FreeRDP/commit/03ab68318966c3a22935a02838daaea7b7fbe96c The most serious of these bugs allow the remote server (or any server in between) to execute arbitrary code on your machine. However, these changes do not apply cleanly to our version of FreeRDP. I don't have to port these changes back right now. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 2017-08-03 22:05 bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 Leo Famulari @ 2017-08-03 23:22 ` Marius Bakke 2017-08-04 8:34 ` Thomas Danckaert 0 siblings, 1 reply; 8+ messages in thread From: Marius Bakke @ 2017-08-03 23:22 UTC (permalink / raw) To: Leo Famulari, 27939-done; +Cc: Thomas Danckaert [-- Attachment #1: Type: text/plain, Size: 564 bytes --] Leo Famulari <leo@famulari.name> writes: > The bugs corresponding to CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 > CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 were recently fixed in the > FreeRDP Git repo: > > https://github.com/FreeRDP/FreeRDP/commit/03ab68318966c3a22935a02838daaea7b7fbe96c > > The most serious of these bugs allow the remote server (or any server in > between) to execute arbitrary code on your machine. Yikes! Thanks for the heads-up. I went ahead and updated to the 2.0.0 rc which contain this fix in c89091459f24dee4ba4959d65e38589efc1d8d9e. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 2017-08-03 23:22 ` Marius Bakke @ 2017-08-04 8:34 ` Thomas Danckaert 2017-08-04 14:56 ` Leo Famulari 0 siblings, 1 reply; 8+ messages in thread From: Thomas Danckaert @ 2017-08-04 8:34 UTC (permalink / raw) To: mbakke; +Cc: 27939-done From: Marius Bakke <mbakke@fastmail.com> Subject: Re: bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 Date: Fri, 04 Aug 2017 01:22:01 +0200 > Leo Famulari <leo@famulari.name> writes: > >> The bugs corresponding to CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 >> CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 were recently fixed in >> the >> FreeRDP Git repo: >> >> https://github.com/FreeRDP/FreeRDP/commit/03ab68318966c3a22935a02838daaea7b7fbe96c >> >> The most serious of these bugs allow the remote server (or any >> server in >> between) to execute arbitrary code on your machine. > > Yikes! Thanks for the heads-up. > > I went ahead and updated to the 2.0.0 rc which contain this fix in > c89091459f24dee4ba4959d65e38589efc1d8d9e. Thanks! Unfortunately, vinagre doesn't build against freerdp 2. I'll try to fix that, or otherwise try to backport the patches to freerdp 1.x. Thomas ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 2017-08-04 8:34 ` Thomas Danckaert @ 2017-08-04 14:56 ` Leo Famulari 2017-08-09 17:05 ` Thomas Danckaert 0 siblings, 1 reply; 8+ messages in thread From: Leo Famulari @ 2017-08-04 14:56 UTC (permalink / raw) To: Thomas Danckaert; +Cc: 27939-done [-- Attachment #1: Type: text/plain, Size: 331 bytes --] On Fri, Aug 04, 2017 at 10:34:55AM +0200, Thomas Danckaert wrote: > Unfortunately, vinagre doesn't build against freerdp 2. I'll try to fix > that, or otherwise try to backport the patches to freerdp 1.x. I think it should not be too hard to backport the patches if that's what we need to do, but I don't have the time this week. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 2017-08-04 14:56 ` Leo Famulari @ 2017-08-09 17:05 ` Thomas Danckaert 2017-08-09 21:34 ` Marius Bakke 0 siblings, 1 reply; 8+ messages in thread From: Thomas Danckaert @ 2017-08-09 17:05 UTC (permalink / raw) To: leo; +Cc: 27939-done [-- Attachment #1: Type: Text/Plain, Size: 1214 bytes --] From: Leo Famulari <leo@famulari.name> Subject: Re: bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 Date: Fri, 4 Aug 2017 10:56:15 -0400 > On Fri, Aug 04, 2017 at 10:34:55AM +0200, Thomas Danckaert wrote: >> Unfortunately, vinagre doesn't build against freerdp 2. I'll try >> to fix >> that, or otherwise try to backport the patches to freerdp 1.x. > > I think it should not be too hard to backport the patches if that's > what > we need to do, but I don't have the time this week. I tried applying the patch for https://github.com/FreeRDP/FreeRDP/commit/03ab68318966c3a22935a02838daaea7b7fbe96c to freerdp@1.2.0-beta1+android9, fixed the conflicts, and came up with the attached patch. I can confirm freerdp1.2beta with this patch compiles and runs, but cannot guarantee this fixes all those issues, because I'm totally unfamiliar with the code (and with rdp) ... is this enough to create a freerdp-1.2 package? The alternative is to downgrade to freerdp@1.1, or to disable rdp from vinagre. When I first submitted these packages, I ran into trouble trying to build freerdp@1.1, but I don't remember exactly what the problem was :). Thomas [-- Attachment #2: freerdp-CVE-2017-2834-2839.patch --] [-- Type: Text/X-Patch, Size: 22828 bytes --] diff --git a/libfreerdp/core/capabilities.c b/libfreerdp/core/capabilities.c index 91bc8a931..6c1a004dc 100644 --- a/libfreerdp/core/capabilities.c +++ b/libfreerdp/core/capabilities.c @@ -3464,12 +3464,12 @@ BOOL rdp_recv_get_active_header(rdpRdp* rdp, wStream* s, UINT16* pChannelId) if (rdp->settings->DisableEncryption) { - if (!rdp_read_security_header(s, &securityFlags)) + if (!rdp_read_security_header(s, &securityFlags, &length)) return FALSE; if (securityFlags & SEC_ENCRYPT) { - if (!rdp_decrypt(rdp, s, length - 4, securityFlags)) + if (!rdp_decrypt(rdp, s, length, securityFlags)) { DEBUG_WARN( "rdp_decrypt failed\n"); return FALSE; diff --git a/libfreerdp/core/certificate.c b/libfreerdp/core/certificate.c index 8829c0cd9..ff825d4af 100644 --- a/libfreerdp/core/certificate.c +++ b/libfreerdp/core/certificate.c @@ -357,7 +357,6 @@ static BOOL certificate_process_server_public_key(rdpCertificate* certificate, w UINT32 keylen; UINT32 bitlen; UINT32 datalen; - UINT32 modlen; if (Stream_GetRemainingLength(s) < 20) return FALSE; @@ -374,12 +373,11 @@ static BOOL certificate_process_server_public_key(rdpCertificate* certificate, w Stream_Read_UINT32(s, bitlen); Stream_Read_UINT32(s, datalen); Stream_Read(s, certificate->cert_info.exponent, 4); - modlen = keylen - 8; - if (Stream_GetRemainingLength(s) < modlen + 8) // count padding + if ((keylen <= 8) || (Stream_GetRemainingLength(s) < keylen)) return FALSE; - certificate->cert_info.ModulusLength = modlen; + certificate->cert_info.ModulusLength = keylen - 8; certificate->cert_info.Modulus = malloc(certificate->cert_info.ModulusLength); if (!certificate->cert_info.Modulus) @@ -543,7 +541,7 @@ BOOL certificate_read_server_proprietary_certificate(rdpCertificate* certificate BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate, wStream* s) { - int i; + UINT32 i; UINT32 certLength; UINT32 numCertBlobs; BOOL ret; @@ -558,7 +556,7 @@ BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate, if (!certificate->x509_cert_chain) return FALSE; - for (i = 0; i < (int) numCertBlobs; i++) + for (i = 0; i < numCertBlobs; i++) { if (Stream_GetRemainingLength(s) < 4) return FALSE; @@ -615,7 +613,7 @@ BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate, * @param length certificate length */ -BOOL certificate_read_server_certificate(rdpCertificate* certificate, BYTE* server_cert, int length) +BOOL certificate_read_server_certificate(rdpCertificate* certificate, BYTE* server_cert, size_t length) { wStream* s; UINT32 dwVersion; diff --git a/libfreerdp/core/certificate.h b/libfreerdp/core/certificate.h index 2d726cf8a..1e5fbfe99 100644 --- a/libfreerdp/core/certificate.h +++ b/libfreerdp/core/certificate.h @@ -50,7 +50,7 @@ void certificate_free_x509_certificate_chain(rdpX509CertChain* x509_cert_chain); BOOL certificate_read_server_proprietary_certificate(rdpCertificate* certificate, wStream* s); BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate, wStream* s); -BOOL certificate_read_server_certificate(rdpCertificate* certificate, BYTE* server_cert, int length); +BOOL certificate_read_server_certificate(rdpCertificate* certificate, BYTE* server_cert, size_t length); rdpCertificate* certificate_new(void); void certificate_free(rdpCertificate* certificate); diff --git a/libfreerdp/core/connection.c b/libfreerdp/core/connection.c index 52947636a..a79940b04 100644 --- a/libfreerdp/core/connection.c +++ b/libfreerdp/core/connection.c @@ -522,11 +522,8 @@ BOOL rdp_server_establish_keys(rdpRdp* rdp, wStream* s) return FALSE; } - if (!rdp_read_security_header(s, &sec_flags)) - { - DEBUG_WARN( "%s: invalid security header\n", __FUNCTION__); + if (!rdp_read_security_header(s, &sec_flags, NULL)) return FALSE; - } if ((sec_flags & SEC_EXCHANGE_PKT) == 0) { @@ -770,7 +767,12 @@ BOOL rdp_client_connect_auto_detect(rdpRdp* rdp, wStream *s) { if (channelId == rdp->mcs->messageChannelId) { - if (rdp_recv_message_channel_pdu(rdp, s) == 0) + UINT16 securityFlags = 0; + + if (!rdp_read_security_header(s, &securityFlags, &length)) + return FALSE; + + if (rdp_recv_message_channel_pdu(rdp, s, securityFlags) == 0) return TRUE; } } diff --git a/libfreerdp/core/gcc.c b/libfreerdp/core/gcc.c index ac75bc4ba..25b46e977 100644 --- a/libfreerdp/core/gcc.c +++ b/libfreerdp/core/gcc.c @@ -979,10 +979,10 @@ BOOL gcc_read_server_security_data(wStream* s, rdpMcs* mcs) Stream_Read_UINT32(s, settings->ServerRandomLength); /* serverRandomLen */ Stream_Read_UINT32(s, settings->ServerCertificateLength); /* serverCertLen */ - if (Stream_GetRemainingLength(s) < settings->ServerRandomLength + settings->ServerCertificateLength) + if (settings->ServerRandomLength == 0 || settings->ServerCertificateLength == 0) return FALSE; - if ((settings->ServerRandomLength <= 0) || (settings->ServerCertificateLength <= 0)) + if (Stream_GetRemainingLength(s) < settings->ServerRandomLength) return FALSE; /* serverRandom */ @@ -991,22 +991,34 @@ BOOL gcc_read_server_security_data(wStream* s, rdpMcs* mcs) return FALSE; Stream_Read(s, settings->ServerRandom, settings->ServerRandomLength); - /* serverCertificate */ + if(Stream_GetRemainingLength(s) < settings->ServerCertificateLength) + goto out_fail1; settings->ServerCertificate = (BYTE*) malloc(settings->ServerCertificateLength); if (!settings->ServerCertificate) - return FALSE; - Stream_Read(s, settings->ServerCertificate, settings->ServerCertificateLength); + goto out_fail1; + Stream_Read(s, settings->ServerCertificate, settings->ServerCertificateLength); certificate_free(settings->RdpServerCertificate); settings->RdpServerCertificate = certificate_new(); if (!settings->RdpServerCertificate) - return FALSE; + goto out_fail2; data = settings->ServerCertificate; length = settings->ServerCertificateLength; - return certificate_read_server_certificate(settings->RdpServerCertificate, data, length); + if (certificate_read_server_certificate(settings->RdpServerCertificate, data, length) < 1) + goto out_fail2; + + return TRUE; + + out_fail2: + free(settings->ServerCertificate); + settings->ServerCertificate = NULL; + out_fail1: + free(settings->ServerRandom); + settings->ServerRandom = NULL; + return FALSE; } static const BYTE initial_signature[] = diff --git a/libfreerdp/core/info.c b/libfreerdp/core/info.c index 217b3ff21..e48092729 100644 --- a/libfreerdp/core/info.c +++ b/libfreerdp/core/info.c @@ -582,7 +582,7 @@ BOOL rdp_recv_client_info(rdpRdp* rdp, wStream* s) if (!rdp_read_header(rdp, s, &length, &channelId)) return FALSE; - if (!rdp_read_security_header(s, &securityFlags)) + if (!rdp_read_security_header(s, &securityFlags, &length)) return FALSE; if ((securityFlags & SEC_INFO_PKT) == 0) @@ -598,7 +598,7 @@ BOOL rdp_recv_client_info(rdpRdp* rdp, wStream* s) if (securityFlags & SEC_ENCRYPT) { - if (!rdp_decrypt(rdp, s, length - 4, securityFlags)) + if (!rdp_decrypt(rdp, s, length, securityFlags)) { DEBUG_WARN( "rdp_decrypt failed\n"); return FALSE; diff --git a/libfreerdp/core/license.c b/libfreerdp/core/license.c index aa2c17f1d..d10ed72dc 100644 --- a/libfreerdp/core/license.c +++ b/libfreerdp/core/license.c @@ -232,12 +232,12 @@ int license_recv(rdpLicense* license, wStream* s) return -1; } - if (!rdp_read_security_header(s, &securityFlags)) + if (!rdp_read_security_header(s, &securityFlags, &length)) return -1; if (securityFlags & SEC_ENCRYPT) { - if (!rdp_decrypt(license->rdp, s, length - 4, securityFlags)) + if (!rdp_decrypt(license->rdp, s, length, securityFlags)) { DEBUG_WARN("%s: rdp_decrypt failed\n", __FUNCTION__); return -1; @@ -458,23 +458,41 @@ BOOL license_read_product_info(wStream* s, LICENSE_PRODUCT_INFO* productInfo) Stream_Read_UINT32(s, productInfo->dwVersion); /* dwVersion (4 bytes) */ Stream_Read_UINT32(s, productInfo->cbCompanyName); /* cbCompanyName (4 bytes) */ - if (Stream_GetRemainingLength(s) < productInfo->cbCompanyName + 4) + /* Name must be > 0, but there is no upper limit defined, use UINT32_MAX */ + if ((productInfo->cbCompanyName < 2) || (productInfo->cbCompanyName % 2 != 0)) + return FALSE; + + if (Stream_GetRemainingLength(s) < productInfo->cbCompanyName) return FALSE; productInfo->pbCompanyName = (BYTE*) malloc(productInfo->cbCompanyName); + if (!productInfo->pbCompanyName) + return FALSE; Stream_Read(s, productInfo->pbCompanyName, productInfo->cbCompanyName); + + if (Stream_GetRemainingLength(s) < 4) + goto out_fail; + Stream_Read_UINT32(s, productInfo->cbProductId); /* cbProductId (4 bytes) */ + if ((productInfo->cbProductId < 2) || (productInfo->cbProductId % 2 != 0)) + goto out_fail; + if (Stream_GetRemainingLength(s) < productInfo->cbProductId) - { - free(productInfo->pbCompanyName); - productInfo->pbCompanyName = NULL; - return FALSE; - } + goto out_fail; productInfo->pbProductId = (BYTE*) malloc(productInfo->cbProductId); + if (!productInfo->pbProductId) + goto out_fail; + Stream_Read(s, productInfo->pbProductId, productInfo->cbProductId); return TRUE; + + out_fail: + free(productInfo->pbCompanyName); + productInfo->pbCompanyName = NULL; + return FALSE; + } /** @@ -764,7 +782,10 @@ BOOL license_read_platform_challenge_packet(rdpLicense* license, wStream* s) Stream_Read_UINT32(s, ConnectFlags); /* ConnectFlags, Reserved (4 bytes) */ /* EncryptedPlatformChallenge */ license->EncryptedPlatformChallenge->type = BB_ANY_BLOB; - license_read_binary_blob(s, license->EncryptedPlatformChallenge); + + if (!license_read_binary_blob(s, license->EncryptedPlatformChallenge)) + return FALSE; + license->EncryptedPlatformChallenge->type = BB_ENCRYPTED_DATA_BLOB; if (Stream_GetRemainingLength(s) < 16) diff --git a/libfreerdp/core/mcs.c b/libfreerdp/core/mcs.c index 326622116..fef3f3532 100644 --- a/libfreerdp/core/mcs.c +++ b/libfreerdp/core/mcs.c @@ -217,7 +217,8 @@ BOOL mcs_read_domain_mcspdu_header(wStream* s, enum DomainMCSPDU* domainMCSPDU, BYTE choice; enum DomainMCSPDU MCSPDU; - *length = tpkt_read_header(s); + if (!tpkt_read_header(s, length)) + return FALSE; if (!tpdu_read_data(s, &li)) return FALSE; @@ -467,8 +468,13 @@ BOOL mcs_recv_connect_initial(rdpMcs* mcs, wStream* s) UINT16 li; int length; BOOL upwardFlag; + UINT16 tlength; + + if (!mcs || !s) + return FALSE; - tpkt_read_header(s); + if (!tpkt_read_header(s, &tlength)) + return FALSE; if (!tpdu_read_data(s, &li)) return FALSE; @@ -644,8 +650,13 @@ BOOL mcs_recv_connect_response(rdpMcs* mcs, wStream* s) BYTE result; UINT16 li; UINT32 calledConnectId; + UINT16 tlength; - tpkt_read_header(s); + if (!mcs || !s) + return FALSE; + + if (!tpkt_read_header(s, &tlength)) + return FALSE; if (!tpdu_read_data(s, &li)) return FALSE; diff --git a/libfreerdp/core/nego.c b/libfreerdp/core/nego.c index d5c98ef29..5b675469a 100644 --- a/libfreerdp/core/nego.c +++ b/libfreerdp/core/nego.c @@ -537,9 +537,7 @@ int nego_recv(rdpTransport* transport, wStream* s, void* extra) UINT16 length; rdpNego* nego = (rdpNego*) extra; - length = tpkt_read_header(s); - - if (length == 0) + if (!tpkt_read_header(s, &length) || length == 0) return -1; if (!tpdu_read_connection_confirm(s, &li)) @@ -613,8 +611,10 @@ BOOL nego_read_request(rdpNego* nego, wStream* s) BYTE li; BYTE c; BYTE type; + UINT16 length; - tpkt_read_header(s); + if (!tpkt_read_header(s, &length)) + return FALSE; if (!tpdu_read_connection_request(s, &li)) return FALSE; diff --git a/libfreerdp/core/peer.c b/libfreerdp/core/peer.c index e2f3f48f2..449e01daa 100644 --- a/libfreerdp/core/peer.c +++ b/libfreerdp/core/peer.c @@ -189,12 +189,12 @@ static int peer_recv_tpkt_pdu(freerdp_peer* client, wStream* s) if (rdp->settings->DisableEncryption) { - if (!rdp_read_security_header(s, &securityFlags)) + if (!rdp_read_security_header(s, &securityFlags, &length)) return -1; if (securityFlags & SEC_ENCRYPT) { - if (!rdp_decrypt(rdp, s, length - 4, securityFlags)) + if (!rdp_decrypt(rdp, s, length, securityFlags)) { DEBUG_WARN( "rdp_decrypt failed\n"); return -1; diff --git a/libfreerdp/core/rdp.c b/libfreerdp/core/rdp.c index 5360d53d6..4113347b2 100644 --- a/libfreerdp/core/rdp.c +++ b/libfreerdp/core/rdp.c @@ -77,13 +77,17 @@ const char* DATA_PDU_TYPE_STRINGS[80] = * @param flags security flags */ -BOOL rdp_read_security_header(wStream* s, UINT16* flags) +BOOL rdp_read_security_header(wStream* s, UINT16* flags, UINT16* length) { /* Basic Security Header */ - if (Stream_GetRemainingLength(s) < 4) + if (Stream_GetRemainingLength(s) < 4 || (length && (*length < 4))) return FALSE; Stream_Read_UINT16(s, *flags); /* flags */ Stream_Seek(s, 2); /* flagsHi (unused) */ + + if (length) + *length -= 4; + return TRUE; } @@ -284,7 +288,10 @@ BOOL rdp_read_header(rdpRdp* rdp, wStream* s, UINT16* length, UINT16* channelId) return FALSE; } - if ((size_t) (*length - 8) > Stream_GetRemainingLength(s)) + if (*length < 8) + return FALSE; + + if (*length - 8 > Stream_GetRemainingLength(s)) return FALSE; if (MCSPDU == DomainMCSPDU_DisconnectProviderUltimatum) @@ -334,8 +341,12 @@ BOOL rdp_read_header(rdpRdp* rdp, wStream* s, UINT16* length, UINT16* channelId) if (Stream_GetRemainingLength(s) < 5) return FALSE; - per_read_integer16(s, &initiator, MCS_BASE_CHANNEL_ID); /* initiator (UserId) */ - per_read_integer16(s, channelId, 0); /* channelId */ + if (!per_read_integer16(s, &initiator, MCS_BASE_CHANNEL_ID)) /* initiator (UserId) */ + return FALSE; + + if (!per_read_integer16(s, channelId, 0)) /* channelId */ + return FALSE; + Stream_Read_UINT8(s, byte); /* dataPriority + Segmentation (0x70) */ if (!per_read_length(s, length)) /* userData (OCTET_STRING) */ @@ -362,7 +373,7 @@ void rdp_write_header(rdpRdp* rdp, wStream* s, UINT16 length, UINT16 channelId) MCSPDU = (rdp->settings->ServerMode) ? DomainMCSPDU_SendDataIndication : DomainMCSPDU_SendDataRequest; - if ((rdp->sec_flags & SEC_ENCRYPT) && (rdp->settings->EncryptionMethods == ENCRYPTION_METHOD_FIPS)) + if ((rdp->sec_flags & SEC_ENCRYPT) && (rdp->settings->EncryptionMethods == ENCRYPTION_METHOD_FIPS)) { int pad; @@ -840,13 +851,8 @@ int rdp_recv_data_pdu(rdpRdp* rdp, wStream* s) return 0; } -int rdp_recv_message_channel_pdu(rdpRdp* rdp, wStream* s) +int rdp_recv_message_channel_pdu(rdpRdp* rdp, wStream* s, UINT16 securityFlags) { - UINT16 securityFlags; - - if (!rdp_read_security_header(s, &securityFlags)) - return -1; - if (securityFlags & SEC_AUTODETECT_REQ) { /* Server Auto-Detect Request PDU */ @@ -898,16 +904,20 @@ int rdp_recv_out_of_sequence_pdu(rdpRdp* rdp, wStream* s) * @param length int */ -BOOL rdp_decrypt(rdpRdp* rdp, wStream* s, int length, UINT16 securityFlags) +BOOL rdp_decrypt(rdpRdp* rdp, wStream* s, INT32 length, UINT16 securityFlags) { BYTE cmac[8]; BYTE wmac[8]; + if (!rdp || !s || length < 0) + return FALSE; + if (rdp->settings->EncryptionMethods == ENCRYPTION_METHOD_FIPS) { UINT16 len; BYTE version, pad; BYTE* sig; + INT64 padLength; if (Stream_GetRemainingLength(s) < 12) return FALSE; @@ -920,6 +930,10 @@ BOOL rdp_decrypt(rdpRdp* rdp, wStream* s, int length, UINT16 securityFlags) Stream_Seek(s, 8); /* signature */ length -= 12; + padLength = length - pad; + + if (length <= 0 || padLength <= 0) + return FALSE; if (!security_fips_decrypt(Stream_Pointer(s), length, rdp)) { @@ -937,11 +951,13 @@ BOOL rdp_decrypt(rdpRdp* rdp, wStream* s, int length, UINT16 securityFlags) return TRUE; } - if (Stream_GetRemainingLength(s) < 8) + if (Stream_GetRemainingLength(s) < sizeof(wmac)) return FALSE; Stream_Read(s, wmac, sizeof(wmac)); length -= sizeof(wmac); + if (length <= 0) + return FALSE; if (!security_decrypt(Stream_Pointer(s), length, rdp)) return FALSE; @@ -994,12 +1010,12 @@ static int rdp_recv_tpkt_pdu(rdpRdp* rdp, wStream* s) if (rdp->settings->DisableEncryption) { - if (!rdp_read_security_header(s, &securityFlags)) + if (!rdp_read_security_header(s, &securityFlags, &length)) return -1; if (securityFlags & (SEC_ENCRYPT | SEC_REDIRECTION_PKT)) { - if (!rdp_decrypt(rdp, s, length - 4, securityFlags)) + if (!rdp_decrypt(rdp, s, length, securityFlags)) { DEBUG_WARN( "rdp_decrypt failed\n"); return -1; @@ -1060,7 +1076,7 @@ static int rdp_recv_tpkt_pdu(rdpRdp* rdp, wStream* s) } else if (rdp->mcs->messageChannelId && channelId == rdp->mcs->messageChannelId) { - return rdp_recv_message_channel_pdu(rdp, s); + return rdp_recv_message_channel_pdu(rdp, s, securityFlags); } else { diff --git a/libfreerdp/core/rdp.h b/libfreerdp/core/rdp.h index f830e737b..9a2268d3f 100644 --- a/libfreerdp/core/rdp.h +++ b/libfreerdp/core/rdp.h @@ -170,7 +170,7 @@ struct rdp_rdp rdpSettings* settingsCopy; }; -BOOL rdp_read_security_header(wStream* s, UINT16* flags); +BOOL rdp_read_security_header(wStream* s, UINT16* flags, UINT16* length); void rdp_write_security_header(wStream* s, UINT16 flags); BOOL rdp_read_share_control_header(wStream* s, UINT16* length, UINT16* type, UINT16* channel_id); @@ -200,7 +200,7 @@ int rdp_send_channel_data(rdpRdp* rdp, UINT16 channelId, BYTE* data, int size); wStream* rdp_message_channel_pdu_init(rdpRdp* rdp); BOOL rdp_send_message_channel_pdu(rdpRdp* rdp, wStream* s, UINT16 sec_flags); -int rdp_recv_message_channel_pdu(rdpRdp* rdp, wStream* s); +int rdp_recv_message_channel_pdu(rdpRdp* rdp, wStream* s, UINT16 sec_flags); int rdp_recv_out_of_sequence_pdu(rdpRdp* rdp, wStream* s); @@ -217,7 +217,7 @@ void rdp_free(rdpRdp* rdp); #define DEBUG_RDP(fmt, ...) DEBUG_NULL(fmt, ## __VA_ARGS__) #endif -BOOL rdp_decrypt(rdpRdp* rdp, wStream* s, int length, UINT16 securityFlags); +BOOL rdp_decrypt(rdpRdp* rdp, wStream* s, INT32 length, UINT16 securityFlags); BOOL rdp_set_error_info(rdpRdp* rdp, UINT32 errorInfo); diff --git a/libfreerdp/core/security.c b/libfreerdp/core/security.c index 4bd8427c9..e99814d0a 100644 --- a/libfreerdp/core/security.c +++ b/libfreerdp/core/security.c @@ -564,7 +564,7 @@ BOOL security_key_update(BYTE* key, BYTE* update_key, int key_len, rdpRdp* rdp) return TRUE; } -BOOL security_encrypt(BYTE* data, int length, rdpRdp* rdp) +BOOL security_encrypt(BYTE* data, size_t length, rdpRdp* rdp) { if (rdp->encrypt_use_count >= 4096) { @@ -584,7 +584,7 @@ BOOL security_encrypt(BYTE* data, int length, rdpRdp* rdp) return TRUE; } -BOOL security_decrypt(BYTE* data, int length, rdpRdp* rdp) +BOOL security_decrypt(BYTE* data, size_t length, rdpRdp* rdp) { if (rdp->rc4_decrypt_key == NULL) return FALSE; @@ -607,7 +607,7 @@ BOOL security_decrypt(BYTE* data, int length, rdpRdp* rdp) return TRUE; } -void security_hmac_signature(const BYTE* data, int length, BYTE* output, rdpRdp* rdp) +void security_hmac_signature(const BYTE* data, size_t length, BYTE* output, rdpRdp* rdp) { BYTE buf[20]; BYTE use_count_le[4]; @@ -622,20 +622,20 @@ void security_hmac_signature(const BYTE* data, int length, BYTE* output, rdpRdp* memmove(output, buf, 8); } -BOOL security_fips_encrypt(BYTE* data, int length, rdpRdp* rdp) +BOOL security_fips_encrypt(BYTE* data, size_t length, rdpRdp* rdp) { crypto_des3_encrypt(rdp->fips_encrypt, length, data, data); rdp->encrypt_use_count++; return TRUE; } -BOOL security_fips_decrypt(BYTE* data, int length, rdpRdp* rdp) +BOOL security_fips_decrypt(BYTE* data, size_t length, rdpRdp* rdp) { crypto_des3_decrypt(rdp->fips_decrypt, length, data, data); return TRUE; } -BOOL security_fips_check_signature(const BYTE* data, int length, const BYTE* sig, rdpRdp* rdp) +BOOL security_fips_check_signature(const BYTE* data, size_t length, const BYTE* sig, rdpRdp* rdp) { BYTE buf[20]; BYTE use_count_le[4]; diff --git a/libfreerdp/core/security.h b/libfreerdp/core/security.h index ffcebdfdd..c6b603866 100644 --- a/libfreerdp/core/security.h +++ b/libfreerdp/core/security.h @@ -37,12 +37,12 @@ void security_mac_signature(rdpRdp *rdp, const BYTE* data, UINT32 length, BYTE* void security_salted_mac_signature(rdpRdp *rdp, const BYTE* data, UINT32 length, BOOL encryption, BYTE* output); BOOL security_establish_keys(const BYTE* client_random, rdpRdp* rdp); -BOOL security_encrypt(BYTE* data, int length, rdpRdp* rdp); -BOOL security_decrypt(BYTE* data, int length, rdpRdp* rdp); +BOOL security_encrypt(BYTE* data, size_t length, rdpRdp* rdp); +BOOL security_decrypt(BYTE* data, size_t length, rdpRdp* rdp); -void security_hmac_signature(const BYTE* data, int length, BYTE* output, rdpRdp* rdp); -BOOL security_fips_encrypt(BYTE* data, int length, rdpRdp* rdp); -BOOL security_fips_decrypt(BYTE* data, int length, rdpRdp* rdp); -BOOL security_fips_check_signature(const BYTE* data, int length, const BYTE* sig, rdpRdp* rdp); +void security_hmac_signature(const BYTE* data, size_t length, BYTE* output, rdpRdp* rdp); +BOOL security_fips_encrypt(BYTE* data, size_t length, rdpRdp* rdp); +BOOL security_fips_decrypt(BYTE* data, size_t length, rdpRdp* rdp); +BOOL security_fips_check_signature(const BYTE* data, size_t length, const BYTE* sig, rdpRdp* rdp); #endif /* __SECURITY_H */ diff --git a/libfreerdp/core/tpkt.c b/libfreerdp/core/tpkt.c index 5689d62e4..900e288fd 100644 --- a/libfreerdp/core/tpkt.c +++ b/libfreerdp/core/tpkt.c @@ -81,25 +81,37 @@ BOOL tpkt_verify_header(wStream* s) * @return length */ -UINT16 tpkt_read_header(wStream* s) +BOOL tpkt_read_header(wStream* s, UINT16* length) { BYTE version; - UINT16 length; + + if (Stream_GetRemainingLength(s) < 1) + return FALSE; Stream_Peek_UINT8(s, version); if (version == 3) { + UINT16 len; + + if (Stream_GetRemainingLength(s) < 4) + return FALSE; + Stream_Seek(s, 2); - Stream_Read_UINT16_BE(s, length); + Stream_Read_UINT16_BE(s, len); + + if (len < 4) + return FALSE; + + *length = len; } else { /* not a TPKT header */ - length = 0; + *length = 0; } - return length; + return TRUE; } /** diff --git a/libfreerdp/core/tpkt.h b/libfreerdp/core/tpkt.h index af984c11c..9b5174906 100644 --- a/libfreerdp/core/tpkt.h +++ b/libfreerdp/core/tpkt.h @@ -28,7 +28,7 @@ #define TPKT_HEADER_LENGTH 4 BOOL tpkt_verify_header(wStream* s); -UINT16 tpkt_read_header(wStream* s); +BOOL tpkt_read_header(wStream* s, UINT16* length); void tpkt_write_header(wStream* s, UINT16 length); #endif /* __TPKT_H */ ^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 2017-08-09 17:05 ` Thomas Danckaert @ 2017-08-09 21:34 ` Marius Bakke 2017-08-16 20:37 ` Thomas Danckaert 0 siblings, 1 reply; 8+ messages in thread From: Marius Bakke @ 2017-08-09 21:34 UTC (permalink / raw) To: Thomas Danckaert, leo; +Cc: 27939-done [-- Attachment #1: Type: text/plain, Size: 1717 bytes --] Thomas Danckaert <post@thomasdanckaert.be> writes: > From: Leo Famulari <leo@famulari.name> > Subject: Re: bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 > CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 > Date: Fri, 4 Aug 2017 10:56:15 -0400 > >> On Fri, Aug 04, 2017 at 10:34:55AM +0200, Thomas Danckaert wrote: >>> Unfortunately, vinagre doesn't build against freerdp 2. I'll try >>> to fix >>> that, or otherwise try to backport the patches to freerdp 1.x. >> >> I think it should not be too hard to backport the patches if that's >> what >> we need to do, but I don't have the time this week. > > I tried applying the patch for > https://github.com/FreeRDP/FreeRDP/commit/03ab68318966c3a22935a02838daaea7b7fbe96c > to freerdp@1.2.0-beta1+android9, fixed the conflicts, and came up > with the attached patch. I can confirm freerdp1.2beta with this > patch compiles and runs, but cannot guarantee this fixes all those > issues, because I'm totally unfamiliar with the code (and with rdp) > ... is this enough to create a freerdp-1.2 package? > > The alternative is to downgrade to freerdp@1.1, or to disable rdp > from vinagre. When I first submitted these packages, I ran into > trouble trying to build freerdp@1.1, but I don't remember exactly > what the problem was :). I doubt many users of Guix use RDP, disabling it in Vinagre until it supports the new version of FreeRDP sounds reasonable to me. Otherwise we're effectively "forking" FreeRDP, just for Vinagre. That said, since we have the backported patch already, I'm fine with either approach. But we should decide soon so Vinagre works again. :-) The patch looks good to my untrained eyes. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 2017-08-09 21:34 ` Marius Bakke @ 2017-08-16 20:37 ` Thomas Danckaert 2017-08-17 20:56 ` Leo Famulari 0 siblings, 1 reply; 8+ messages in thread From: Thomas Danckaert @ 2017-08-16 20:37 UTC (permalink / raw) To: mbakke; +Cc: 27939-done [-- Attachment #1: Type: Text/Plain, Size: 1301 bytes --] From: Marius Bakke <mbakke@fastmail.com> Subject: Re: bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 Date: Wed, 09 Aug 2017 23:34:27 +0200 >> The alternative is to downgrade to freerdp@1.1, or to disable rdp >> from vinagre. When I first submitted these packages, I ran into >> trouble trying to build freerdp@1.1, but I don't remember exactly >> what the problem was :). > > I doubt many users of Guix use RDP, disabling it in Vinagre until it > supports the new version of FreeRDP sounds reasonable to me. > Otherwise > we're effectively "forking" FreeRDP, just for Vinagre. > > That said, since we have the backported patch already, I'm fine with > either approach. But we should decide soon so Vinagre works again. > :-) > > The patch looks good to my untrained eyes. With some delay... here's a patch to revert freerdp to the tip of upstream branch 1.1 (which includes the CVE fixes, backported by the FreeRDP maintainers), and allow vinagre to build against that. Vinagre is the only Guix package which uses freerdp, so I think it's ok to just have freerdp branch 1.1 for now (1.1 is also the last “stable” branch). If you agree, I'll push this patch, and close this bug. cheers, Thomas [-- Attachment #2: 0001-gnu-freerdp-Revert-to-version-1.1.patch --] [-- Type: Text/X-Patch, Size: 24162 bytes --] From 66512fdd6e143bcb5debe84da502b480902d1244 Mon Sep 17 00:00:00 2001 From: Thomas Danckaert <thomas.danckaert@gmail.com> Date: Wed, 16 Aug 2017 21:49:17 +0200 Subject: [PATCH] gnu: freerdp: Revert to version 1.1. * gnu/packages/rdesktop.scm (freerdp) [version, source]: Revert to upstream branch 1.1. [inputs]: Use ffmpeg-2.8. * gnu/packages/gnome.scm (vinagre): Add patches required to build against freerdp branch 1.1. * gnu/packages/patches/vinagre-revert-1.patch, gnu/packages/patches/vinagre-revert-2.patch: New files. * gnu/local.mk (dist_patch_DATA): Add them. --- gnu/local.mk | 2 + gnu/packages/gnome.scm | 3 + gnu/packages/patches/vinagre-revert-1.patch | 56 ++++ gnu/packages/patches/vinagre-revert-2.patch | 448 ++++++++++++++++++++++++++++ gnu/packages/rdesktop.scm | 16 +- 5 files changed, 518 insertions(+), 7 deletions(-) create mode 100644 gnu/packages/patches/vinagre-revert-1.patch create mode 100644 gnu/packages/patches/vinagre-revert-2.patch diff --git a/gnu/local.mk b/gnu/local.mk index 2ab6901d8..172e92c0a 100644 --- a/gnu/local.mk +++ b/gnu/local.mk @@ -1082,6 +1082,8 @@ dist_patch_DATA = \ %D%/packages/patches/util-linux-tests.patch \ %D%/packages/patches/upower-builddir.patch \ %D%/packages/patches/valgrind-enable-arm.patch \ + %D%/packages/patches/vinagre-revert-1.patch \ + %D%/packages/patches/vinagre-revert-2.patch \ %D%/packages/patches/virglrenderer-CVE-2017-6386.patch \ %D%/packages/patches/vorbis-tools-CVE-2014-9638+CVE-2014-9639.patch \ %D%/packages/patches/vorbis-tools-CVE-2014-9640.patch \ diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm index b362ba5e2..dab450acb 100644 --- a/gnu/packages/gnome.scm +++ b/gnu/packages/gnome.scm @@ -2123,6 +2123,9 @@ selection and URL hints."))) (uri (string-append "mirror://gnome/sources/" name "/" (version-major+minor version) "/" name "-" version ".tar.xz")) + (patches ; We have to revert 2 commits to build against freerdp 1.1. + (search-patches "vinagre-revert-1.patch" + "vinagre-revert-2.patch")) (sha256 (base32 "10jya3jyrm18nbw3v410gbkc7677bqamax44pzgd3j15randn76d")))) diff --git a/gnu/packages/patches/vinagre-revert-1.patch b/gnu/packages/patches/vinagre-revert-1.patch new file mode 100644 index 000000000..5a983770b --- /dev/null +++ b/gnu/packages/patches/vinagre-revert-1.patch @@ -0,0 +1,56 @@ +Patch taken from Debian: revert changes that prevent building against freerdp +version 1.1 branch. + +From 8ebc0685b85e0d1f70eb00171f2e7712de3d44bd Mon Sep 17 00:00:00 2001 +From: Michael Biebl <biebl@debian.org> +Date: Thu, 22 Sep 2016 01:15:55 +0200 +Subject: [PATCH 1/2] Revert "Improve FreeRDP authentication failure handling" + +This reverts commit d7b4f88943e8615d252d27e1efc58cb64a9e1821. +--- + plugins/rdp/vinagre-rdp-tab.c | 10 ++++++---- + 1 file changed, 6 insertions(+), 4 deletions(-) + +diff --git a/plugins/rdp/vinagre-rdp-tab.c b/plugins/rdp/vinagre-rdp-tab.c +index b731f9b..8572bc3 100644 +--- a/plugins/rdp/vinagre-rdp-tab.c ++++ b/plugins/rdp/vinagre-rdp-tab.c +@@ -1195,8 +1195,8 @@ open_freerdp (VinagreRdpTab *rdp_tab) + VinagreTab *tab = VINAGRE_TAB (rdp_tab); + GtkWindow *window = GTK_WINDOW (vinagre_tab_get_window (tab)); + gboolean success = TRUE; ++ gboolean authentication_error = FALSE; + gboolean cancelled = FALSE; +- guint authentication_errors = 0; + + priv->events = g_queue_new (); + +@@ -1205,12 +1205,14 @@ open_freerdp (VinagreRdpTab *rdp_tab) + + do + { ++ authentication_error = FALSE; ++ + /* Run FreeRDP session */ + success = freerdp_connect (priv->freerdp_session); + if (!success) + { +- authentication_errors += freerdp_get_last_error (priv->freerdp_session->context) == 0x20009 || +- freerdp_get_last_error (priv->freerdp_session->context) == 0x2000c; ++ authentication_error = freerdp_get_last_error (priv->freerdp_session->context) == 0x20009 || ++ freerdp_get_last_error (priv->freerdp_session->context) == 0x2000c; + + cancelled = freerdp_get_last_error (priv->freerdp_session->context) == 0x2000b; + +@@ -1218,7 +1220,7 @@ open_freerdp (VinagreRdpTab *rdp_tab) + init_freerdp (rdp_tab); + } + } +- while (!success && authentication_errors < 3); ++ while (!success && authentication_error); + + if (!success) + { +-- +2.9.3 + diff --git a/gnu/packages/patches/vinagre-revert-2.patch b/gnu/packages/patches/vinagre-revert-2.patch new file mode 100644 index 000000000..686ee203e --- /dev/null +++ b/gnu/packages/patches/vinagre-revert-2.patch @@ -0,0 +1,448 @@ +Patch taken from Debian: revert changes that prevent building against freerdp +version 1.1 branch. + +From bb1828b6b7eb29bb037bcc687cf10f916ddc7561 Mon Sep 17 00:00:00 2001 +From: Michael Biebl <biebl@debian.org> +Date: Thu, 22 Sep 2016 01:18:16 +0200 +Subject: [PATCH 2/2] Revert "Store credentials for RDP" + +This reverts commit 60dea279a24c7f0e398b89a0a60d45e80087ed1d. +--- + plugins/rdp/vinagre-rdp-connection.c | 22 +--- + plugins/rdp/vinagre-rdp-plugin.c | 29 +---- + plugins/rdp/vinagre-rdp-tab.c | 231 +++++++++++++++++------------------ + 3 files changed, 123 insertions(+), 159 deletions(-) + +diff --git a/plugins/rdp/vinagre-rdp-connection.c b/plugins/rdp/vinagre-rdp-connection.c +index f0ff02b..c5f6ed1 100644 +--- a/plugins/rdp/vinagre-rdp-connection.c ++++ b/plugins/rdp/vinagre-rdp-connection.c +@@ -127,25 +127,9 @@ rdp_parse_item (VinagreConnection *conn, xmlNode *root) + static void + rdp_parse_options_widget (VinagreConnection *conn, GtkWidget *widget) + { +- const gchar *text; +- GtkWidget *u_entry, *d_entry, *spin_button, *scaling_button; +- gboolean scaling; +- guint width, height; +- +- d_entry = g_object_get_data (G_OBJECT (widget), "domain_entry"); +- if (!d_entry) +- { +- g_warning ("Wrong widget passed to rdp_parse_options_widget()"); +- return; +- } +- +- text = gtk_entry_get_text (GTK_ENTRY (d_entry)); +- vinagre_cache_prefs_set_string ("rdp-connection", "domain", text); +- +- g_object_set (conn, +- "domain", text != NULL && *text != '\0' ? text : NULL, +- NULL); +- ++ GtkWidget *u_entry, *spin_button, *scaling_button; ++ gboolean scaling; ++ guint width, height; + + u_entry = g_object_get_data (G_OBJECT (widget), "username_entry"); + if (!u_entry) +diff --git a/plugins/rdp/vinagre-rdp-plugin.c b/plugins/rdp/vinagre-rdp-plugin.c +index 4751102..f41da37 100644 +--- a/plugins/rdp/vinagre-rdp-plugin.c ++++ b/plugins/rdp/vinagre-rdp-plugin.c +@@ -100,7 +100,7 @@ vinagre_rdp_plugin_init (VinagreRdpPlugin *plugin) + static GtkWidget * + impl_get_connect_widget (VinagreProtocol *plugin, VinagreConnection *conn) + { +- GtkWidget *grid, *label, *u_entry, *d_entry, *spin_button, *check; ++ GtkWidget *grid, *label, *u_entry, *spin_button, *check; + gchar *str; + gint width, height; + +@@ -146,29 +146,10 @@ impl_get_connect_widget (VinagreProtocol *plugin, VinagreConnection *conn) + g_free (str); + + +- label = gtk_label_new_with_mnemonic (_("_Domain:")); +- gtk_misc_set_alignment (GTK_MISC (label), 0.0, 0.5); +- gtk_grid_attach (GTK_GRID (grid), label, 0, 3, 1, 1); +- gtk_widget_set_margin_left (label, 12); +- +- d_entry = gtk_entry_new (); +- /* Translators: This is the tooltip for the domain field in a RDP connection */ +- gtk_widget_set_tooltip_text (d_entry, _("Optional.")); +- g_object_set_data (G_OBJECT (grid), "domain_entry", d_entry); +- gtk_grid_attach (GTK_GRID (grid), d_entry, 1, 3, 1, 1); +- gtk_label_set_mnemonic_widget (GTK_LABEL (label), d_entry); +- str = g_strdup (VINAGRE_IS_CONNECTION (conn) ? +- vinagre_connection_get_domain (conn) : +- vinagre_cache_prefs_get_string ("rdp-connection", "domain", "")); +- gtk_entry_set_text (GTK_ENTRY (d_entry), str); +- gtk_entry_set_activates_default (GTK_ENTRY (d_entry), TRUE); +- g_free (str); +- +- + /* Host width */ + label = gtk_label_new_with_mnemonic (_("_Width:")); + gtk_misc_set_alignment (GTK_MISC (label), 0.0, 0.5); +- gtk_grid_attach (GTK_GRID (grid), label, 0, 4, 1, 1); ++ gtk_grid_attach (GTK_GRID (grid), label, 0, 3, 1, 1); + gtk_widget_set_margin_left (label, 12); + + spin_button = gtk_spin_button_new_with_range (MIN_SIZE, MAX_SIZE, 1); +@@ -176,7 +157,7 @@ impl_get_connect_widget (VinagreProtocol *plugin, VinagreConnection *conn) + gtk_widget_set_tooltip_text (spin_button, _("Set width of the remote desktop")); + gtk_spin_button_set_value (GTK_SPIN_BUTTON (spin_button), DEFAULT_WIDTH); + g_object_set_data (G_OBJECT (grid), "width_spin_button", spin_button); +- gtk_grid_attach (GTK_GRID (grid), spin_button, 1, 4, 1, 1); ++ gtk_grid_attach (GTK_GRID (grid), spin_button, 1, 3, 1, 1); + gtk_label_set_mnemonic_widget (GTK_LABEL (label), spin_button); + width = VINAGRE_IS_CONNECTION (conn) ? + vinagre_connection_get_width (conn) : +@@ -188,7 +169,7 @@ impl_get_connect_widget (VinagreProtocol *plugin, VinagreConnection *conn) + /* Host height */ + label = gtk_label_new_with_mnemonic (_("_Height:")); + gtk_misc_set_alignment (GTK_MISC (label), 0.0, 0.5); +- gtk_grid_attach (GTK_GRID (grid), label, 0, 5, 1, 1); ++ gtk_grid_attach (GTK_GRID (grid), label, 0, 4, 1, 1); + gtk_widget_set_margin_left (label, 12); + + spin_button = gtk_spin_button_new_with_range (MIN_SIZE, MAX_SIZE, 1); +@@ -196,7 +177,7 @@ impl_get_connect_widget (VinagreProtocol *plugin, VinagreConnection *conn) + gtk_widget_set_tooltip_text (spin_button, _("Set height of the remote desktop")); + gtk_spin_button_set_value (GTK_SPIN_BUTTON (spin_button), DEFAULT_HEIGHT); + g_object_set_data (G_OBJECT (grid), "height_spin_button", spin_button); +- gtk_grid_attach (GTK_GRID (grid), spin_button, 1, 5, 1, 1); ++ gtk_grid_attach (GTK_GRID (grid), spin_button, 1, 4, 1, 1); + gtk_label_set_mnemonic_widget (GTK_LABEL (label), spin_button); + height = VINAGRE_IS_CONNECTION (conn) ? + vinagre_connection_get_height (conn) : +diff --git a/plugins/rdp/vinagre-rdp-tab.c b/plugins/rdp/vinagre-rdp-tab.c +index 8572bc3..f3d9c08 100644 +--- a/plugins/rdp/vinagre-rdp-tab.c ++++ b/plugins/rdp/vinagre-rdp-tab.c +@@ -70,8 +70,6 @@ struct _VinagreRdpTabPrivate + gboolean scaling; + double scale; + double offset_x, offset_y; +- +- guint authentication_attempts; + }; + + G_DEFINE_TYPE (VinagreRdpTab, vinagre_rdp_tab, VINAGRE_TYPE_TAB) +@@ -611,7 +609,6 @@ frdp_post_connect (freerdp *instance) + 0, 0, + gdi->width, gdi->height); + +- vinagre_tab_save_credentials_in_keyring (VINAGRE_TAB (rdp_tab)); + vinagre_tab_add_recent_used (VINAGRE_TAB (rdp_tab)); + vinagre_tab_set_state (VINAGRE_TAB (rdp_tab), VINAGRE_TAB_STATE_CONNECTED); + +@@ -862,76 +859,114 @@ frdp_mouse_moved (GtkWidget *widget, + return TRUE; + } + ++static void ++entry_text_changed_cb (GtkEntry *entry, ++ GtkBuilder *builder) ++{ ++ const gchar *text; ++ GtkWidget *widget; ++ gsize username_length; ++ gsize password_length; ++ ++ widget = GTK_WIDGET (gtk_builder_get_object (builder, "username_entry")); ++ text = gtk_entry_get_text (GTK_ENTRY (widget)); ++ username_length = strlen (text); ++ ++ widget = GTK_WIDGET (gtk_builder_get_object (builder, "password_entry")); ++ text = gtk_entry_get_text (GTK_ENTRY (widget)); ++ password_length = strlen (text); ++ ++ widget = GTK_WIDGET (gtk_builder_get_object (builder, "ok_button")); ++ gtk_widget_set_sensitive (widget, password_length > 0 && username_length > 0); ++} ++ + static gboolean + frdp_authenticate (freerdp *instance, + char **username, + char **password, + char **domain) + { +- VinagreTab *tab = VINAGRE_TAB (((frdpContext *) instance->context)->rdp_tab); +- VinagreRdpTab *rdp_tab = VINAGRE_RDP_TAB (tab); +- VinagreRdpTabPrivate *priv = rdp_tab->priv; +- VinagreConnection *conn = vinagre_tab_get_conn (tab); +- GtkWindow *window = GTK_WINDOW (vinagre_tab_get_window (tab)); +- gboolean save_in_keyring = FALSE; +- gchar *keyring_domain = NULL; +- gchar *keyring_username = NULL; +- gchar *keyring_password = NULL; ++ VinagreTab *tab = VINAGRE_TAB (((frdpContext *) instance->context)->rdp_tab); ++ VinagreConnection *conn = vinagre_tab_get_conn (tab); ++ const gchar *user_name; ++ const gchar *domain_name; ++ GtkBuilder *builder; ++ GtkWidget *dialog; ++ GtkWidget *widget; ++ GtkWidget *username_entry; ++ GtkWidget *password_entry; ++ GtkWidget *domain_entry; ++ gboolean save_credential_check_visible; ++ gboolean domain_label_visible; ++ gboolean domain_entry_visible; ++ gint response; + +- priv->authentication_attempts++; ++ builder = vinagre_utils_get_builder (); + +- if (priv->authentication_attempts == 1) +- { +- vinagre_tab_find_credentials_in_keyring (tab, &keyring_domain, &keyring_username, &keyring_password); +- if (keyring_password != NULL && keyring_username != NULL) +- { +- *domain = keyring_domain; +- *username = keyring_username; +- *password = keyring_password; ++ dialog = GTK_WIDGET (gtk_builder_get_object (builder, "auth_required_dialog")); ++ gtk_window_set_modal ((GtkWindow *) dialog, TRUE); ++ gtk_window_set_transient_for ((GtkWindow *) dialog, GTK_WINDOW (vinagre_tab_get_window (tab))); + +- return TRUE; +- } +- else +- { +- g_free (keyring_domain); +- g_free (keyring_username); +- g_free (keyring_password); +- } ++ widget = GTK_WIDGET (gtk_builder_get_object (builder, "host_label")); ++ gtk_label_set_text (GTK_LABEL (widget), vinagre_connection_get_host (conn)); ++ ++ username_entry = GTK_WIDGET (gtk_builder_get_object (builder, "username_entry")); ++ password_entry = GTK_WIDGET (gtk_builder_get_object (builder, "password_entry")); ++ domain_entry = GTK_WIDGET (gtk_builder_get_object (builder, "domain_entry")); ++ ++ if (*username != NULL && *username[0] != '\0') ++ { ++ gtk_entry_set_text (GTK_ENTRY (username_entry), *username); ++ gtk_widget_grab_focus (password_entry); + } + +- if (vinagre_utils_request_credential (window, +- "RDP", +- vinagre_connection_get_host (conn), +- vinagre_connection_get_domain (conn), +- vinagre_connection_get_username (conn), +- TRUE, +- TRUE, +- TRUE, +- 20, +- domain, +- username, +- password, +- &save_in_keyring)) ++ g_signal_connect (username_entry, "changed", G_CALLBACK (entry_text_changed_cb), builder); ++ g_signal_connect (password_entry, "changed", G_CALLBACK (entry_text_changed_cb), builder); ++ ++ ++ widget = GTK_WIDGET (gtk_builder_get_object (builder, "save_credential_check")); ++ save_credential_check_visible = gtk_widget_get_visible (widget); ++ gtk_widget_set_visible (widget, FALSE); ++ ++ widget = GTK_WIDGET (gtk_builder_get_object (builder, "domain_label")); ++ domain_label_visible = gtk_widget_get_visible (widget); ++ gtk_widget_set_visible (widget, TRUE); ++ ++ domain_entry_visible = gtk_widget_get_visible (domain_entry); ++ gtk_widget_set_visible (domain_entry, TRUE); ++ ++ ++ response = gtk_dialog_run (GTK_DIALOG (dialog)); ++ gtk_widget_hide (dialog); ++ ++ ++ widget = GTK_WIDGET (gtk_builder_get_object (builder, "save_credential_check")); ++ gtk_widget_set_visible (widget, save_credential_check_visible); ++ ++ widget = GTK_WIDGET (gtk_builder_get_object (builder, "domain_label")); ++ gtk_widget_set_visible (widget, domain_label_visible); ++ ++ gtk_widget_set_visible (domain_entry, domain_entry_visible); ++ ++ ++ if (response == GTK_RESPONSE_OK) + { +- if (*domain && **domain != '\0') +- vinagre_connection_set_domain (conn, *domain); ++ domain_name = gtk_entry_get_text (GTK_ENTRY (domain_entry)); ++ if (g_strcmp0 (*domain, domain_name) != 0) ++ *domain = g_strdup (domain_name); + +- if (*username && **username != '\0') +- vinagre_connection_set_username (conn, *username); ++ user_name = gtk_entry_get_text (GTK_ENTRY (username_entry)); ++ if (g_strcmp0 (*username, user_name) != 0) ++ *username = g_strdup (user_name); + +- if (*password && **password != '\0') +- vinagre_connection_set_password (conn, *password); ++ *password = g_strdup (gtk_entry_get_text (GTK_ENTRY (password_entry))); + +- vinagre_tab_set_save_credentials (tab, save_in_keyring); ++ return TRUE; + } + else + { +- vinagre_tab_remove_from_notebook (tab); +- + return FALSE; + } +- +- return TRUE; + } + + static BOOL +@@ -1028,25 +1063,30 @@ frdp_changed_certificate_verify (freerdp *instance, + #endif + + static void +-init_freerdp (VinagreRdpTab *rdp_tab) ++open_freerdp (VinagreRdpTab *rdp_tab) + { + VinagreRdpTabPrivate *priv = rdp_tab->priv; +- rdpSettings *settings; + VinagreTab *tab = VINAGRE_TAB (rdp_tab); + VinagreConnection *conn = vinagre_tab_get_conn (tab); +- gboolean scaling; +- gchar *hostname; +- gint width, height; +- gint port; ++ rdpSettings *settings; ++ GtkWindow *window = GTK_WINDOW (vinagre_tab_get_window (tab)); ++ gboolean success = TRUE; ++ gboolean fullscreen, scaling; ++ gchar *hostname, *username; ++ gint port, width, height; + + g_object_get (conn, + "port", &port, + "host", &hostname, + "width", &width, + "height", &height, ++ "fullscreen", &fullscreen, + "scaling", &scaling, ++ "username", &username, + NULL); + ++ priv->events = g_queue_new (); ++ + /* Setup FreeRDP session */ + priv->freerdp_session = freerdp_new (); + priv->freerdp_session->PreConnect = frdp_pre_connect; +@@ -1111,6 +1151,17 @@ init_freerdp (VinagreRdpTab *rdp_tab) + settings->port = port; + #endif + ++ /* Set username */ ++ username = g_strstrip (username); ++ if (username != NULL && username[0] != '\0') ++ { ++#if HAVE_FREERDP_1_1 ++ settings->Username = g_strdup (username); ++#else ++ settings->username = g_strdup (username); ++#endif ++ } ++ + /* Set keyboard layout */ + #if HAVE_FREERDP_1_1 + freerdp_keyboard_init (KBD_US); +@@ -1120,24 +1171,6 @@ init_freerdp (VinagreRdpTab *rdp_tab) + + /* Allow font smoothing by default */ + settings->AllowFontSmoothing = TRUE; +-} +- +-static void +-init_display (VinagreRdpTab *rdp_tab) +-{ +- VinagreRdpTabPrivate *priv = rdp_tab->priv; +- VinagreTab *tab = VINAGRE_TAB (rdp_tab); +- VinagreConnection *conn = vinagre_tab_get_conn (tab); +- GtkWindow *window = GTK_WINDOW (vinagre_tab_get_window (tab)); +- gboolean fullscreen, scaling; +- gint width, height; +- +- g_object_get (conn, +- "width", &width, +- "height", &height, +- "fullscreen", &fullscreen, +- "scaling", &scaling, +- NULL); + + /* Setup display for FreeRDP session */ + priv->display = gtk_drawing_area_new (); +@@ -1186,54 +1219,20 @@ init_display (VinagreRdpTab *rdp_tab) + priv->key_release_handler_id = g_signal_connect (GTK_WIDGET (tab), "key-release-event", + G_CALLBACK (frdp_key_pressed), + rdp_tab); +-} +- +-static void +-open_freerdp (VinagreRdpTab *rdp_tab) +-{ +- VinagreRdpTabPrivate *priv = rdp_tab->priv; +- VinagreTab *tab = VINAGRE_TAB (rdp_tab); +- GtkWindow *window = GTK_WINDOW (vinagre_tab_get_window (tab)); +- gboolean success = TRUE; +- gboolean authentication_error = FALSE; +- gboolean cancelled = FALSE; +- +- priv->events = g_queue_new (); +- +- init_freerdp (rdp_tab); +- init_display (rdp_tab); +- +- do +- { +- authentication_error = FALSE; + +- /* Run FreeRDP session */ +- success = freerdp_connect (priv->freerdp_session); +- if (!success) +- { +- authentication_error = freerdp_get_last_error (priv->freerdp_session->context) == 0x20009 || +- freerdp_get_last_error (priv->freerdp_session->context) == 0x2000c; +- +- cancelled = freerdp_get_last_error (priv->freerdp_session->context) == 0x2000b; +- +- freerdp_free (priv->freerdp_session); +- init_freerdp (rdp_tab); +- } +- } +- while (!success && authentication_error); ++ /* Run FreeRDP session */ ++ success = freerdp_connect (priv->freerdp_session); + + if (!success) + { + gtk_window_unfullscreen (window); +- if (!cancelled) +- vinagre_utils_show_error_dialog (_("Error connecting to host."), +- NULL, +- window); ++ vinagre_utils_show_error_dialog (_("Error connecting to host."), ++ NULL, ++ window); + g_idle_add ((GSourceFunc) idle_close, rdp_tab); + } + else + { +- priv->authentication_attempts = 0; + priv->update_id = g_idle_add ((GSourceFunc) update, rdp_tab); + } + } +-- +2.9.3 + diff --git a/gnu/packages/rdesktop.scm b/gnu/packages/rdesktop.scm index 7946cde79..cf69cdaa2 100644 --- a/gnu/packages/rdesktop.scm +++ b/gnu/packages/rdesktop.scm @@ -72,14 +72,16 @@ to remotely control a user's Windows desktop.") (define-public freerdp (package (name "freerdp") - (version "2.0.0-rc0") + (version "1.1.0-beta") (source (origin - (method url-fetch) - (uri (string-append "https://github.com/FreeRDP/FreeRDP/archive/" - version ".tar.gz")) - (file-name (string-append name "-" version ".tar.gz")) + (method git-fetch) + (uri (git-reference + ;; We need the 1.1 branch for RDP support in vinagre. + (url "git://github.com/FreeRDP/FreeRDP.git") + (commit "03ab68318966c3a22935a02838daaea7b7fbe96c"))) + (file-name (git-file-name name version)) (sha256 - (base32 "0r36zwhl7fhmdng5pvl2a106gqbcqq184g2i2klz6ilna8pxjcml")))) + (base32 "07ish8rmvbk2zd99k91qybmmh5h4afly75l5kbvslhq1r6k8pbmp")))) (build-system cmake-build-system) (native-inputs `(("pkg-config" ,pkg-config) @@ -98,7 +100,7 @@ to remotely control a user's Windows desktop.") ("libxml2" ,libxml2) ("libxslt" ,libxslt) ("cups" ,cups) - ("ffmpeg" ,ffmpeg) + ("ffmpeg" ,ffmpeg-2.8) ("pulseaudio" ,pulseaudio) ("alsa-lib" ,alsa-lib) ("gstreamer" ,gstreamer) -- 2.13.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 2017-08-16 20:37 ` Thomas Danckaert @ 2017-08-17 20:56 ` Leo Famulari 0 siblings, 0 replies; 8+ messages in thread From: Leo Famulari @ 2017-08-17 20:56 UTC (permalink / raw) To: Thomas Danckaert; +Cc: 27939-done [-- Attachment #1: Type: text/plain, Size: 535 bytes --] On Wed, Aug 16, 2017 at 10:37:40PM +0200, Thomas Danckaert wrote: > With some delay... here's a patch to revert freerdp to the tip of upstream > branch 1.1 (which includes the CVE fixes, backported by the FreeRDP > maintainers), and allow vinagre to build against that. Vinagre is the only > Guix package which uses freerdp, so I think it's ok to just have freerdp > branch 1.1 for now (1.1 is also the last “stable” branch). > > If you agree, I'll push this patch, and close this bug. That sounds good to me. Thanks! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-08-17 20:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-03 22:05 bug#27939: FreeRDP CVE-2017-2834 CVE-2017-2835 CVE-2017-2836 CVE-2017-2837 CVE-2017-2838 CVE-2017-2839 Leo Famulari 2017-08-03 23:22 ` Marius Bakke 2017-08-04 8:34 ` Thomas Danckaert 2017-08-04 14:56 ` Leo Famulari 2017-08-09 17:05 ` Thomas Danckaert 2017-08-09 21:34 ` Marius Bakke 2017-08-16 20:37 ` Thomas Danckaert 2017-08-17 20:56 ` Leo Famulari
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/guix.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.