Generating (presumably) unique `nonce` fields for Twitter OAuth

Question

This is code that's part of a library (closed source) that I have that generates a unique nonce value for Twitter OAuth. Essentially, it generates a random number, combines it in string form with an extra string separated by a pipe, computes a Sha-1 hash, and then repeats the process if the Sha-1 hash (when converted to Base64) contained any non-alpha-numeric characters, which are subsequently stripped.

/// <summary>
/// Generates a (presumably) unique NONCE string for use in Twitter OAuth requests.
/// </summary>
/// <param name="extra">Extra data to add during generation.</param>
/// <returns>
/// A (presumably) unique NONCE string.
/// </returns>
/// <remarks>
/// While we can assume the generated NONCE strings will be somewhat unique, there is a small possibility that they will not be, and in such cases a new NONCE should be generated.
/// 
/// The generated NONCE values are 32 characters in length, and are generated by a Sha-1 hash and a Random generator, which generates four random numbers to be hashed until the generated string is 32 characters in length.
/// </remarks>
public string GenerateNonce(string extra = "")
{
 string result = "";
 SHA1 sha1 = SHA1.Create();
 Random rand = new Random();
 while (result.Length < 32)
 {
 string[] generatedRandoms = new string[4];
 for (int i = 0; i < 4; i++)
 {
 generatedRandoms[i] = rand.Next().ToString();
 }
 result += Convert.ToBase64String(sha1.ComputeHash(Encoding.ASCII.GetBytes(string.Join("", generatedRandoms) + "|" + extra))).Replace("=", "").Replace("/", "").Replace("+", "");
 }
 return result.Substring(0, 32);
}

The idea is to generate somewhat unique nonce strings for use with Twitter OAuth (et al.).

I use the term "presumably," because, while it's not considered a bug that the nonce strings will not be guaranteed to be unique, collisions are expected to be fairly rare. I do welcome any and all suggestions that would lead to more uniqueness.

• John Babb: There is also a Nonce Class - msdn.microsoft.com/en-us/library/…

Answer 1

If I understand the requirements correctly, Guid.NewGuid().ToString("N") would work for generating a nonce.

A GUID is a 128-bit integer (16 bytes) that can be used across all computers and networks wherever a unique identifier is required. Such an identifier has a very low probability of being duplicated.

The "N" format specifier will return the GUID as 32 (lowercase) hexadecimal digits.

• Der Kommissar: I feel like an idiot for missing that solution.

Answer 2

Although @mjolka has provided the way to go, I would like to review the code nevertheless.

public string GenerateNonce(string extra = "")
{
 string result = "";
 SHA1 sha1 = SHA1.Create();
 Random rand = new Random();
 while (result.Length < 32)
 {
 string[] generatedRandoms = new string[4];
 for (int i = 0; i < 4; i++)
 {
 generatedRandoms[i] = rand.Next().ToString();
 }
 result += Convert.ToBase64String(sha1.ComputeHash(Encoding.ASCII.GetBytes(string.Join("", generatedRandoms) + "|" + extra))).Replace("=", "").Replace("/", "").Replace("+", "");
 }
 return result.Substring(0, 32);
} 

There are a few things that bothers me.

1. the creation of the string[] generatedRandoms in a loop and later on using string.join() with an empty string as separator

   This should be replaced using a StringBuilder like so

   public static string GenerateNonce(string extra = "")
   {
    string result = "";
    SHA1 sha1 = SHA1.Create();
    Random rand = new Random();
    StringBuilder sb = new StringBuilder(1024);
    while (result.Length < 32)
    {
    sb.Length = 0;
    string[] generatedRandoms = new string[4];
    for (int i = 0; i < 4; i++)
    {
    sb.Append(rand.Next());
    }
    sb.Append("|")
    .Append(extra);
    result += Convert.ToBase64String(sha1.ComputeHash(Encoding.ASCII.GetBytes(sb.ToString()))).Replace("=", "").Replace("/", "").Replace("+", "");
    }
    return result.Substring(0, 32);
   }
   

2. this very long line of code where you add to the result

   This could be made pretier by using multiple lines like so

   result += Convert.ToBase64String(
    sha1.ComputeHash(Encoding.ASCII.GetBytes(sb.ToString()))
    ).Replace("=", "")
    .Replace("/", "")
    .Replace("+", ""); 
   

   in this way one wouldn't need to scroll to the right that much

3. the usage of Replace to replace a = from the string

   For a base64 string the only place a = can occur will be at the end of the string. So instead of using Replace you could do a simple TrimEnd which is a lot faster.

   public static string GenerateNonce(string extra = "")
   {
    string result = "";
    SHA1 sha1 = SHA1.Create();
    Random rand = new Random();
    StringBuilder sb = new StringBuilder(1024);
    while (result.Length < 32)
    {
    sb.Length = 0;
    string[] generatedRandoms = new string[4];
    for (int i = 0; i < 4; i++)
    {
    sb.Append(rand.Next());
    }
    sb.Append("|")
    .Append(extra);
    result += Convert.ToBase64String(
    sha1.ComputeHash(Encoding.ASCII.GetBytes(sb.ToString()))
    ).TrimEnd('=')
    .Replace("/", "")
    .Replace("+", "");
    }
    return result.Substring(0, 32);
   }
   

• Der Kommissar: This is exactly what I wanted it to do - the only change I made was replacing string result with a StringBuilder as well.