Skip to content

[pull] devel from neutrinolabs:devel #117

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 6, 2025
Merged

[pull] devel from neutrinolabs:devel #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
463e500
Security improvements
matt335672 Mar 10, 2025
b2d4077
Merge pull request #3455 from matt335672/additional_fips_checks
matt335672 May 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion common/xrdp_client_info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ struct display_size_description
unsigned int session_height;
};

/* Values used for the security_layer */
/* TODO: Make this an enum, and move it below xrdp_client_info */
#define SECURITY_LAYER_NEGOTIATE 0
#define SECURITY_LAYER_RDP 1
#define SECURITY_LAYER_TLS 2

enum client_resize_mode
{
CRMODE_NONE,
Expand Down Expand Up @@ -174,7 +180,7 @@ struct xrdp_client_info
int use_fast_path;
int require_credentials; /* when true, credentials *must* be passed on cmd line */

int security_layer; /* 0 = rdp, 1 = tls , 2 = hybrid */
int security_layer; /* SECURITY_LAYER_* */
int multimon; /* 0 = deny , 1 = allow */
struct display_size_description display_sizes;

Expand Down
4 changes: 4 additions & 0 deletions docs/man/xrdp-keygen.8.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ If \fBauto\fP is used as \fIoutfile\fP, the default file \fI@sysconfdir@/@syscon
.B test
Generate a test key pair and print information to standard output.

.SH NOTES
On machines with FIPS enabled, this program will generate an empty file,
and a warning. On these machines, xrdp cannot use Classic RDP encryption.

.SH FILES
.TP
.I @sysconfdir@/@sysconfsubdir@/rsakeys.ini
Expand Down
12 changes: 7 additions & 5 deletions docs/man/xrdp.ini.5.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ See section \fBCHANNELS\fP below for more fine grained options.
.TP
\fBcrypt_level\fP=\fI[low|medium|high|fips]\fP
.\" <http://blogs.msdn.com/b/openspecification/archive/2011/12/08/encryption-negotiation-in-rdp-connection.aspx>
Regulate encryption level of Standard RDP Security.
Regulate encryption level of Classic RDP Security.
This parameter is effective only if \fBsecurity_layer\fP is set to \fBrdp\fP or \fBnegotiate\fP.

Encryption in Standard RDP Security is controlled by two settings: \fIEncryption Level\fP
Encryption in Classic RDP Security is controlled by two settings: \fIEncryption Level\fP
and \fIEncryption Method\fP. The only supported \fIEncryption Method\fP are \fB40BIT_ENCRYPTION\fP
and \fB128BIT_ENCRYPTION\fP. \fB56BIT_ENCRYPTION\fP is not supported.
This option controls the \fIEncryption Level\fP:
Expand All @@ -86,7 +86,8 @@ the server's maximum key strength (sever compatible).
.TP
.B fips
All data sent between the client and server is protected using Federal Information
Processing Standard 140-1 validated encryption methods.
Processing Standard 140-1 validated encryption methods. Note that FIPS 140-1 is
no longer considered secure.
.I This level is required for Windows clients (mstsc.exe) if the client's group policy
.I enforces FIPS-compliance mode.
.RE
Expand Down Expand Up @@ -174,8 +175,9 @@ verification, and server authentication) are implemented by TLS.

.TP
.B rdp
Standard RDP Security, which is not safe from man-in-the-middle attack, is used. The encryption level
of Standard RDP Security is controlled by \fBcrypt_level\fP.
Classic RDP Security is used. The encryption level
of Classic RDP Security is controlled by \fBcrypt_level\fP.
Use this setting for testing only.

.TP
.B negotiate
Expand Down
83 changes: 44 additions & 39 deletions keygen/keygen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,13 @@ save_all(const char *e_data, int e_len, const char *n_data, int n_len,

if (fd != -1)
{
if (e_data == NULL)
{
/* FIPS mode */
g_file_close(fd);
return 0;
}

if (g_file_write(fd, "[keys]\n", 7) == -1)
{
g_writeln("problem writing to %s, maybe no rights", filename);
Expand All @@ -397,48 +404,49 @@ save_all(const char *e_data, int e_len, const char *n_data, int n_len,
static int
key_gen(const char *path_and_file_name)
{
char *e_data;
char *n_data;
char *d_data;
char *sign_data;
int e_len;
int n_len;
int d_len;
int sign_len;
int error;

e_data = (char *)g_exponent;
n_data = (char *)g_malloc(256, 0);
d_data = (char *)g_malloc(256, 0);
sign_data = (char *)g_malloc(64, 0);
e_len = 4;
n_len = g_key_size_bits / 8;
d_len = n_len;
sign_len = 64;
error = 0;
g_writeln("%s", "");
g_writeln("Generating %d bit rsa key...", g_key_size_bits);
g_writeln("%s", "");

if (error == 0)
char *e_data = NULL;
char n_data[256] = {0};
char d_data[256] = {0};
char sign_data[64] = {0};
int e_len = 4;
int n_len = g_key_size_bits / 8;
int d_len = n_len;
int sign_len = sizeof(sign_data);
int error = 0;

if (g_fips_mode_enabled())
{
error = ssl_gen_key_xrdp1(g_key_size_bits, e_data, e_len, n_data, n_len,
d_data, d_len);
if (error != 0)
{
g_writeln("error %d in key_gen, ssl_gen_key_xrdp1", error);
}
g_writeln("%s", "");
g_writeln("This machine is running in FIPS mode - keys will not be generated");
g_writeln("%s", "");
}

if (error == 0)
else
{
g_writeln("ssl_gen_key_xrdp1 ok");
e_data = (char *)g_exponent;
g_writeln("%s", "");
g_writeln("Generating %d bit rsa key...", g_key_size_bits);
g_writeln("%s", "");
error = sign_key(e_data, e_len, n_data, n_len, sign_data, sign_len);

if (error != 0)
if (error == 0)
{
g_writeln("error %d in key_gen, sign_key", error);
error = ssl_gen_key_xrdp1(g_key_size_bits, e_data, e_len, n_data, n_len,
d_data, d_len);
if (error != 0)
{
g_writeln("error %d in key_gen, ssl_gen_key_xrdp1", error);
}
}

if (error == 0)
{
g_writeln("ssl_gen_key_xrdp1 ok");
g_writeln("%s", "");
error = sign_key(e_data, e_len, n_data, n_len, sign_data, sign_len);

if (error != 0)
{
g_writeln("error %d in key_gen, sign_key", error);
}
}
}

Expand All @@ -453,9 +461,6 @@ key_gen(const char *path_and_file_name)
}
}

g_free(n_data);
g_free(d_data);
g_free(sign_data);
return error;
}

Expand Down
169 changes: 79 additions & 90 deletions libxrdp/xrdp_iso.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,20 +60,7 @@ protocol_mask_to_str(int protocol, char *buff, int bufflen)
BITMASK_STRING_END_OF_LIST
};

int rlen = g_bitmask_to_str(protocol, bits, delim, buff, bufflen);

/* Append "RDP" */
if (rlen == 0)
{
/* String is empty */
rlen = g_snprintf(buff, bufflen, "RDP");
}
else if (rlen > 0 && rlen < bufflen)
{
rlen += g_snprintf(buff + rlen, bufflen - rlen, "%cRDP", delim);
}

return rlen;
return g_bitmask_to_str(protocol, bits, delim, buff, bufflen);
}

/*****************************************************************************/
Expand Down Expand Up @@ -105,96 +92,98 @@ xrdp_iso_delete(struct xrdp_iso *self)
static int
xrdp_iso_negotiate_security(struct xrdp_iso *self)
{
char requested_str[64];
const char *selected_str = "";
const char *configured_str = "";

int rv = 0;
struct xrdp_client_info *client_info = &(self->mcs_layer->sec_layer->rdp_layer->client_info);
char protostr[64];
int got_protocol = 0;
int security_type_mask;

/* Can we do TLS/SSL? (basic check) */
int ssl_capable = g_file_readable(client_info->certificate) &&
g_file_readable(client_info->key_file);

/* Work out what's actually configured in xrdp.ini. The
* selection happens later, but we can do some error checking here */
switch (client_info->security_layer)
/* Map the configuration from xrdp.ini to a mask of allowed
* security types ([MS-RDPBCGR] 2.2.1.2.1)
*
* There's some oddness around PROTOCOL_RDP. This value is 0,
* for compatibility reasons, and it's OK for the server to
* suggest RDP as the fallback protocol if nothing else is
* agreed on. Nowadays, classic RDP security should
* not be used, if at all avoidable */

/* At present we only support SSL and RDP security */
if (client_info->security_layer == SECURITY_LAYER_RDP)
{
security_type_mask = PROTOCOL_RDP;
}
else
{
case PROTOCOL_RDP:
configured_str = "RDP";
break;
security_type_mask = PROTOCOL_SSL;
}

case PROTOCOL_SSL:
/* We *must* use TLS. Check we can offer it, and it's requested */
if (ssl_capable)
{
configured_str = "SSL";
if ((self->requestedProtocol & PROTOCOL_SSL) == 0)
{
LOG(LOG_LEVEL_ERROR, "Server requires TLS for security, "
"but the client did not request TLS.");
self->failureCode = SSL_REQUIRED_BY_SERVER;
rv = 1; /* error */
}
}
else
/* Logically 'and' this value with the mask requested by the client, and
* see what's left */
protocol_mask_to_str(self->requestedProtocol, protostr, sizeof(protostr));
LOG(LOG_LEVEL_INFO, "Client requested security types (RDP assumed) : %s",
protostr);
security_type_mask &= self->requestedProtocol;

/* Is there a match on SSL/TLS? */
if ((security_type_mask & PROTOCOL_SSL) != 0)
{
/* Can we do TLS? (basic check) */
if (g_file_readable(client_info->certificate) &&
g_file_readable(client_info->key_file))
{
LOG(LOG_LEVEL_INFO, "Selected TLS security");
self->selectedProtocol = PROTOCOL_SSL;
got_protocol = 1;
}
else
{
LOG(LOG_LEVEL_WARNING, "Cannot accept TLS connections because "
"certificate or private key file is not readable. "
"certificate file: [%s], private key file: [%s]",
client_info->certificate, client_info->key_file);

/* If we're configured to ONLY use TLS, this is a problem.
* If not, we can fall back to RDP */
if (client_info->security_layer == SECURITY_LAYER_TLS)
{
configured_str = "";
LOG(LOG_LEVEL_ERROR, "Cannot accept TLS connections because "
"certificate or private key file is not readable. "
"certificate file: [%s], private key file: [%s]",
client_info->certificate, client_info->key_file);
LOG(LOG_LEVEL_ERROR,
"Server requires TLS (security_layer=tls)");
self->failureCode = SSL_CERT_NOT_ON_SERVER;
rv = 1; /* error */
}
break;
case PROTOCOL_HYBRID:
case PROTOCOL_HYBRID_EX:
default:
/* We don't yet support CredSSP */
if (ssl_capable)
{
configured_str = "SSL|RDP";
rv = 1;
}
else
{
/*
* Tell the user we can't offer TLS, but this isn't fatal */
configured_str = "RDP";
LOG(LOG_LEVEL_WARNING, "Cannot accept TLS connections because "
"certificate or private key file is not readable. "
"certificate file: [%s], private key file: [%s]",
client_info->certificate, client_info->key_file);
}
break;
}

/* Currently the choice comes down to RDP or SSL */
if (rv != 0)
{
self->selectedProtocol = PROTOCOL_RDP;
selected_str = "";
}
}
else if (ssl_capable && (self->requestedProtocol &
client_info->security_layer &
PROTOCOL_SSL) != 0)
else if (client_info->security_layer == SECURITY_LAYER_TLS)
{
self->selectedProtocol = PROTOCOL_SSL;
selected_str = "SSL";
/* We don't have a match on TLS, but we'll accept nothing less */
LOG(LOG_LEVEL_ERROR, "Server requires TLS (security_layer=tls)");
self->failureCode = SSL_REQUIRED_BY_SERVER;
rv = 1;
}
else

/* If we haven't got a match so far, and we haven't got a fail,
* try RDP */
if (!got_protocol && !rv)
{
self->selectedProtocol = PROTOCOL_RDP;
selected_str = "RDP";
if (g_fips_mode_enabled())
{
/* This is a FIPS-mode machine, and we don't support classic RDP
* encryption */
LOG(LOG_LEVEL_ERROR,
"Server in FIPS mode requires TLS for security");
self->failureCode = SSL_REQUIRED_BY_SERVER;
rv = 1; /* error */
}
else
{
self->selectedProtocol = PROTOCOL_RDP;
LOG(LOG_LEVEL_INFO, "Selected classic RDP security");
LOG(LOG_LEVEL_WARNING, "Classic RDP security is not secure -"
" please configure TLS on the client and server");
got_protocol = 1;
}
}

protocol_mask_to_str(self->requestedProtocol,
requested_str, sizeof(requested_str));

LOG(LOG_LEVEL_INFO, "Security protocol: configured [%s], requested [%s],"
" selected [%s]",
configured_str, requested_str, selected_str);

return rv;
}

Expand Down
Loading