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.

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

Issue 2630 #2631

Closed
wants to merge 2 commits into from
Closed

Issue 2630 #2631

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
0b5a055
issue-2630: change handling of Password inside ConnectionInfo. Public…
magriggs Aug 19, 2024
9ab1707
Add additional comments in code.
magriggs Aug 19, 2024
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
4 changes: 2 additions & 2 deletions ...Config/Serializers/ConnectionSerializers/Csv/CsvConnectionsDeserializerMremotengFormat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ private ConnectionInfo ParseConnectionInfo(IList<string> headers, string[] conne
: "";

connectionRecord.Password = headers.Contains("Password")
? connectionCsv[headers.IndexOf("Password")].ConvertToSecureString()
: "".ConvertToSecureString();
? connectionCsv[headers.IndexOf("Password")] : "";


connectionRecord.Domain = headers.Contains("Domain")
? connectionCsv[headers.IndexOf("Domain")]
Expand Down
4 changes: 2 additions & 2 deletions ...G/Config/Serializers/ConnectionSerializers/Csv/CsvConnectionsSerializerMremotengFormat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ private void SerializeConnectionInfo(ConnectionInfo con, StringBuilder sb)
sb.Append(FormatForCsv(con.Username));

if (_saveFilter.SavePassword)
sb.Append(con.Password?.ConvertToUnsecureString() + ";");

sb.Append(con.GetPlaintextPassword() + ";");
if (_saveFilter.SaveDomain)
sb.Append(FormatForCsv(con.Domain));

Expand Down
2 changes: 1 addition & 1 deletion ...ers/ConnectionSerializers/Csv/RemoteDesktopManager/CsvConnectionsDeserializerRdmFormat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public ConnectionTreeModel Deserialize(string serializedData)
Hostname = hostString,
Port = port,
Username = username,
Password = password?.ConvertToSecureString(),
Password = password,
Domain = domain,
Icon = connectionType.IconName ?? "mRemoteNG",
Description = description,
Expand Down
13 changes: 7 additions & 6 deletions mRemoteNG/Config/Serializers/ConnectionSerializers/Sql/DataTableDeserializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,16 @@ private void PopulateConnectionInfoFromDatarow(DataRow dataRow, ConnectionInfo c
connectionInfo.OpeningCommand = (string)dataRow["OpeningCommand"];
connectionInfo.Panel = (string)dataRow["Panel"];
var pw = dataRow["Password"] as string;
connectionInfo.Password = DecryptValue(pw ?? "").ConvertToSecureString();
//connectionInfo.Password = DecryptValue(pw ?? "");
connectionInfo.SetPasswordFromSecureString(DecryptValue(pw ?? ""));
connectionInfo.Port = (int)dataRow["Port"];
connectionInfo.PostExtApp = (string)dataRow["PostExtApp"];
connectionInfo.PreExtApp = (string)dataRow["PreExtApp"];
connectionInfo.Protocol = (ProtocolType)Enum.Parse(typeof(ProtocolType), (string)dataRow["Protocol"]);
connectionInfo.PuttySession = (string)dataRow["PuttySession"];
connectionInfo.RDGatewayDomain = (string)dataRow["RDGatewayDomain"];
connectionInfo.RDGatewayHostname = (string)dataRow["RDGatewayHostname"];
connectionInfo.RDGatewayPassword = DecryptValue((string)dataRow["RDGatewayPassword"]);
connectionInfo.RDGatewayPassword = DecryptValue((string)dataRow["RDGatewayPassword"]).ConvertToUnsecureString();
connectionInfo.RDGatewayUsageMethod = (RDGatewayUsageMethod)Enum.Parse(typeof(RDGatewayUsageMethod), (string)dataRow["RDGatewayUsageMethod"]);
connectionInfo.RDGatewayUseConnectionCredentials = (RDGatewayUseConnectionCredentials)Enum.Parse(typeof(RDGatewayUseConnectionCredentials), (string)dataRow["RDGatewayUseConnectionCredentials"]);
connectionInfo.RDGatewayUsername = (string)dataRow["RDGatewayUsername"];
Expand Down Expand Up @@ -158,7 +159,7 @@ private void PopulateConnectionInfoFromDatarow(DataRow dataRow, ConnectionInfo c
connectionInfo.VNCCompression = (ProtocolVNC.Compression)Enum.Parse(typeof(ProtocolVNC.Compression), (string)dataRow["VNCCompression"]);
connectionInfo.VNCEncoding = (ProtocolVNC.Encoding)Enum.Parse(typeof(ProtocolVNC.Encoding), (string)dataRow["VNCEncoding"]);
connectionInfo.VNCProxyIP = (string)dataRow["VNCProxyIP"];
connectionInfo.VNCProxyPassword = DecryptValue((string)dataRow["VNCProxyPassword"]);
connectionInfo.VNCProxyPassword = DecryptValue((string)dataRow["VNCProxyPassword"]).ConvertToUnsecureString();
connectionInfo.VNCProxyPort = (int)dataRow["VNCProxyPort"];
connectionInfo.VNCProxyType = (ProtocolVNC.ProxyType)Enum.Parse(typeof(ProtocolVNC.ProxyType), (string)dataRow["VNCProxyType"]);
connectionInfo.VNCProxyUsername = (string)dataRow["VNCProxyUsername"];
Expand Down Expand Up @@ -245,16 +246,16 @@ private void PopulateConnectionInfoFromDatarow(DataRow dataRow, ConnectionInfo c
connectionInfo.Inheritance.VNCViewOnly = MiscTools.GetBooleanValue(dataRow["InheritVNCViewOnly"]);
}

private string DecryptValue(string cipherText)
private SecureString DecryptValue(string cipherText)
{
try
{
return _cryptographyProvider.Decrypt(cipherText, _decryptionKey);
return _cryptographyProvider.DecryptSecure(cipherText, _decryptionKey);
}
catch (EncryptionException)
{
// value may not be encrypted
return cipherText;
return cipherText.ConvertToSecureString();
}
}

Expand Down
4 changes: 2 additions & 2 deletions mRemoteNG/Config/Serializers/ConnectionSerializers/Sql/DataTableSerializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ private bool IsRowUpdated(ConnectionInfo connectionInfo, DataRow dataRow)
dataRow["InheritVNCViewOnly"].Equals(false);
}

bool pwd = dataRow["Password"].Equals(_saveFilter.SavePassword ? _cryptographyProvider.Encrypt(connectionInfo.Password?.ConvertToUnsecureString(), _encryptionKey) : "") &&
bool pwd = dataRow["Password"].Equals(_saveFilter.SavePassword ? _cryptographyProvider.Encrypt(connectionInfo.GetPassword(), _encryptionKey) : "") &&
dataRow["VNCProxyPassword"].Equals(_cryptographyProvider.Encrypt(connectionInfo.VNCProxyPassword, _encryptionKey)) &&
dataRow["RDGatewayPassword"].Equals(_cryptographyProvider.Encrypt(connectionInfo.RDGatewayPassword, _encryptionKey));
return !(pwd && isFieldNotChange && isInheritanceFieldNotChange);
Expand Down Expand Up @@ -575,7 +575,7 @@ private void SerializeConnectionInfo(ConnectionInfo connectionInfo)
dataRow["OpeningCommand"] = connectionInfo.OpeningCommand;
dataRow["Panel"] = connectionInfo.Panel;
dataRow["ParentID"] = connectionInfo.Parent?.ConstantID ?? "";
dataRow["Password"] = _saveFilter.SavePassword ? _cryptographyProvider.Encrypt(connectionInfo.Password?.ConvertToUnsecureString(), _encryptionKey) : "";
dataRow["Password"] = _saveFilter.SavePassword ? _cryptographyProvider.Encrypt(connectionInfo.GetPassword(), _encryptionKey) : "";
dataRow["Port"] = connectionInfo.Port;
dataRow["PositionID"] = _currentNodeIndex;
dataRow["PostExtApp"] = connectionInfo.PostExtApp;
Expand Down
2 changes: 1 addition & 1 deletion mRemoteNG/Config/Serializers/ConnectionSerializers/Xml/XmlConnectionNodeSerializer26.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private void SetElementAttributes(XContainer element, ConnectionInfo connectionI
: new XAttribute("Domain", ""));

if (_saveFilter.SavePassword && !connectionInfo.Inheritance.Password)
element.Add(new XAttribute("Password", _cryptographyProvider.Encrypt(connectionInfo.Password.ConvertToUnsecureString(), _encryptionKey)));
element.Add(new XAttribute("Password", _cryptographyProvider.Encrypt(connectionInfo.GetPassword(), _encryptionKey)));
else
element.Add(new XAttribute("Password", ""));

Expand Down
2 changes: 1 addition & 1 deletion mRemoteNG/Config/Serializers/ConnectionSerializers/Xml/XmlConnectionNodeSerializer27.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ private void SetElementAttributes(XContainer element, ConnectionInfo connectionI
: new XAttribute("Domain", ""));

if (_saveFilter.SavePassword && !connectionInfo.Inheritance.Password)
element.Add(new XAttribute("Password", _cryptographyProvider.Encrypt(connectionInfo.Password.ConvertToUnsecureString(), _encryptionKey)));
element.Add(new XAttribute("Password", _cryptographyProvider.Encrypt(connectionInfo.GetPassword(), _encryptionKey)));
else
element.Add(new XAttribute("Password", ""));
}
Expand Down
2 changes: 1 addition & 1 deletion mRemoteNG/Config/Serializers/ConnectionSerializers/Xml/XmlConnectionNodeSerializer28.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ private void SetElementAttributes(XContainer element, ConnectionInfo connectionI
: new XAttribute("Domain", ""));

if (_saveFilter.SavePassword && !connectionInfo.Inheritance.Password)
element.Add(new XAttribute("Password", _cryptographyProvider.Encrypt(connectionInfo.Password?.ConvertToUnsecureString(), _encryptionKey)));
element.Add(new XAttribute("Password", _cryptographyProvider.Encrypt(connectionInfo.GetPassword(), _encryptionKey)));
else
element.Add(new XAttribute("Password", ""));
}
Expand Down
8 changes: 4 additions & 4 deletions mRemoteNG/Config/Serializers/ConnectionSerializers/Xml/XmlConnectionsDeserializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public ConnectionTreeModel Deserialize(string xml, bool import)
bool fullFileEncryptionValue = rootXmlElement.GetAttributeAsBool("FullFileEncryption");
if (fullFileEncryptionValue)
{
string decryptedContent = _decryptor.Decrypt(rootXmlElement.InnerText);
string decryptedContent = _decryptor.DecryptUnsecure(rootXmlElement.InnerText);
rootXmlElement.InnerXml = decryptedContent;
}
}
Expand Down Expand Up @@ -218,7 +218,7 @@ private ConnectionInfo GetConnectionInfoFromXml(XmlNode xmlnode)
{
connectionInfo.Username = xmlnode.GetAttributeAsString("Username");
//connectionInfo.Password = _decryptor.Decrypt(xmlnode.GetAttributeAsString("Password"));
connectionInfo.Password = _decryptor.Decrypt(xmlnode.GetAttributeAsString("Password")).ConvertToSecureString();
connectionInfo.SetPasswordFromSecureString(_decryptor.Decrypt(xmlnode.GetAttributeAsString("Password")));
connectionInfo.Domain = xmlnode.GetAttributeAsString("Domain");
}
}
Expand Down Expand Up @@ -382,7 +382,7 @@ private ConnectionInfo GetConnectionInfoFromXml(XmlNode xmlnode)
connectionInfo.VNCProxyIP = xmlnode.GetAttributeAsString("VNCProxyIP");
connectionInfo.VNCProxyPort = xmlnode.GetAttributeAsInt("VNCProxyPort");
connectionInfo.VNCProxyUsername = xmlnode.GetAttributeAsString("VNCProxyUsername");
connectionInfo.VNCProxyPassword = _decryptor.Decrypt(xmlnode.GetAttributeAsString("VNCProxyPassword"));
connectionInfo.VNCProxyPassword = _decryptor.DecryptUnsecure(xmlnode.GetAttributeAsString("VNCProxyPassword"));
connectionInfo.VNCColors = xmlnode.GetAttributeAsEnum<ProtocolVNC.Colors>("VNCColors");
connectionInfo.VNCSmartSizeMode = xmlnode.GetAttributeAsEnum<ProtocolVNC.SmartSizeMode>("VNCSmartSizeMode");
connectionInfo.VNCViewOnly = xmlnode.GetAttributeAsBool("VNCViewOnly");
Expand Down Expand Up @@ -432,7 +432,7 @@ private ConnectionInfo GetConnectionInfoFromXml(XmlNode xmlnode)
connectionInfo.RDGatewayHostname = xmlnode.GetAttributeAsString("RDGatewayHostname");
connectionInfo.RDGatewayUseConnectionCredentials = xmlnode.GetAttributeAsEnum<RDGatewayUseConnectionCredentials>("RDGatewayUseConnectionCredentials");
connectionInfo.RDGatewayUsername = xmlnode.GetAttributeAsString("RDGatewayUsername");
connectionInfo.RDGatewayPassword = _decryptor.Decrypt(xmlnode.GetAttributeAsString("RDGatewayPassword"));
connectionInfo.RDGatewayPassword = _decryptor.DecryptUnsecure(xmlnode.GetAttributeAsString("RDGatewayPassword"));
connectionInfo.RDGatewayDomain = xmlnode.GetAttributeAsString("RDGatewayDomain");

// Get inheritance settings
Expand Down
2 changes: 1 addition & 1 deletion mRemoteNG/Config/Serializers/MiscSerializers/PuttyConnectionManagerDeserializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ private ConnectionInfo ConnectionInfoFromXml(XmlNode xmlNode)

XmlNode loginNode = xmlNode.SelectSingleNode("./login");
connectionInfo.Username = loginNode?.SelectSingleNode("login")?.InnerText;
connectionInfo.Password = loginNode?.SelectSingleNode("password")?.InnerText.ConvertToSecureString();
connectionInfo.Password = loginNode?.SelectSingleNode("password")?.InnerText;
// ./prompt

// ./timeout/connectiontimeout
Expand Down
6 changes: 3 additions & 3 deletions mRemoteNG/Config/Serializers/MiscSerializers/RemoteDesktopConnectionManagerDeserializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,12 @@ private static ConnectionInfo ConnectionInfoFromXml(XmlNode xmlNode)
if (_schemaVersion == 1) // Version 2.2 allows clear text passwords
{
connectionInfo.Password = passwordNode?.Attributes?["storeAsClearText"]?.Value == "True"
? passwordNode.InnerText.ConvertToSecureString()
: DecryptRdcManPassword(passwordNode?.InnerText).ConvertToSecureString();
? passwordNode.InnerText
: DecryptRdcManPassword(passwordNode?.InnerText);
}
else
{
connectionInfo.Password = DecryptRdcManPassword(passwordNode?.InnerText).ConvertToSecureString();
connectionInfo.Password = DecryptRdcManPassword(passwordNode?.InnerText);
}

connectionInfo.Domain = logonCredentialsNode.SelectSingleNode("./domain")?.InnerText ?? string.Empty;
Expand Down
10 changes: 9 additions & 1 deletion mRemoteNG/Config/Serializers/XmlConnectionsDecryptor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,21 @@ public XmlConnectionsDecryptor(BlockCipherEngines blockCipherEngine, BlockCipher
_rootNodeInfo = rootNodeInfo;
}

public string Decrypt(string plainText)
public SecureString Decrypt(string plainText)
{
return plainText == ""
? "".ConvertToSecureString()
: _cryptographyProvider.Decrypt(plainText, _rootNodeInfo.PasswordString.ConvertToSecureString()).ConvertToSecureString();
}

public string DecryptUnsecure(string plainText)
{
return plainText == ""
? ""
: _cryptographyProvider.Decrypt(plainText, _rootNodeInfo.PasswordString.ConvertToSecureString());
}


public string LegacyFullFileDecrypt(string xml)
{
if (string.IsNullOrEmpty(xml)) return "";
Expand Down
35 changes: 32 additions & 3 deletions mRemoteNG/Connection/AbstractConnectionRecord.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using mRemoteNG.Resources.Language;
using System.Runtime.Versioning;
using System.Security;
using mRemoteNG.Security;

namespace mRemoteNG.Connection
{
Expand Down Expand Up @@ -214,10 +215,38 @@ public virtual string Username
LocalizedAttributes.LocalizedDescription(nameof(Language.PropertyDescriptionPassword)),
PasswordPropertyText(true),
AttributeUsedInAllProtocolsExcept(ProtocolType.Telnet, ProtocolType.Rlogin, ProtocolType.RAW)]
public virtual SecureString Password
public virtual string Password
{
get => GetPropertyValue("Password", _password);
set => SetField(ref _password, value, "Password");
get
{
// ensure the UI shows that a password has been entered
return "ThisPasswordIsNeverUseful_useGetPasswordInstead";
}
set
{
// immediately convert a password held in an insecure string to a SecureString
SetPasswordFromSecureString(value.ConvertToSecureString());
value = string.Empty;
}
}

/// <summary>
/// Access the plaintext password field. Only use this at the point of connection, or for unit tests.
/// </summary>
/// <returns>The plaintext password not stored in a SecureString</returns>
public string GetPlaintextPassword()
{
if (_password != null)
return _password.ConvertToUnsecureString();
return string.Empty;
}

public SecureString GetPassword() { return _password; }

public void SetPasswordFromSecureString(SecureString securePassword)
{
this._password = securePassword;
RaisePropertyChangedEvent(this, new PropertyChangedEventArgs("Password"));
}

[LocalizedAttributes.LocalizedCategory(nameof(Language.Connection), 2),
Expand Down
2 changes: 1 addition & 1 deletion mRemoteNG/Connection/Protocol/PowerShell/Connection.Protocol.PowerShell.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ This approach avoids possible string conversion problems caused by double quotes

// Setup process for script with arguments
//* The -NoProfile parameter would be a valuable addition but should be able to be deactivated.
string arguments = $@"-NoExit -Command ""& {{ {psScriptBlock} }}"" -Hostname ""'{_connectionInfo.Hostname}'"" -Username ""'{psUsername}'"" -Password ""'{_connectionInfo.Password}'"" -LoginAttempts {psLoginAttempts}";
string arguments = $@"-NoExit -Command ""& {{ {psScriptBlock} }}"" -Hostname ""'{_connectionInfo.Hostname}'"" -Username ""'{psUsername}'"" -Password ""'{_connectionInfo.GetPlaintextPassword()}'"" -LoginAttempts {psLoginAttempts}";
string hostname = _connectionInfo.Hostname.Trim().ToLower();
bool useLocalHost = hostname == "" || hostname.Equals("localhost");
if (useLocalHost)
Expand Down
2 changes: 1 addition & 1 deletion mRemoteNG/Connection/Protocol/PuttyBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public override bool Connect()
{

string username = InterfaceControl.Info?.Username ?? "";
string password = InterfaceControl.Info?.Password.ConvertToUnsecureString() ?? "";
string password = InterfaceControl.Info?.GetPlaintextPassword() ?? "";
string domain = InterfaceControl.Info?.Domain ?? "";
string UserViaAPI = InterfaceControl.Info?.UserViaAPI ?? "";
string privatekey = "";
Expand Down
4 changes: 2 additions & 2 deletions mRemoteNG/Connection/Protocol/RDP/RdpProtocol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ private void SetRdGateway()
{
case RDGatewayUseConnectionCredentials.Yes:
_rdpClient.TransportSettings2.GatewayUsername = connectionInfo.Username;
_rdpClient.TransportSettings2.GatewayPassword = connectionInfo.Password.ConvertToUnsecureString();
_rdpClient.TransportSettings2.GatewayPassword = connectionInfo.GetPlaintextPassword();
_rdpClient.TransportSettings2.GatewayDomain = connectionInfo?.Domain;
break;
case RDGatewayUseConnectionCredentials.SmartCard:
Expand Down Expand Up @@ -540,7 +540,7 @@ private void SetCredentials()
string domain = connectionInfo?.Domain ?? "";
string userViaApi = connectionInfo?.UserViaAPI ?? "";
string pkey = "";
string password = (connectionInfo?.Password?.ConvertToUnsecureString() ?? "");
string password = (connectionInfo?.GetPlaintextPassword() ?? "");

// access secret server api if necessary
if (InterfaceControl.Info.ExternalCredentialProvider == ExternalCredentialProvider.DelineaSecretServer)
Expand Down
4 changes: 2 additions & 2 deletions mRemoteNG/Connection/Protocol/VNC/Connection.Protocol.VNC.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ private void SetEventHandlers()
_vnc.ConnectComplete += VNCEvent_Connected;
_vnc.ConnectionLost += VNCEvent_Disconnected;
FrmMain.ClipboardChanged += VNCEvent_ClipboardChanged;
if (!Force.HasFlag(ConnectionInfo.Force.NoCredentials) && _info?.Password?.Length > 0)
if (!Force.HasFlag(ConnectionInfo.Force.NoCredentials) && _info?.GetPassword()?.Length > 0)
{
_vnc.GetPassword = VNCEvent_Authenticate;
}
Expand Down Expand Up @@ -226,7 +226,7 @@ private void VNCEvent_ClipboardChanged()

private string VNCEvent_Authenticate()
{
return _info.Password.ConvertToUnsecureString();
return _info.GetPlaintextPassword();
}

#endregion
Expand Down
2 changes: 1 addition & 1 deletion mRemoteNG/Connection/PuttySessionInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public override string Panel

[ReadOnly(true)] public override string Username { get; set; }

[ReadOnly(true), Browsable(false)] public override SecureString Password { get; set; }
[ReadOnly(true), Browsable(false)] public override string Password { get; set; }

[ReadOnly(true)] public override ProtocolType Protocol { get; set; }

Expand Down
Loading