Seems like valid RSA keys aren't parsed properly

[EugeneKrapivin]

Hey!
Stumbled upon your library in some code, used to validate RSA private keys.
However, I've found that some cases where the parameter is starting with 00 byte the library would remove it, leaving the PEM it parsed not readable.
Interestingly enough, this happens only on windows, failed to reproduce on linux. I assume it's something with how the windows implementation of RSA in dotnet behaves creating values that may start with a 1 bit, requiring to pad the value with 00 byte.

I also might be missing something (possibly 😄)

The code creates a RSA pair, exports it to a PEM, tries to decode it with the library. If the decoding fails I print the following outputs:

1. the RSAParameters the library decoded
2. the RSAParameters from the originally created rsa pair
3. the RSAParameters from a version of RSA importing the same PEM that failed with the library.

here is some C# code that replicates it every time on windows.
Edit: I'm running on net9 and net6, on windows it reproduces quite constantly. On linux not once... maybe a difference between bcrypt and openssl?

using System.Security.Cryptography;

using OpenSSL.PrivateKeyDecoder;


for (int i = 0 ; i< 200; i++)
{
    var rsa = RSA.Create(512);
    var cert = rsa.ExportRSAPrivateKeyPem();

    var decoder = new OpenSSLPrivateKeyDecoder();
    try
    {
        var decoded = decoder.Decode(cert, null);
    }
    catch
    {
        Console.WriteLine(cert);
        
        PrintLineSeparator();

        Console.WriteLine("OpenSSLPrivateKeyDecoder:");
        var parameters = decoder.DecodeParameters(cert);
        Dump(parameters);

        PrintLineSeparator();

        Console.WriteLine("dotnet original:");

        Dump(rsa.ExportParameters(true));

        PrintLineSeparator();

        Console.WriteLine("dotnet reimport:");
        var rsa2 = RSA.Create();
        rsa2.ImportFromPem(cert);
        var reimported = rsa2.ExportParameters(true);
        Dump(reimported);

        PrintLineSeparator();

        CompareRSAParameters(parameters, reimported);

        break;
    }
}

void Dump(RSAParameters parameters)
{
    Console.WriteLine($"D:  {BitConverter.ToString(parameters.D!)}");
    Console.WriteLine($"DP: {BitConverter.ToString(parameters.DP!)}");
    Console.WriteLine($"DQ: {BitConverter.ToString(parameters.DQ!)}");
    Console.WriteLine($"EX: {BitConverter.ToString(parameters.Exponent!)}");
    Console.WriteLine($"IQ: {BitConverter.ToString(parameters.InverseQ!)}");
    Console.WriteLine($"M:  {BitConverter.ToString(parameters.Modulus!)}");
    Console.WriteLine($"P:  {BitConverter.ToString(parameters.P!)}");
    Console.WriteLine($"Q:  {BitConverter.ToString(parameters.Q!)}");
}

bool CompareRSAParameters(RSAParameters left, RSAParameters right)
{
    return CompareByteArrays(left.D, right.D, "D")
        || CompareByteArrays(left.DP, right.DP, "DP")
        || CompareByteArrays(left.DQ, right.DQ, "DQ")
        || CompareByteArrays(left.Exponent, right.Exponent, "Exponent")
        || CompareByteArrays(left.InverseQ, right.InverseQ, "InverseQ")
        || CompareByteArrays(left.Modulus, right.Modulus, "Modulus")
        || CompareByteArrays(left.P, right.P, "P")
        || CompareByteArrays(left.Q, right.Q, "Q");
}

bool CompareByteArrays(byte[]? left, byte[]? right, string title)
{
    if (left == null || right == null)
    {
        Console.WriteLine("One or both byte arrays are null.");
        return false;
    }

    int minLength = Math.Min(left.Length, right.Length);
    bool differenceFound = false;

    for (int i = 0; i < minLength; i++)
    {
        if (left[i] != right[i])
        {
            Console.WriteLine($"field diff: " + title);
            Console.WriteLine($"left:  {BitConverter.ToString(left)}");
            Console.ForegroundColor = ConsoleColor.Red;
            Console.WriteLine("       " + new string(' ', i * 3) + "\\/");
            Console.WriteLine("       " + new string(' ', i * 3) + "||");
            Console.WriteLine("       " + new string(' ', i * 3) + "\\/");
            Console.ResetColor();
            Console.WriteLine($"right: {BitConverter.ToString(right)}");
            differenceFound = true;
            break;
        }
    }

    if (left.Length != right.Length)
    {
        Console.WriteLine($"length diff for array: " + title);
        if (!differenceFound) { 
            Console.WriteLine("left:  " + BitConverter.ToString(left));
            Console.WriteLine("right: " + BitConverter.ToString(right));
        }
        Console.WriteLine("Arrays have different lengths.");
        Console.WriteLine($"{nameof(left)} Length: {left.Length}");
        Console.WriteLine($"{nameof(right)} Length: {right.Length}");
        differenceFound = true;
    }

    return differenceFound;
}

static void PrintLineSeparator()
{
    Console.WriteLine();
    Console.WriteLine("================================================");
    Console.WriteLine();
}

what are your thoughts on the matter?

[StefH]

@EugeneKrapivin
Actually, I don't have enough knowledge to assess your statements.

If you think you are correct and the code needs a fix, you can make a PR and maybe introduce a feature flag in order to not break previous functionality?