Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

redesigned the framework to make it less vulnerable to native exceptions. #951

Closed
wants to merge 23 commits into from
Closed

redesigned the framework to make it less vulnerable to native exceptions. #951

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
300e0fa
improve the code with the help of new try-catch
radetsky Sep 2, 2022
8d2eb52
integrate isBase64 into code
radetsky Sep 15, 2022
a47fad8
review Obj-C code to make all try-catch and error checks
radetsky Sep 16, 2022
9a27632
fix an errors
radetsky Sep 16, 2022
241e0ac
review Java code
radetsky Sep 21, 2022
9907cb5
review Obj-C code
radetsky Sep 21, 2022
ecb3457
review Javascript code
radetsky Sep 21, 2022
0d9de20
moved old example and add ThemisTest for RN app
radetsky Sep 22, 2022
9c8377f
new Android Studio support file
radetsky Sep 22, 2022
90a0401
failed/passed fixes in order
radetsky Sep 23, 2022
c4eecd6
comparator can return nil without an error
radetsky Sep 23, 2022
6ecbec4
update example app
radetsky Sep 23, 2022
19496ce
Merge branch 'release/0.14' into rad-dev
radetsky Sep 23, 2022
e3a0322
changelog update
radetsky Sep 23, 2022
84218b2
check int value before casting to unsigned char
radetsky Sep 26, 2022
cdd443f
fix the compare procedure; var naming due the suggestion;
radetsky Sep 26, 2022
480702a
remove privateKey from secureMessageVerify functions
radetsky Sep 26, 2022
99342d1
remove publicKey from secureMessageSign64
radetsky Sep 26, 2022
297c93d
rewrite Obj-C code to use NSError instead of try/catch
radetsky Sep 29, 2022
cc5c3bb
fix the tests. The issue was in incorrect display of error
radetsky Sep 29, 2022
aa72af2
updated the code according to the comments
radetsky Sep 29, 2022
35ec1e9
updated the code according to the comments
radetsky Oct 2, 2022
913fec3
move examples to the previous state
radetsky Oct 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions docs/examples/react-native/ThemisTest/src/tests/12_verifyMessage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export async function testVerifyMessage(key) {
const privateKey64 = "UkVDMgAAAC36wNC7AAyZVdHhdB6ODitCszw7xZQy0FoPRLrNwXJ9KyIy6i1g";
const publicKey64 = "VUVDMgAAAC1wW3YTAsPE7kDJjwDXIBdXZwddni/PqwGk15BNDKuXJw2iHDoR";
const signed64 = "ICYEJg4AAABHAAAASGVsbG8sIFRoZW1pcyEwRQIgR/brzPiPPQRM2OxqJu+QlCLUATWhzCcUp50qsuARissCIQD0zrHKcZNn8Ao88JQsCmES2eoDZ8wisyjfg1DrrDeT3A==";
const verified = await secureMessageVerify64(signed64, privateKey64, publicKey64);
const verified = await secureMessageVerify64(signed64, publicKey64);
const component = <View key={ key }>
<Text style={ styles.title }>secureMessageVerify64</Text>
<Text style={ styles.text }>Wait for verified message</Text>
Expand All @@ -31,7 +31,7 @@ export async function testVerifyMessageWithoutPrivateKey(key) {
try {
const publicKey64 = "VUVDMgAAAC1wW3YTAsPE7kDJjwDXIBdXZwddni/PqwGk15BNDKuXJw2iHDoR";
const signed64 = "ICYEJg4AAABHAAAASGVsbG8sIFRoZW1pcyEwRQIgR/brzPiPPQRM2OxqJu+QlCLUATWhzCcUp50qsuARissCIQD0zrHKcZNn8Ao88JQsCmES2eoDZ8wisyjfg1DrrDeT3A==";
const verified = await secureMessageVerify64(signed64, null, publicKey64);
const verified = await secureMessageVerify64(signed64, publicKey64);
const component = <View key={ key }>
<Text style={ styles.title }>secureMessageVerify64</Text>
<Text style={ styles.text }>Wait for verified message without private key</Text>
Expand All @@ -54,7 +54,7 @@ export async function testVerifyMessageWithIncorrectPublicKey(key) {
try {
const publicKey64 = "VUVDMgAAAC1VlAYyAh742Ur7CbzIZH0LXpo1iplumKuNDMYhcqnaAU8YnoLL";
const signed64 = "ICYEJg4AAABHAAAASGVsbG8sIFRoZW1pcyEwRQIgR/brzPiPPQRM2OxqJu+QlCLUATWhzCcUp50qsuARissCIQD0zrHKcZNn8Ao88JQsCmES2eoDZ8wisyjfg1DrrDeT3A==";
const verified = await secureMessageVerify64(signed64, null, publicKey64);
const verified = await secureMessageVerify64(signed64, publicKey64);
const component = <View key={ key }>
<Text style={ styles.title }>secureMessageVerify64</Text>
<Text style={ styles.text }>Wait for error because of incorrect public key</Text>
Expand All @@ -77,7 +77,7 @@ export async function testVerifyMessageWithEmptyPublicKey(key) {
try {
const publicKey64 = "";
const signed64 = "ICYEJg4AAABHAAAASGVsbG8sIFRoZW1pcyEwRQIgR/brzPiPPQRM2OxqJu+QlCLUATWhzCcUp50qsuARissCIQD0zrHKcZNn8Ao88JQsCmES2eoDZ8wisyjfg1DrrDeT3A==";
const verified = await secureMessageVerify64(signed64, null, publicKey64);
const verified = await secureMessageVerify64(signed64, publicKey64);
const component = <View key={ key }>
<Text style={ styles.title }>secureMessageVerify64</Text>
<Text style={ styles.text }>Wait for error because of empty public key</Text>
Expand All @@ -100,7 +100,7 @@ export async function testVerifyMessageWithEmptyMessage(key) {
try {
const publicKey64 = "VUVDMgAAAC1VlAYyAh742Ur7CbzIZH0LXpo1iplumKuNDMYhcqnaAU8YnoLL";
const signed64 = "";
const verified = await secureMessageVerify64(signed64, null, publicKey64);
const verified = await secureMessageVerify64(signed64, publicKey64);
const component = <View key={ key }>
<Text style={ styles.title }>secureMessageVerify64</Text>
<Text style={ styles.text }>Wait for error because of empty message</Text>
Expand Down
12 changes: 3 additions & 9 deletions .../themis/react-native-themis/android/src/main/java/com/reactnativethemis/ThemisModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.reactnativethemis;

import android.util.Log;

import androidx.annotation.NonNull;

import com.cossacklabs.themis.AsymmetricKey;
Expand Down Expand Up @@ -388,7 +390,6 @@ public void secureMessageSign(String message,

@ReactMethod
public void secureMessageVerify(ReadableArray message,
ReadableArray privateKey,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it break backward compatibility? do we know that these methods are used anywhere or by someone?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little. Verify method now has only two required parameters. I deleted the middle one - an optional private key. The sign method is compatible.
I think we should describe the changes in our documentation for react-native-themis.
The GitHub search tool returns only 8 code results and all results contain in our repositories.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it break backward compatibility?

A little.

No such thing. Either applications are going to break after update, or they will not.

Aren't these methods semi-private? As in, they are an implementation detail of ReactNative Themis, and the users will not be calling them directly, but rather through JS code?

Though, I see JS API being changed as well, so it's a genuine breaking change.

Please, don't make breaking changes in patch releases, that's irresponsible.

In fact, better avoid making breaking changes at all until the next major version. Deprecations are okay though.

Or else, be honest, slap a prominent red banner in ReactNative documentation that the package is currently under development, is unstable, anything can be changed at any time, and it will be totally the user's fault if their application stops working after an update.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. I returned the middle param privateKey in the public API function. But mark it as unused and optional.

ReadableArray publicKey,
Callback successCallback,
Callback errorCallback) {
Expand All @@ -404,14 +405,7 @@ public void secureMessageVerify(ReadableArray message,
byte[] bkey = dataDeserialize(publicKey);
PublicKey pubKey = new PublicKey(bkey);
byte[] msg = dataDeserialize(message);
SecureMessage secureMessage;
if (privateKey == null || privateKey.size() == 0) {
secureMessage = new SecureMessage(pubKey);
} else {
bkey = dataDeserialize(privateKey);
PrivateKey pvtKey = new PrivateKey(bkey);
secureMessage = new SecureMessage(pvtKey, pubKey);
}
SecureMessage secureMessage = new SecureMessage(pubKey);
byte[] verifiedMessage = secureMessage.verify(msg, pubKey);
WritableArray response = dataSerialize(verifiedMessage);
successCallback.invoke(response);
Expand Down
8 changes: 1 addition & 7 deletions src/wrappers/themis/react-native-themis/ios/RCTThemis.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ + (NSData*)dataDeserialize:(NSArray*) data
];
@throw exception;
}

NSNumber *uchar_min = [NSNumber numberWithInt:0];
NSNumber *uchar_max = [NSNumber numberWithInt:255];

Expand Down Expand Up @@ -558,7 +557,6 @@ - (TSCellContextImprint *)newContextImprint:(NSArray*) symmetricKey
}

RCT_EXPORT_METHOD(secureMessageVerify:(NSArray*) message
privateKey:(NSArray*) privateKey
publicKey:(NSArray*) publicKey
successCallback:(RCTResponseSenderBlock)successCallback
errorCallback:(RCTResponseErrorBlock)errorCallback
Expand All @@ -571,14 +569,10 @@ - (TSCellContextImprint *)newContextImprint:(NSArray*) symmetricKey
return;
}

NSData* pvtKey;
NSData* pubKey;
NSData* msg;

@try {
if (privateKey != nil) {
pvtKey = [RCTThemis dataDeserialize:privateKey];
}
pubKey = [RCTThemis dataDeserialize:publicKey];
msg = [RCTThemis dataDeserialize:message];
}
Expand All @@ -589,7 +583,7 @@ - (TSCellContextImprint *)newContextImprint:(NSArray*) symmetricKey
return;
}

TSMessage *secureMessage = [[TSMessage alloc] initInSignVerifyModeWithPrivateKey:pvtKey
TSMessage *secureMessage = [[TSMessage alloc] initInSignVerifyModeWithPrivateKey:nil
peerPublicKey:pubKey];

NSError *error;
Expand Down
11 changes: 2 additions & 9 deletions src/wrappers/themis/react-native-themis/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ export function secureMessageSign64(plaintext, privateKey64, publicKey64 = "") {
});
});
}
export function secureMessageVerify64(signed64, privateKey64 = "", publicKey64) {
export function secureMessageVerify64(signed64, publicKey64) {
if (signed64 === "" || signed64 === undefined || signed64 === null) {
throw new Error("Parameter signed64 can not be empty");
}
Expand All @@ -266,16 +266,9 @@ export function secureMessageVerify64(signed64, privateKey64 = "", publicKey64)
throw new Error("Parameter publicKey64 is not base64 encoded");
}
const publicKey = Array.from(Buffer.from(publicKey64, 'base64'));
if (privateKey64 === undefined || privateKey64 === null) {
privateKey64 = "";
}
if (privateKey64 && !isBase64(privateKey64)) {
throw new Error("Optional parameter privateKey64 is not base64 encoded");
}
const privateKey = privateKey64 === "" ? null : Array.from(Buffer.from(privateKey64, 'base64'));
const signed = Array.from(Buffer.from(signed64, 'base64'));
return new Promise((resolve, reject) => {
Themis.secureMessageVerify(signed, privateKey, publicKey, (verified) => {
Themis.secureMessageVerify(signed, publicKey, (verified) => {
resolve(Buffer.from(new Uint8Array(verified)).toString());
}, (error) => {
reject(error);
Expand Down
12 changes: 1 addition & 11 deletions src/wrappers/themis/react-native-themis/src/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,6 @@ export function secureMessageSign64(

export function secureMessageVerify64(
signed64: String,
privateKey64: String = "",
publicKey64: String): Promise<string> {

if (signed64 === "" || signed64 === undefined || signed64 === null) {
Expand All @@ -354,19 +353,10 @@ export function secureMessageVerify64(
throw new Error("Parameter publicKey64 is not base64 encoded");
}
const publicKey = Array.from(Buffer.from(publicKey64, 'base64'));

if (privateKey64 === undefined || privateKey64 === null) {
privateKey64 = "";
}
if (privateKey64 && !isBase64(privateKey64)) {
throw new Error("Optional parameter privateKey64 is not base64 encoded");
}
const privateKey = privateKey64 === "" ? null : Array.from(Buffer.from(privateKey64, 'base64'));

const signed = Array.from(Buffer.from(signed64, 'base64'));

return new Promise((resolve, reject) => {
Themis.secureMessageVerify(signed, privateKey, publicKey, (verified: any) => {
Themis.secureMessageVerify(signed, publicKey, (verified: any) => {
resolve(Buffer.from(new Uint8Array(verified)).toString())
}, (error: any) => {
reject(error)
Expand Down