c272 / SharpRSA

An RSA implementation in pure C#, using the BigInt class.
8 stars 5 forks source link

Cannot encrypt/decrypt larger payload #2

Open stsrki opened 2 years ago

stsrki commented 2 years ago

Hello, I found your project while searching for a pure C# RSA library. It works well for smaller payloads like strings and byte arrays. But as soon as the payload is larger, I get an error.

"Bytes given are longer than length of key element n (129 bytes)."

Code to reproduce:

KeyPair kp = RSA.GenerateKeyPair( 1024 );
var raw = "Private keys cannot be passed over, as they are set to private, readonly and are not part of the DataContract specified. This is to prevent incorrect asymmetric key usage.";

//These bytes are now encrypted using RSA, of the bitlength specified before.
var encrypted = RSA.Encrypt( raw, kp.public_ );
var decrypted = RSA.Decrypt( encrypted, kp.private_ );

I have tried to split it into smaller chunks and then do the EncryptBytes separately, but when I try to decrypt, I get mismatches saying

"Marker byte for padding (0xFF) not found in plain bytes...


Is there any workaround that I need to do to make it possible to encrypt and decrypt larger content?

stsrki commented 2 years ago

This seems to work as expected.

public static string Encrypt( string text, Key key, Encoding encoding = null )
{
    encoding ??= Encoding;

    var chunkSize = key.n.ToByteArray().Length / 3;
    var encodedChunks = new List<string>();

    for ( int i = 0; i < text.Length; i += chunkSize )
    {
        var chunk = text.Skip( i ).Take( chunkSize ).ToArray();
        var bytes = encoding.GetBytes( chunk );
        var ciphed = EncryptBytes( bytes, key );

        encodedChunks.Add( Convert.ToBase64String( ciphed ) );
    }

    return string.Join( ';', encodedChunks );
}

public static string Decrypt( string text, Key key, Encoding encoding = null )
{
    encoding ??= Encoding;

    var encodedChunks = text.Split( ';', StringSplitOptions.None );
    var decodedChunks = new List<string>();

    foreach ( var chunk in encodedChunks )
    {
        byte[] bytes = Convert.FromBase64String( chunk );
        byte[] deciphed = DecryptBytes( bytes, key );

        decodedChunks.Add( encoding.GetString( deciphed ) );
    }

    return string.Join( string.Empty, decodedChunks );
}
stsrki commented 2 years ago

@c272 Sorry to bother you, but do you know how I can implement the Verify method in SharpRSA? The idea is to be able to verify the content on the client side by using the public key.

Use-case:

KeyPair kp = RSA.GenerateKeyPair( 1024 );
var raw = "message to send";

var encrypted = RSA.Encrypt( raw, kp.private_ );
var decrypted = RSA.Verify( encrypted, kp.public_ );
c272 commented 2 years ago

Hi @stsrki, the current architecture of SharpRSA does not add any magic numbers or static, known data to any payload apart from the padding bytes (0xFF) at the end. This makes implementing a Verify function that works with arbitrary encrypted data non-trivial (RSA hashing is not the same as RSA encryption). You must inherently know the expected content of the decrypted data to perform verification.

To quote this StackOverflow question on RSA signature verification:

Concluding RSA encrypted messages as signatures can be insufficient depending on the scenario, thus hash functions are commonly used in digital signature generation ...

You could try and decrypt the data, and detect the padding bytes at the end (match the final byte as 0xFF), however this is not reliable as a random garbage decryption could also result in the payload ending in 0xFF. One solution to this could be to append a header to every payload you have, such as 0xBADBEEF0 or any other arbitrary known bytes, and then verify that this magic number is present in the decrypted payload.

This wouldn't be something to implement at the library level I don't think, but I'm happy to accept suggestions.

c272 commented 2 years ago

This seems to work as expected.

public static string Encrypt( string text, Key key, Encoding encoding = null )
{
    encoding ??= Encoding;

    var chunkSize = key.n.ToByteArray().Length / 3;
    var encodedChunks = new List<string>();

    for ( int i = 0; i < text.Length; i += chunkSize )
    {
        var chunk = text.Skip( i ).Take( chunkSize ).ToArray();
        var bytes = encoding.GetBytes( chunk );
        var ciphed = EncryptBytes( bytes, key );

        encodedChunks.Add( Convert.ToBase64String( ciphed ) );
    }

    return string.Join( ';', encodedChunks );
}

public static string Decrypt( string text, Key key, Encoding encoding = null )
{
    encoding ??= Encoding;

    var encodedChunks = text.Split( ';', StringSplitOptions.None );
    var decodedChunks = new List<string>();

    foreach ( var chunk in encodedChunks )
    {
        byte[] bytes = Convert.FromBase64String( chunk );
        byte[] deciphed = DecryptBytes( bytes, key );

        decodedChunks.Add( encoding.GetString( deciphed ) );
    }

    return string.Join( string.Empty, decodedChunks );
}

Also, thank you very much for pointing out this issue! Raw byte encryption expects the user to split their bytes according to the size of the RSA modulo, however it doesn't make any sense for them to have to do so for string encryption! I can push a fix shortly utilising a chunking solution.

Alternatively, if you would like to make a pull request so you are listed as a contributor, this would be great too. Let me know what you think.

stsrki commented 2 years ago

I can create a PR easily if you think my solution is not too specific. For example, I have decided on the chunk size of key.n.ToByteArray().Length / 3, and that the splitter is ';'. But it can basically be different for other projects/users.

c272 commented 2 years ago

The chunk size should be key.n.ToByteArray().Length - 1, as this is the maximum available size for a chunk of bytes to be encrypted. Splitter wise, anything that is not present in base64 is perfectly acceptable!

stsrki commented 2 years ago

Hi @c272. I sent a PM a few days ago to an email found on your website: https://c272.org/. Did you receive it?

c272 commented 2 years ago

Hi @stsrki,

I've just read your email, and I've been thinking of changing the API a little bit to support these common use cases. The changes I was considering making are the following:

This, of course, would be a breaking change, so would turn up as a major release.

stsrki commented 2 years ago

That sounds good. I'm interested in how the actual API would look up in the end. Do you have any estimate on when you would begin working on these new features?

PS. my proposal is still open.