Using System.Security.Cryptography.ProtectedData, do you see any issue with encryption and encoding?

Question

The application needs to keep a secret which is known to logged in windows user. User enters secret in windows form's text box which is read by application. This is in form of System.String. Now I have to persist this secret into a XML file. And later able to decipher it back and use it.

Questions:

1. Do you see any concern with encoding done below?
2. Is there any concern with using System.String to begin with? I was considering to use System.Security.SecureString but at some point down the line the application needs to have base underlying string in clear text.

I'm using System.Security.Cryptography.ProtectedData to do the encryption.

The code

using System.Security.Cryptography;

static public string Encrypt(string password, string salt)
{
    byte[] passwordBytes = Encoding.Unicode.GetBytes(password);
    byte[] saltBytes = Encoding.Unicode.GetBytes(salt);

    byte[] cipherBytes = ProtectedData.Protect(passwordBytes, saltBytes, DataProtectionScope.CurrentUser);

    return Convert.ToBase64String(cipherBytes);
}

static public string Decrypt(string cipher, string salt)
{
    byte[] cipherBytes = Convert.FromBase64String(cipher);
    byte[] saltBytes = Encoding.Unicode.GetBytes(salt);

    byte[] passwordBytes = ProtectedData.Unprotect(cipherBytes, saltBytes, DataProtectionScope.CurrentUser);

    return Encoding.Unicode.GetString(passwordBytes);
}

Typical usage

[TestMethod()]
public void EncryptDecryptTest()
{
    string password = "Gussme!";
    string salt = new Random().Next().ToString();

    string cipher = Authenticator.Encrypt(password, salt);
    Assert.IsFalse(cipher.Contains(password), "Unable to encrypt");
    Assert.IsFalse(cipher.Contains(salt), "Unable to encrypt");

    string decipher = Authenticator.Decrypt(cipher, salt);
    Assert.AreEqual(password, decipher);
}

Answer 1

1. Your code is correct. I usually use Encoding.UTF8 because it is more compact for typical character sets.
2. SecureString is kind of a useless class. It is meant to never keep a secret unencrypted even in memory. This would only help if an attacker could read your processes memory. If he can do that you have lost already because he can read every character of your password the moment you use it or add it to the SecureString. Anyway, what attack scenario on earth allows an attacker to read your webserver process memory and not do other more dangerous things at the same time? In that case your box is probably compromised entirely. So you are right not using this class.