Skip to content

Commit

Permalink
Optimize Secure Message length (#777)
Browse files Browse the repository at this point in the history 
* Fix a possible memory leak

Instead of immediately returning from this function, previously
allocated resources need to be freed first. Also, use "goto err" just
for the sake of error handling consistency.

* Fix Secure Message output length

JavaThemis has the same issue as GoThemis and ThemisPP had: it
disregards the real size of Secure Message output as reported by the
second call to themis_secure_message_...() functions. The first call
will only report an upper bound to the output length, not the actual
length--which is only know after processing.

Unfortunately, JVM arrays cannot be resized after they are allocated
and JavaThemis API returns plain arrays. Hence, in order for the array
to have correct size, we need to buffer Themis output in the native
code before allocating a JVM array.

This has a side effect of adding more allocations to the happy path:
previously, if you were lucky, Themis Core could write directly into
JVM heap and no copying or reallocation would have taken place. Now
JNI code always allocates a temporary buffer and makes a copy of it.

(Another minor side effect is that in case of a failure JNI code no
longer allocates a JVM array beforehand, slightly relieving the GC
pressure.)

* Drop a line into the changelog

777 GET!

* Test for overlong signatures in Java

Port the test for overlong Secure Message signatures from GoThemis and
ThemisPP into Java land. Just to make sure.
  • Loading branch information
@ilammy
ilammy authored Feb 13, 2021
1 parent 2337228 commit 339e682
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 15 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ _Code:_
- Error `ErrOverflow` is now deprecated in favor of `ErrOutOfMemory`, new error types were added ([#711](https://github.com/cossacklabs/themis/pull/711)).
- `SecureMessage.Sign()` output is a bit smaller now ([#775](https://github.com/cossacklabs/themis/pull/775)).

- **Java / Kotlin**

- `SecureMessage#sign()` output is a bit smaller now ([#777](https://github.com/cossacklabs/themis/pull/777)).

- **Objective-C**

- Updated Objective-C examples (iOS and macOS, Carthage and CocoaPods) to showcase usage of the newest Secure Cell API: generating symmetric keys and using Secure Cell with Passphrase ([#688](https://github.com/cossacklabs/themis/pull/688)) and to use latest Themis 0.13.4 ([#701](https://github.com/cossacklabs/themis/pull/701), [#703](https://github.com/cossacklabs/themis/pull/703), [#706](https://github.com/cossacklabs/themis/pull/706), [#723](https://github.com/cossacklabs/themis/pull/723), [#724](https://github.com/cossacklabs/themis/pull/724), [#726](https://github.com/cossacklabs/themis/pull/726), [#740](https://github.com/cossacklabs/themis/pull/740)).
Expand Down
43 changes: 28 additions & 15 deletions jni/themis_message.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include <jni.h>
#include <string.h>
#include <themis/secure_message.h>
#include <themis/themis_error.h>

Expand All @@ -40,7 +41,8 @@ JNIEXPORT jbyteArray JNICALL Java_com_cossacklabs_themis_SecureMessage_process(J
jbyte* priv_buf = NULL;
jbyte* pub_buf = NULL;
jbyte* message_buf = NULL;
jbyte* output_buf = NULL;
jbyte* output_bytes = NULL;
uint8_t* output_buffer = NULL;

themis_status_t res = THEMIS_FAIL;
jbyteArray output = NULL;
Expand Down Expand Up @@ -124,16 +126,12 @@ JNIEXPORT jbyteArray JNICALL Java_com_cossacklabs_themis_SecureMessage_process(J
*/
if (output_length > INT32_MAX) {
res = THEMIS_NO_MEMORY;
return NULL;
}

output = (*env)->NewByteArray(env, output_length);
if (!output) {
goto err;
}

output_buf = (*env)->GetByteArrayElements(env, output, NULL);
if (!output_buf) {
output_buffer = malloc(output_length);
if (!output_buffer) {
res = THEMIS_NO_MEMORY;
goto err;
}

Expand All @@ -145,7 +143,7 @@ JNIEXPORT jbyteArray JNICALL Java_com_cossacklabs_themis_SecureMessage_process(J
public_key_length,
(uint8_t*)message_buf,
message_length,
(uint8_t*)output_buf,
output_buffer,
&output_length);
break;
case SECURE_MESSAGE_DECRYPT:
Expand All @@ -155,35 +153,50 @@ JNIEXPORT jbyteArray JNICALL Java_com_cossacklabs_themis_SecureMessage_process(J
public_key_length,
(uint8_t*)message_buf,
message_length,
(uint8_t*)output_buf,
output_buffer,
&output_length);
break;
case SECURE_MESSAGE_SIGN:
res = themis_secure_message_sign((uint8_t*)priv_buf,
private_key_length,
(uint8_t*)message_buf,
message_length,
(uint8_t*)output_buf,
output_buffer,
&output_length);
break;
case SECURE_MESSAGE_VERIFY:
res = themis_secure_message_verify((uint8_t*)pub_buf,
public_key_length,
(uint8_t*)message_buf,
message_length,
(uint8_t*)output_buf,
output_buffer,
&output_length);
break;
default:
res = THEMIS_NOT_SUPPORTED;
break;
}

err:
if (res != THEMIS_SUCCESS) {
goto err;
}

if (output_buf) {
(*env)->ReleaseByteArrayElements(env, output, output_buf, 0);
output = (*env)->NewByteArray(env, output_length);
if (!output) {
res = THEMIS_NO_MEMORY;
goto err;
}

output_bytes = (*env)->GetPrimitiveArrayCritical(env, output, NULL);
if (!output_bytes) {
res = THEMIS_FAIL;
goto err;
}
memmove(output_bytes, output_buffer, output_length);
(*env)->ReleasePrimitiveArrayCritical(env, output, output_bytes, 0);

err:
free(output_buffer);

if (message_buf) {
(*env)->ReleaseByteArrayElements(env, message, message_buf, 0);
Expand Down
136 changes: 136 additions & 0 deletions tests/themis/wrappers/android/com/cossacklabs/themis/test/SecureMessageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@

import java.util.Arrays;
import java.util.Random;
import java.nio.charset.StandardCharsets;

import com.cossacklabs.themis.KeyGenerationException;
import com.cossacklabs.themis.Keypair;
import com.cossacklabs.themis.KeypairGenerator;
import com.cossacklabs.themis.NullArgumentException;
import com.cossacklabs.themis.PublicKey;
import com.cossacklabs.themis.SecureMessage;
import com.cossacklabs.themis.SecureMessageWrapException;

Expand Down Expand Up @@ -88,4 +90,138 @@ public void runTest() {
fail(e.getClass().getCanonicalName());
}
}

// Make sure JavaThemis can handle overlong signed messages produced by
// old versions of GoThemis, ThemisPP, and JavaThemis itself.
@Test
public void verifyOverlongEllipticSignatures() {

final PublicKey publicKey = new PublicKey(asByteArray(new int[] {
0x55, 0x45, 0x43, 0x32, 0x00, 0x00, 0x00, 0x2d, 0x84, 0xa1, 0x02, 0xb2, 0x03, 0x32, 0x23,
0xfc, 0x44, 0xb0, 0x30, 0xe9, 0xed, 0x8b, 0x31, 0x79, 0xa3, 0xe6, 0x86, 0xb4, 0x6f, 0xda,
0xde, 0xb7, 0x77, 0xf0, 0x71, 0x37, 0x1e, 0xaf, 0x58, 0xc7, 0x04, 0xba, 0x6c, 0x82, 0x10,
}));

// NIST P-256 curve, 69..72 bytes long signatures, with and without zero padding
final byte[][] signatures = new byte[][] {
asByteArray(new int[] {
0x20, 0x26, 0x04, 0x26, 0x4f, 0x00, 0x00, 0x00, 0x48, 0x00, 0x00, 0x00, 0x42, 0x6f, 0x6c,
0xc3, 0xa9, 0x72, 0x6f, 0x20, 0x69, 0x73, 0x20, 0x61, 0x20, 0x6f, 0x6e, 0x65, 0x2d, 0x6d,
0x6f, 0x76, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x20, 0x6f, 0x72, 0x63, 0x68, 0x65, 0x73, 0x74,
0x72, 0x61, 0x6c, 0x20, 0x70, 0x69, 0x65, 0x63, 0x65, 0x20, 0x62, 0x79, 0x20, 0x74, 0x68,
0x65, 0x20, 0x46, 0x72, 0x65, 0x6e, 0x63, 0x68, 0x20, 0x63, 0x6f, 0x6d, 0x70, 0x6f, 0x73,
0x65, 0x72, 0x20, 0x4d, 0x61, 0x75, 0x72, 0x69, 0x63, 0x65, 0x20, 0x52, 0x61, 0x76, 0x65,
0x6c, 0x30, 0x46, 0x02, 0x21, 0x00, 0xd9, 0x14, 0xeb, 0xbb, 0x55, 0xc8, 0x12, 0x35, 0x6e,
0x39, 0x56, 0x61, 0x58, 0x88, 0x94, 0xeb, 0xc8, 0x02, 0x19, 0x2d, 0x6f, 0x2c, 0x86, 0xaf,
0x2b, 0x6f, 0xe1, 0xf1, 0x3a, 0x77, 0xe7, 0x23, 0x02, 0x21, 0x00, 0xf6, 0xb4, 0xf2, 0xd9,
0xfb, 0xb1, 0x93, 0x3e, 0xb2, 0x20, 0x1e, 0xf1, 0x4f, 0x8b, 0xcd, 0xa2, 0x1f, 0xb8, 0x21,
0x28, 0x44, 0xa5, 0xb1, 0xb1, 0x70, 0x35, 0x14, 0xae, 0xe1, 0x05, 0x5a, 0xa4,
}),
asByteArray(new int[] {
0x20, 0x26, 0x04, 0x26, 0x4f, 0x00, 0x00, 0x00, 0x47, 0x00, 0x00, 0x00, 0x42, 0x6f, 0x6c,
0xc3, 0xa9, 0x72, 0x6f, 0x20, 0x69, 0x73, 0x20, 0x61, 0x20, 0x6f, 0x6e, 0x65, 0x2d, 0x6d,
0x6f, 0x76, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x20, 0x6f, 0x72, 0x63, 0x68, 0x65, 0x73, 0x74,
0x72, 0x61, 0x6c, 0x20, 0x70, 0x69, 0x65, 0x63, 0x65, 0x20, 0x62, 0x79, 0x20, 0x74, 0x68,
0x65, 0x20, 0x46, 0x72, 0x65, 0x6e, 0x63, 0x68, 0x20, 0x63, 0x6f, 0x6d, 0x70, 0x6f, 0x73,
0x65, 0x72, 0x20, 0x4d, 0x61, 0x75, 0x72, 0x69, 0x63, 0x65, 0x20, 0x52, 0x61, 0x76, 0x65,
0x6c, 0x30, 0x45, 0x02, 0x20, 0x6c, 0x69, 0x2e, 0x82, 0x71, 0x78, 0x4c, 0xb9, 0x53, 0x41,
0x90, 0xc2, 0xe1, 0xf7, 0x8c, 0x21, 0xe8, 0xcc, 0xeb, 0x08, 0x67, 0xff, 0x8a, 0xfe, 0x58,
0x01, 0xbc, 0xf0, 0xb1, 0xab, 0x14, 0x72, 0x02, 0x21, 0x00, 0xa9, 0xbf, 0x8d, 0x6b, 0x6f,
0xc5, 0xf4, 0x5d, 0x2f, 0xb8, 0x14, 0xc6, 0xbd, 0xf2, 0x51, 0x4d, 0x3c, 0x81, 0xf7, 0x74,
0x65, 0x08, 0x45, 0x50, 0xe8, 0xa9, 0xa0, 0xe1, 0x09, 0x72, 0xf5, 0xc3, 0x00,
}),
asByteArray(new int[] {
0x20, 0x26, 0x04, 0x26, 0x4f, 0x00, 0x00, 0x00, 0x47, 0x00, 0x00, 0x00, 0x42, 0x6f, 0x6c,
0xc3, 0xa9, 0x72, 0x6f, 0x20, 0x69, 0x73, 0x20, 0x61, 0x20, 0x6f, 0x6e, 0x65, 0x2d, 0x6d,
0x6f, 0x76, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x20, 0x6f, 0x72, 0x63, 0x68, 0x65, 0x73, 0x74,
0x72, 0x61, 0x6c, 0x20, 0x70, 0x69, 0x65, 0x63, 0x65, 0x20, 0x62, 0x79, 0x20, 0x74, 0x68,
0x65, 0x20, 0x46, 0x72, 0x65, 0x6e, 0x63, 0x68, 0x20, 0x63, 0x6f, 0x6d, 0x70, 0x6f, 0x73,
0x65, 0x72, 0x20, 0x4d, 0x61, 0x75, 0x72, 0x69, 0x63, 0x65, 0x20, 0x52, 0x61, 0x76, 0x65,
0x6c, 0x30, 0x45, 0x02, 0x20, 0x6c, 0x69, 0x2e, 0x82, 0x71, 0x78, 0x4c, 0xb9, 0x53, 0x41,
0x90, 0xc2, 0xe1, 0xf7, 0x8c, 0x21, 0xe8, 0xcc, 0xeb, 0x08, 0x67, 0xff, 0x8a, 0xfe, 0x58,
0x01, 0xbc, 0xf0, 0xb1, 0xab, 0x14, 0x72, 0x02, 0x21, 0x00, 0xa9, 0xbf, 0x8d, 0x6b, 0x6f,
0xc5, 0xf4, 0x5d, 0x2f, 0xb8, 0x14, 0xc6, 0xbd, 0xf2, 0x51, 0x4d, 0x3c, 0x81, 0xf7, 0x74,
0x65, 0x08, 0x45, 0x50, 0xe8, 0xa9, 0xa0, 0xe1, 0x09, 0x72, 0xf5, 0xc3,
}),
asByteArray(new int[] {
0x20, 0x26, 0x04, 0x26, 0x4f, 0x00, 0x00, 0x00, 0x46, 0x00, 0x00, 0x00, 0x42, 0x6f, 0x6c,
0xc3, 0xa9, 0x72, 0x6f, 0x20, 0x69, 0x73, 0x20, 0x61, 0x20, 0x6f, 0x6e, 0x65, 0x2d, 0x6d,
0x6f, 0x76, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x20, 0x6f, 0x72, 0x63, 0x68, 0x65, 0x73, 0x74,
0x72, 0x61, 0x6c, 0x20, 0x70, 0x69, 0x65, 0x63, 0x65, 0x20, 0x62, 0x79, 0x20, 0x74, 0x68,
0x65, 0x20, 0x46, 0x72, 0x65, 0x6e, 0x63, 0x68, 0x20, 0x63, 0x6f, 0x6d, 0x70, 0x6f, 0x73,
0x65, 0x72, 0x20, 0x4d, 0x61, 0x75, 0x72, 0x69, 0x63, 0x65, 0x20, 0x52, 0x61, 0x76, 0x65,
0x6c, 0x30, 0x44, 0x02, 0x20, 0x4c, 0x0e, 0xd6, 0x8b, 0xa2, 0x74, 0x42, 0xf7, 0x0c, 0xdc,
0xe3, 0x28, 0xf8, 0x0b, 0xfe, 0x73, 0x1c, 0x94, 0x85, 0x97, 0x19, 0xd6, 0x27, 0x50, 0x52,
0xdb, 0xbf, 0x56, 0xa5, 0xbb, 0x21, 0xce, 0x02, 0x20, 0x46, 0x65, 0xc4, 0xa0, 0x6c, 0x9e,
0x7e, 0xff, 0xd6, 0xe0, 0x99, 0x79, 0x4e, 0x2b, 0x63, 0x29, 0x0a, 0xd1, 0x6e, 0x95, 0x9b,
0xd5, 0x12, 0x3a, 0x8e, 0x09, 0xf2, 0xb6, 0x2d, 0xe4, 0x87, 0x7c, 0x00, 0x00,
}),
asByteArray(new int[] {
0x20, 0x26, 0x04, 0x26, 0x4f, 0x00, 0x00, 0x00, 0x46, 0x00, 0x00, 0x00, 0x42, 0x6f, 0x6c,
0xc3, 0xa9, 0x72, 0x6f, 0x20, 0x69, 0x73, 0x20, 0x61, 0x20, 0x6f, 0x6e, 0x65, 0x2d, 0x6d,
0x6f, 0x76, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x20, 0x6f, 0x72, 0x63, 0x68, 0x65, 0x73, 0x74,
0x72, 0x61, 0x6c, 0x20, 0x70, 0x69, 0x65, 0x63, 0x65, 0x20, 0x62, 0x79, 0x20, 0x74, 0x68,
0x65, 0x20, 0x46, 0x72, 0x65, 0x6e, 0x63, 0x68, 0x20, 0x63, 0x6f, 0x6d, 0x70, 0x6f, 0x73,
0x65, 0x72, 0x20, 0x4d, 0x61, 0x75, 0x72, 0x69, 0x63, 0x65, 0x20, 0x52, 0x61, 0x76, 0x65,
0x6c, 0x30, 0x44, 0x02, 0x20, 0x4c, 0x0e, 0xd6, 0x8b, 0xa2, 0x74, 0x42, 0xf7, 0x0c, 0xdc,
0xe3, 0x28, 0xf8, 0x0b, 0xfe, 0x73, 0x1c, 0x94, 0x85, 0x97, 0x19, 0xd6, 0x27, 0x50, 0x52,
0xdb, 0xbf, 0x56, 0xa5, 0xbb, 0x21, 0xce, 0x02, 0x20, 0x46, 0x65, 0xc4, 0xa0, 0x6c, 0x9e,
0x7e, 0xff, 0xd6, 0xe0, 0x99, 0x79, 0x4e, 0x2b, 0x63, 0x29, 0x0a, 0xd1, 0x6e, 0x95, 0x9b,
0xd5, 0x12, 0x3a, 0x8e, 0x09, 0xf2, 0xb6, 0x2d, 0xe4, 0x87, 0x7c,
}),
asByteArray(new int[] {
0x20, 0x26, 0x04, 0x26, 0x4f, 0x00, 0x00, 0x00, 0x45, 0x00, 0x00, 0x00, 0x42, 0x6f, 0x6c,
0xc3, 0xa9, 0x72, 0x6f, 0x20, 0x69, 0x73, 0x20, 0x61, 0x20, 0x6f, 0x6e, 0x65, 0x2d, 0x6d,
0x6f, 0x76, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x20, 0x6f, 0x72, 0x63, 0x68, 0x65, 0x73, 0x74,
0x72, 0x61, 0x6c, 0x20, 0x70, 0x69, 0x65, 0x63, 0x65, 0x20, 0x62, 0x79, 0x20, 0x74, 0x68,
0x65, 0x20, 0x46, 0x72, 0x65, 0x6e, 0x63, 0x68, 0x20, 0x63, 0x6f, 0x6d, 0x70, 0x6f, 0x73,
0x65, 0x72, 0x20, 0x4d, 0x61, 0x75, 0x72, 0x69, 0x63, 0x65, 0x20, 0x52, 0x61, 0x76, 0x65,
0x6c, 0x30, 0x43, 0x02, 0x20, 0x20, 0x33, 0xd5, 0x62, 0xd7, 0xb2, 0x59, 0x76, 0x9e, 0xc1,
0xe5, 0x05, 0xee, 0xf0, 0x98, 0x1e, 0xde, 0xe6, 0x4d, 0xa7, 0x64, 0x72, 0xf7, 0xd8, 0x3c,
0x17, 0xa5, 0x51, 0x26, 0xd7, 0x70, 0xe5, 0x02, 0x1f, 0x3e, 0x02, 0x53, 0x15, 0xa2, 0x09,
0x0b, 0x3e, 0xc8, 0xbf, 0xca, 0x32, 0x97, 0x2d, 0x5e, 0x0a, 0x48, 0x50, 0x8b, 0x12, 0xc7,
0x52, 0x8c, 0x01, 0x0b, 0x2c, 0x9f, 0xd2, 0x27, 0x45, 0x90, 0x00, 0x00, 0x00,
}),
asByteArray(new int[] {
0x20, 0x26, 0x04, 0x26, 0x4f, 0x00, 0x00, 0x00, 0x45, 0x00, 0x00, 0x00, 0x42, 0x6f, 0x6c,
0xc3, 0xa9, 0x72, 0x6f, 0x20, 0x69, 0x73, 0x20, 0x61, 0x20, 0x6f, 0x6e, 0x65, 0x2d, 0x6d,
0x6f, 0x76, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x20, 0x6f, 0x72, 0x63, 0x68, 0x65, 0x73, 0x74,
0x72, 0x61, 0x6c, 0x20, 0x70, 0x69, 0x65, 0x63, 0x65, 0x20, 0x62, 0x79, 0x20, 0x74, 0x68,
0x65, 0x20, 0x46, 0x72, 0x65, 0x6e, 0x63, 0x68, 0x20, 0x63, 0x6f, 0x6d, 0x70, 0x6f, 0x73,
0x65, 0x72, 0x20, 0x4d, 0x61, 0x75, 0x72, 0x69, 0x63, 0x65, 0x20, 0x52, 0x61, 0x76, 0x65,
0x6c, 0x30, 0x43, 0x02, 0x20, 0x20, 0x33, 0xd5, 0x62, 0xd7, 0xb2, 0x59, 0x76, 0x9e, 0xc1,
0xe5, 0x05, 0xee, 0xf0, 0x98, 0x1e, 0xde, 0xe6, 0x4d, 0xa7, 0x64, 0x72, 0xf7, 0xd8, 0x3c,
0x17, 0xa5, 0x51, 0x26, 0xd7, 0x70, 0xe5, 0x02, 0x1f, 0x3e, 0x02, 0x53, 0x15, 0xa2, 0x09,
0x0b, 0x3e, 0xc8, 0xbf, 0xca, 0x32, 0x97, 0x2d, 0x5e, 0x0a, 0x48, 0x50, 0x8b, 0x12, 0xc7,
0x52, 0x8c, 0x01, 0x0b, 0x2c, 0x9f, 0xd2, 0x27, 0x45, 0x90,
}),
};

final byte[] expected = "Bol\u00E9ro is a one-movement orchestral piece by the French composer Maurice Ravel".getBytes(StandardCharsets.UTF_8);

SecureMessage smessage = new SecureMessage(publicKey);

for (int i = 0; i < signatures.length; i++) {
byte[] verified = null;
try {
verified = smessage.verify(signatures[i]);
} catch (SecureMessageWrapException e) {
fail("verification failed for signature #" + i + ": "
+ e.getClass().getCanonicalName() + ": "
+ e.getMessage());
}
assertArrayEquals(expected, verified);
}
}

// You cannot write byte literals in Java, and even if you could, they
// would be *signed*. Go with this roundabout way to keep blobs readable
// (for a certain definition thereof).
private static byte[] asByteArray(int[] array) {
byte[] bytes = new byte[array.length];
for (int i = 0; i < array.length; i++) {
bytes[i] = (byte)array[i];
}
return bytes;
}
}

0 comments on commit 339e682

Please sign in to comment.