Update after a new review

java/android/OlmLibSdk/olm-sdk/src/androidTest/java/org/matrix/olm/OlmAccountTest.java

@@ -389,17 +389,12 @@ public class OlmAccountTest {
             assertTrue(e.getMessage(),false);
         }
 
-        boolean sessionRetCode = true;
-
         try {
-            sessionRetCode = olmAccount.removeOneTimeKeys(null);
+            olmAccount.removeOneTimeKeys(null);
         } catch (Exception e) {
             assertTrue(e.getMessage(), false);
         }
 
-        // test against no matching keys
-        assertFalse(sessionRetCode);
-
         olmAccount.releaseAccount();
     }
 

java/android/OlmLibSdk/olm-sdk/src/androidTest/java/org/matrix/olm/OlmSessionTest.java

@@ -174,13 +174,11 @@ public class OlmSessionTest {
         assertTrue(clearMsg.equals(decryptedMsg));
 
         // clean objects..
-        boolean res = false;
         try {
-            res = bobAccount.removeOneTimeKeys(bobSession);
+            bobAccount.removeOneTimeKeys(bobSession);
         } catch (Exception e) {
             assertTrue(e.getMessage(), false);
         }
-        assertTrue(res);
 
         // release accounts
         bobAccount.releaseAccount();
@@ -402,13 +400,11 @@ public class OlmSessionTest {
         assertTrue(clearMsg1.equals(decryptedMsg1));
 
         // clean objects..
-        boolean res = false;
         try {
-            res = bobAccount.removeOneTimeKeys(bobSession);
+            bobAccount.removeOneTimeKeys(bobSession);
         } catch (Exception e) {
             assertTrue(e.getMessage(), false);
         }
-        assertTrue(res);
 
         bobAccount.releaseAccount();
         aliceAccount.releaseAccount();
@@ -585,13 +581,11 @@ public class OlmSessionTest {
         //assertTrue(false==bobSession.matchesInboundSessionFrom(bobIdentityKey, encryptedAliceToBobMsg1.mCipherText));
 
         // release objects
-        boolean res = false;
         try {
-            res = bobAccount.removeOneTimeKeys(bobSession);
+            bobAccount.removeOneTimeKeys(bobSession);
         } catch (Exception e) {
             assertTrue(e.getMessage(), false);
         }
-        assertTrue(res);
 
         aliceAccount.releaseAccount();
         bobAccount.releaseAccount();
@@ -782,13 +776,11 @@ public class OlmSessionTest {
             assertTrue(clearMsg3.equals(decryptedMsg3));
 
             // clean objects..
-            boolean res = false;
             try {
-                res = bobAccount.removeOneTimeKeys(bobSession);
+                bobAccount.removeOneTimeKeys(bobSession);
             } catch (Exception e) {
                 assertTrue(e.getMessage(), false);
             }
-            assertTrue(res);
 
             bobAccount.releaseAccount();
             aliceAccount.releaseAccount();
@@ -1002,13 +994,11 @@ public class OlmSessionTest {
         assertTrue(!aliceSession.matchesInboundSessionFrom(null,null));
 
         // release objects
-        boolean res = false;
         try {
-            res = bobAccount.removeOneTimeKeys(bobSession);
+            bobAccount.removeOneTimeKeys(bobSession);
         } catch (Exception e) {
             assertTrue(e.getMessage(), false);
         }
-        assertTrue(res);
 
         aliceAccount.releaseAccount();
         bobAccount.releaseAccount();

java/android/OlmLibSdk/olm-sdk/src/androidTest/java/org/matrix/olm/OlmUtilityTest.java

@@ -104,42 +104,34 @@ public class OlmUtilityTest {
         }
 
         // verify signature
-        boolean isVerified;
-
-        isVerified = false;
         errorMsg = null;
         try {
-            isVerified = utility.verifyEd25519Signature(messageSignature, fingerPrintKey, message);
+            utility.verifyEd25519Signature(messageSignature, fingerPrintKey, message);
         } catch (Exception e) {
             errorMsg = e.getMessage();
         }
-        assertTrue(isVerified);
         assertTrue(TextUtils.isEmpty(errorMsg));
 
         // check a bad signature is detected => errorMsg = BAD_MESSAGE_MAC
         String badSignature = "Bad signature Bad signature Bad signature..";
 
-        isVerified = false;
         errorMsg = null;
         try {
-            isVerified = utility.verifyEd25519Signature(badSignature, fingerPrintKey, message);
+            utility.verifyEd25519Signature(badSignature, fingerPrintKey, message);
         } catch (Exception e) {
             errorMsg = e.getMessage();
         }
-        assertFalse(isVerified);
         assertTrue(!TextUtils.isEmpty(errorMsg));
 
         // check bad fingerprint size => errorMsg = INVALID_BASE64
         String badSizeFingerPrintKey = fingerPrintKey.substring(fingerPrintKey.length()/2);
 
-        isVerified = false;
         errorMsg = null;
         try {
-            isVerified = utility.verifyEd25519Signature(messageSignature, badSizeFingerPrintKey, message);
+            utility.verifyEd25519Signature(messageSignature, badSizeFingerPrintKey, message);
         } catch (Exception e) {
             errorMsg = e.getMessage();
         }
-        assertFalse(isVerified);
         assertTrue(!TextUtils.isEmpty(errorMsg));
 
         utility.releaseUtility();

java/android/OlmLibSdk/olm-sdk/src/main/java/org/matrix/olm/CommonSerializeUtils.java

@@ -70,17 +70,11 @@ abstract class CommonSerializeUtils {
             key = keyAsString.getBytes("UTF-8");
             pickledData = pickledDataAsString.getBytes("UTF-8");
 
+            deserialize(pickledData, key);
         } catch (Exception e) {
             throw new OlmException(OlmException.EXCEPTION_CODE_ACCOUNT_DESERIALIZATION, e.getMessage());
         }
 
-        if (null == key) {
-            throw new OlmException(OlmException.EXCEPTION_CODE_ACCOUNT_DESERIALIZATION, OlmException.EXCEPTION_MSG_INVALID_PARAMS_DESERIALIZATION+" key");
-        } else if (null == pickledData) {
-            throw new OlmException(OlmException.EXCEPTION_CODE_ACCOUNT_DESERIALIZATION, OlmException.EXCEPTION_MSG_INVALID_PARAMS_DESERIALIZATION+" pickle");
-        }
-
-        deserialize(pickledData, key);
         Log.d(LOG_TAG,"## deserializeObject(): success");
     }
 

java/android/OlmLibSdk/olm-sdk/src/main/java/org/matrix/olm/OlmAccount.java

@@ -242,23 +242,16 @@ public class OlmAccount extends CommonSerializeUtils implements Serializable {
     /**
      * Remove the "one time keys" that the session used from the account.
      * @param aSession session instance
-     * @return true if the operation succeeded.
      * @throws OlmException the failure reason
      */
-    public boolean removeOneTimeKeys(OlmSession aSession) throws OlmException {
+    public void removeOneTimeKeys(OlmSession aSession) throws OlmException {
-        boolean res = false;
-
         if (null != aSession) {
             try {
                 removeOneTimeKeysJni(aSession.getOlmSessionId());
-                res = true;
-                Log.d(LOG_TAG,"## removeOneTimeKeysForSession(): result=" + res);
             } catch (Exception e) {
                 throw new OlmException(OlmException.EXCEPTION_CODE_ACCOUNT_REMOVE_ONE_TIME_KEYS, e.getMessage());
             }
         }
-
-        return res;
     }
 
     /**

java/android/OlmLibSdk/olm-sdk/src/main/java/org/matrix/olm/OlmUtility.java

@@ -73,17 +73,14 @@ public class OlmUtility {
 
     /**
      * Verify an ed25519 signature.<br>
-     * If the signature is verified, the method returns true. If false is returned, an error description is provided in aError.
+     * An exception is thrown if the operation fails.
-     * If the key was too small, aError is set to "OLM.INVALID_BASE64".
-     * If the signature was invalid, aError is set to "OLM.BAD_MESSAGE_MAC".<br>
      * @param aSignature the base64-encoded message signature to be checked.
      * @param aFingerprintKey the ed25519 key (fingerprint key)
      * @param aMessage the signed message
-     * @return true if the signature is verified, false otherwise
      * @exception OlmException the failure reason
      */
-    public boolean verifyEd25519Signature(String aSignature, String aFingerprintKey, String aMessage) throws OlmException {
+    public void verifyEd25519Signature(String aSignature, String aFingerprintKey, String aMessage) throws OlmException {
-        String errorMessage = null;
+        String errorMessage;
 
         try {
             if (TextUtils.isEmpty(aSignature) || TextUtils.isEmpty(aFingerprintKey) || TextUtils.isEmpty(aMessage)) {
@@ -94,13 +91,12 @@ public class OlmUtility {
             }
         } catch (Exception e) {
             Log.e(LOG_TAG, "## verifyEd25519Signature(): failed " + e.getMessage());
+            errorMessage = e.getMessage();
         }
 
         if (!TextUtils.isEmpty(errorMessage)) {
             throw new OlmException(OlmException.EXCEPTION_CODE_UTILITY_VERIFY_SIGNATURE, errorMessage);
         }
-
-        return true;
     }
 
     /**

java/android/OlmLibSdk/olm-sdk/src/main/jni/olm_account.cpp

@@ -92,6 +92,7 @@ JNIEXPORT jlong OLM_ACCOUNT_FUNC_DEF(createNewAccountJni)(JNIEnv *env, jobject t
 
         if (randomBuffPtr)
         {
+            memset(randomBuffPtr, 0, randomSize);
             free(randomBuffPtr);
         }
     }
@@ -286,6 +287,7 @@ JNIEXPORT void OLM_ACCOUNT_FUNC_DEF(generateOneTimeKeysJni)(JNIEnv *env, jobject
 
         if (randomBufferPtr)
         {
+            memset(randomBufferPtr, 0, randomLength);
             free(randomBufferPtr);
         }
     }
@@ -642,7 +644,7 @@ JNIEXPORT jlong OLM_ACCOUNT_FUNC_DEF(deserializeJni)(JNIEnv *env, jobject thiz,
         size_t pickledLength = (size_t)env->GetArrayLength(aSerializedDataBuffer);
         size_t keyLength = (size_t)env->GetArrayLength(aKeyBuffer);
         LOGD(" ## deserializeJni(): pickledLength=%lu keyLength=%lu",static_cast<long unsigned int>(pickledLength), static_cast<long unsigned int>(keyLength));
-        LOGD(" ## deserializeJni(): pickled=%s",(char const *)pickledPtr);
+        LOGD(" ## deserializeJni(): pickled=%.*s", static_cast<int> (pickledLength), (char const *)pickledPtr);
 
         size_t result = olm_unpickle_account(accountPtr,
                                              (void const *)keyPtr,

java/android/OlmLibSdk/olm-sdk/src/main/jni/olm_inbound_group_session.cpp

@@ -254,7 +254,7 @@ JNIEXPORT jbyteArray OLM_INBOUND_GROUP_SESSION_FUNC_DEF(decryptMessageJni)(JNIEn
         else
         {
             memcpy(tempEncryptedPtr, encryptedMsgPtr, encryptedMsgLength);
-            LOGD(" ## decryptMessageJni(): encryptedMsgLength=%lu encryptedMsg=%s",static_cast<long unsigned int>(encryptedMsgLength),encryptedMsgPtr);
+            LOGD(" ## decryptMessageJni(): encryptedMsgLength=%lu encryptedMsg=%.*s",static_cast<long unsigned int>(encryptedMsgLength), static_cast<int>(encryptedMsgLength), encryptedMsgPtr);
 
             // get max plaintext length
             size_t maxPlainTextLength = olm_group_decrypt_max_plaintext_length(sessionPtr,
@@ -464,7 +464,7 @@ JNIEXPORT jlong OLM_INBOUND_GROUP_SESSION_FUNC_DEF(deserializeJni)(JNIEnv *env,
         size_t pickledLength = (size_t)env->GetArrayLength(aSerializedDataBuffer);
         size_t keyLength = (size_t)env->GetArrayLength(aKeyBuffer);
         LOGD(" ## deserializeJni(): pickledLength=%lu keyLength=%lu",static_cast<long unsigned int>(pickledLength), static_cast<long unsigned int>(keyLength));
-        LOGD(" ## deserializeJni(): pickled=%s",(char const *)pickledPtr);
+        LOGD(" ## deserializeJni(): pickled=%.*s", static_cast<int>(pickledLength), (char const *)pickledPtr);
 
         size_t result = olm_unpickle_inbound_group_session(sessionPtr,
                                                            (void const *)keyPtr,

java/android/OlmLibSdk/olm-sdk/src/main/jni/olm_outbound_group_session.cpp

@@ -506,7 +506,7 @@ JNIEXPORT jlong OLM_OUTBOUND_GROUP_SESSION_FUNC_DEF(deserializeJni)(JNIEnv *env,
         size_t pickledLength = (size_t)env->GetArrayLength(aSerializedDataBuffer);
         size_t keyLength = (size_t)env->GetArrayLength(aKeyBuffer);
         LOGD(" ## deserializeJni(): pickledLength=%lu keyLength=%lu",static_cast<long unsigned int>(pickledLength), static_cast<long unsigned int>(keyLength));
-        LOGD(" ## deserializeJni(): pickled=%s",(char const *)pickledPtr);
+        LOGD(" ## deserializeJni(): pickled=%.*s", static_cast<int>(pickledLength), (char const *)pickledPtr);
 
         size_t result = olm_unpickle_outbound_group_session(sessionPtr,
                                                             (void const *)keyPtr,

java/android/OlmLibSdk/olm-sdk/src/main/jni/olm_session.cpp

@@ -143,7 +143,7 @@ JNIEXPORT void OLM_SESSION_FUNC_DEF(initOutboundSessionJni)(JNIEnv *env, jobject
             {
                 size_t theirIdentityKeyLength = (size_t)env->GetArrayLength(aTheirIdentityKeyBuffer);
                 size_t theirOneTimeKeyLength  = (size_t)env->GetArrayLength(aTheirOneTimeKeyBuffer);
-                LOGD("## initOutboundSessionJni(): identityKey=%s oneTimeKey=%s",theirIdentityKeyPtr,theirOneTimeKeyPtr);
+                LOGD("## initOutboundSessionJni(): identityKey=%.*s oneTimeKey=%.*s", static_cast<int>(theirIdentityKeyLength), theirIdentityKeyPtr, static_cast<int>(theirOneTimeKeyLength), theirOneTimeKeyPtr);
 
                 size_t sessionResult = olm_create_outbound_session(sessionPtr,
                                                             accountPtr,
@@ -175,6 +175,7 @@ JNIEXPORT void OLM_SESSION_FUNC_DEF(initOutboundSessionJni)(JNIEnv *env, jobject
 
             if (randomBuffPtr)
             {
+                memset(randomBuffPtr, 0, randomSize);
                 free(randomBuffPtr);
             }
         }
@@ -231,7 +232,7 @@ JNIEXPORT void OLM_SESSION_FUNC_DEF(initInboundSessionJni)(JNIEnv *env, jobject
         else
         {
             size_t messageLength = (size_t)env->GetArrayLength(aOneTimeKeyMsgBuffer);
-            LOGD("## initInboundSessionJni(): messageLength=%lu message=%s", static_cast<long unsigned int>(messageLength), messagePtr);
+            LOGD("## initInboundSessionJni(): messageLength=%lu message=%.*s", static_cast<long unsigned int>(messageLength), static_cast<int>(messageLength), messagePtr);
 
             sessionResult = olm_create_inbound_session(sessionPtr, accountPtr, (void*)messagePtr , messageLength);
 
@@ -309,7 +310,7 @@ JNIEXPORT void OLM_SESSION_FUNC_DEF(initInboundSessionFromIdKeyJni)(JNIEnv *env,
         size_t messageLength = (size_t)env->GetArrayLength(aOneTimeKeyMsgBuffer);
         size_t theirIdentityKeyLength = (size_t)env->GetArrayLength(aTheirIdentityKeyBuffer);
 
-        LOGD("## initInboundSessionFromIdKeyJni(): message=%s messageLength=%lu",messagePtr,static_cast<long unsigned int>(messageLength));
+        LOGD("## initInboundSessionFromIdKeyJni(): message=%.*s messageLength=%lu", static_cast<int>(messageLength), messagePtr, static_cast<long unsigned int>(messageLength));
 
         sessionResult = olm_create_inbound_session_from(sessionPtr, accountPtr, theirIdentityKeyPtr, theirIdentityKeyLength, (void*)messagePtr , messageLength);
         if (sessionResult == olm_error())
@@ -571,6 +572,7 @@ JNIEXPORT jbyteArray OLM_SESSION_FUNC_DEF(encryptMessageJni)(JNIEnv *env, jobjec
                 free(encryptedMsgPtr);
             }
 
+            memset(randomBuffPtr, 0, randomLength);
             free(randomBuffPtr);
         }
     }
@@ -659,7 +661,7 @@ JNIEXPORT jbyteArray OLM_SESSION_FUNC_DEF(decryptMessageJni)(JNIEnv *env, jobjec
         // create a dedicated temp buffer to be used in next Olm API calls
         tempEncryptedPtr = static_cast<char*>(malloc(encryptedMsgLength*sizeof(uint8_t)));
         memcpy(tempEncryptedPtr, encryptedMsgPtr, encryptedMsgLength);
-        LOGD("## decryptMessageJni(): MsgType=%lu encryptedMsgLength=%lu encryptedMsg=%s",static_cast<long unsigned int>(encryptedMsgType),static_cast<long unsigned int>(encryptedMsgLength),encryptedMsgPtr);
+        LOGD("## decryptMessageJni(): MsgType=%lu encryptedMsgLength=%lu encryptedMsg=%.*s",static_cast<long unsigned int>(encryptedMsgType),static_cast<long unsigned int>(encryptedMsgLength), static_cast<int>(encryptedMsgLength), encryptedMsgPtr);
 
         // get max plaintext length
         size_t maxPlainTextLength = olm_decrypt_max_plaintext_length(sessionPtr,
@@ -846,7 +848,7 @@ JNIEXPORT jbyteArray OLM_SESSION_FUNC_DEF(serializeJni)(JNIEnv *env, jobject thi
             }
             else
             {
-                LOGD(" ## serializeJni(): success - result=%lu pickled starts with %s", static_cast<long unsigned int>(result), static_cast<char*>(pickledPtr));
+                LOGD(" ## serializeJni(): success - result=%lu pickled=%.*s", static_cast<long unsigned int>(result), static_cast<int>(pickledLength), static_cast<char*>(pickledPtr));
 
                 returnValue = env->NewByteArray(pickledLength);
                 env->SetByteArrayRegion(returnValue, 0 , pickledLength, (jbyte*)pickledPtr);
@@ -916,7 +918,7 @@ JNIEXPORT jlong OLM_SESSION_FUNC_DEF(deserializeJni)(JNIEnv *env, jobject thiz,
         size_t pickledLength = (size_t)env->GetArrayLength(aSerializedDataBuffer);
         size_t keyLength = (size_t)env->GetArrayLength(aKeyBuffer);
         LOGD(" ## deserializeJni(): pickledLength=%lu keyLength=%lu",static_cast<long unsigned int>(pickledLength), static_cast<long unsigned int>(keyLength));
-        LOGD(" ## deserializeJni(): pickled=%s",(char const *)pickledPtr);
+        LOGD(" ## deserializeJni(): pickled=%.*s",static_cast<int>(pickledLength), (char const *)pickledPtr);
 
         size_t result = olm_unpickle_session(sessionPtr,
                                              (void const *)keyPtr,

java/android/OlmLibSdk/olm-sdk/src/main/jni/olm_utility.cpp

@@ -78,9 +78,6 @@ JNIEXPORT void OLM_UTILITY_FUNC_DEF(releaseUtilityJni)(JNIEnv *env, jobject thiz
 
 /**
  * Verify an ed25519 signature.
- * If the key was too small then the message will be "OLM.INVALID_BASE64".
- * If the signature was invalid then the message will be "OLM.BAD_MESSAGE_MAC".
- *
  * @param aSignature the base64-encoded message signature to be checked.
  * @param aKey the ed25519 key (fingerprint key)
  * @param aMessage the message which was signed
@@ -122,7 +119,7 @@ JNIEXPORT jstring OLM_UTILITY_FUNC_DEF(verifyEd25519SignatureJni)(JNIEnv *env, j
         size_t keyLength = (size_t)env->GetArrayLength(aKeyBuffer);
         size_t messageLength = (size_t)env->GetArrayLength(aMessageBuffer);
         LOGD(" ## verifyEd25519SignatureJni(): signatureLength=%lu keyLength=%lu messageLength=%lu",static_cast<long unsigned int>(signatureLength),static_cast<long unsigned int>(keyLength),static_cast<long unsigned int>(messageLength));
-        LOGD(" ## verifyEd25519SignatureJni(): key=%s",keyPtr);
+        LOGD(" ## verifyEd25519SignatureJni(): key=%.*s", static_cast<int>(keyLength), keyPtr);
 
         size_t result = olm_ed25519_verify(utilityPtr,
                                            (void const *)keyPtr,
@@ -194,7 +191,7 @@ JNIEXPORT jbyteArray OLM_UTILITY_FUNC_DEF(sha256Jni)(JNIEnv *env, jobject thiz,
         // get lengths
         size_t messageLength = (size_t)env->GetArrayLength(aMessageToHashBuffer);
         size_t hashLength = olm_sha256_length(utilityPtr);
-        void* hashValuePtr = malloc((hashLength+1)*sizeof(uint8_t));
+        void* hashValuePtr = malloc((hashLength)*sizeof(uint8_t));
 
         if (!hashValuePtr)
         {
@@ -213,12 +210,9 @@ JNIEXPORT jbyteArray OLM_UTILITY_FUNC_DEF(sha256Jni)(JNIEnv *env, jobject thiz,
             }
             else
             {
-                // update length
+                LOGD("## sha256Jni(): success - result=%lu hashValue=%.*s",static_cast<long unsigned int>(result), static_cast<int>(result), (char*)hashValuePtr);
-                (static_cast<char*>(hashValuePtr))[result] = static_cast<char>('\0');
+                sha256Ret = env->NewByteArray(result);
-                LOGD("## sha256Jni(): success - result=%lu hashValue=%s",static_cast<long unsigned int>(result), (char*)hashValuePtr);
+                env->SetByteArrayRegion(sha256Ret, 0 , result, (jbyte*)hashValuePtr);
-
-                 sha256Ret = env->NewByteArray(result);
-                 env->SetByteArrayRegion(sha256Ret, 0 , result, (jbyte*)hashValuePtr);
             }
 
             free(hashValuePtr);