From cddba2738fda480909ba136020a68a21c724f748 Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Sat, 31 Aug 2013 09:28:49 -0700 Subject: [PATCH] Make encoding/decoding more explicit. --- .../crypto/protocol/PreKeyBundleMessage.java | 41 ++++++++----------- .../textsecure/push/IncomingPushMessage.java | 13 ++---- .../textsecure/push}/RawTransportDetails.java | 2 +- .../textsecure/storage/Record.java | 8 +++- .../securesms/gcm/GcmIntentService.java | 4 +- .../securesms/mms/IncomingMediaMessage.java | 4 +- .../securesms/service/SmsReceiver.java | 18 +++++--- .../securesms/sms/IncomingTextMessage.java | 4 +- .../securesms/transport/PushTransport.java | 2 +- .../securesms/transport/SmsTransport.java | 3 +- 10 files changed, 52 insertions(+), 47 deletions(-) rename {src/org/thoughtcrime/securesms/sms => library/src/org/whispersystems/textsecure/push}/RawTransportDetails.java (93%) diff --git a/library/src/org/whispersystems/textsecure/crypto/protocol/PreKeyBundleMessage.java b/library/src/org/whispersystems/textsecure/crypto/protocol/PreKeyBundleMessage.java index 48e17fdedf..bce131e624 100644 --- a/library/src/org/whispersystems/textsecure/crypto/protocol/PreKeyBundleMessage.java +++ b/library/src/org/whispersystems/textsecure/crypto/protocol/PreKeyBundleMessage.java @@ -16,16 +16,13 @@ */ package org.whispersystems.textsecure.crypto.protocol; -import org.whispersystems.textsecure.crypto.MessageCipher; import org.whispersystems.textsecure.crypto.IdentityKey; import org.whispersystems.textsecure.crypto.InvalidKeyException; import org.whispersystems.textsecure.crypto.InvalidVersionException; +import org.whispersystems.textsecure.crypto.MessageCipher; import org.whispersystems.textsecure.crypto.PublicKey; -import org.whispersystems.textsecure.util.Base64; import org.whispersystems.textsecure.util.Conversions; -import java.io.IOException; - /** * Class responsible for parsing and constructing PreKeyBundle messages. * @@ -53,27 +50,25 @@ public class PreKeyBundleMessage { private final PublicKey publicKey; private final byte[] bundledMessage; - public PreKeyBundleMessage(String message) throws InvalidKeyException, InvalidVersionException { - try { - this.messageBytes = Base64.decodeWithoutPadding(message); - this.messageVersion = Conversions.highBitsToInt(this.messageBytes[VERSION_OFFSET]); + public PreKeyBundleMessage(byte[] messageBytes) + throws InvalidKeyException, InvalidVersionException + { + this.messageBytes = messageBytes; + this.messageVersion = Conversions.highBitsToInt(this.messageBytes[VERSION_OFFSET]); - if (messageVersion > MessageCipher.SUPPORTED_VERSION) - throw new InvalidVersionException("Key exchange with version: " + messageVersion + - " but we only support: " + MessageCipher.SUPPORTED_VERSION); + if (messageVersion > MessageCipher.SUPPORTED_VERSION) + throw new InvalidVersionException("Key exchange with version: " + messageVersion + + " but we only support: " + MessageCipher.SUPPORTED_VERSION); - this.supportedVersion = Conversions.lowBitsToInt(messageBytes[VERSION_OFFSET]); - this.publicKey = new PublicKey(messageBytes, PUBLIC_KEY_OFFSET); - this.identityKey = new IdentityKey(messageBytes, IDENTITY_KEY_OFFSET); - this.preKeyId = Conversions.byteArrayToMedium(messageBytes, PREKEY_ID_OFFSET); - this.bundledMessage = new byte[messageBytes.length - IDENTITY_KEY_LENGTH]; + this.supportedVersion = Conversions.lowBitsToInt(messageBytes[VERSION_OFFSET]); + this.publicKey = new PublicKey(messageBytes, PUBLIC_KEY_OFFSET); + this.identityKey = new IdentityKey(messageBytes, IDENTITY_KEY_OFFSET); + this.preKeyId = Conversions.byteArrayToMedium(messageBytes, PREKEY_ID_OFFSET); + this.bundledMessage = new byte[messageBytes.length - IDENTITY_KEY_LENGTH]; - this.bundledMessage[VERSION_OFFSET] = this.messageBytes[VERSION_OFFSET]; - System.arraycopy(messageBytes, IDENTITY_KEY_OFFSET+IDENTITY_KEY_LENGTH, bundledMessage, VERSION_OFFSET+VERSION_LENGTH, bundledMessage.length-VERSION_LENGTH); - } catch (IOException e) { - throw new InvalidKeyException(e); - } + this.bundledMessage[VERSION_OFFSET] = this.messageBytes[VERSION_OFFSET]; + System.arraycopy(messageBytes, IDENTITY_KEY_OFFSET+IDENTITY_KEY_LENGTH, bundledMessage, VERSION_OFFSET+VERSION_LENGTH, bundledMessage.length-VERSION_LENGTH); } public PreKeyBundleMessage(IdentityKey identityKey, byte[] bundledMessage) { @@ -116,8 +111,8 @@ public class PreKeyBundleMessage { return publicKey; } - public String getBundledMessage() { - return Base64.encodeBytesWithoutPadding(bundledMessage); + public byte[] getBundledMessage() { + return bundledMessage; } public int getPreKeyId() { diff --git a/library/src/org/whispersystems/textsecure/push/IncomingPushMessage.java b/library/src/org/whispersystems/textsecure/push/IncomingPushMessage.java index cb2427cd65..57b2fd1427 100644 --- a/library/src/org/whispersystems/textsecure/push/IncomingPushMessage.java +++ b/library/src/org/whispersystems/textsecure/push/IncomingPushMessage.java @@ -50,6 +50,7 @@ public class IncomingPushMessage implements PushMessage, Parcelable { this.destinations = new LinkedList(); this.attachments = new LinkedList(); + this.type = in.readInt(); this.source = in.readString(); in.readStringList(destinations); this.message = new byte[in.readInt()]; @@ -70,15 +71,8 @@ public class IncomingPushMessage implements PushMessage, Parcelable { return attachments; } - public String getMessageText() { - if (type == TYPE_MESSAGE_CIPHERTEXT || - type == TYPE_MESSAGE_KEY_EXCHANGE || - type == TYPE_MESSAGE_PREKEY_BUNDLE) - { - return Base64.encodeBytesWithoutPadding(message); - } - - return new String(message); + public byte[] getBody() { + return message; } public List getDestinations() { @@ -96,6 +90,7 @@ public class IncomingPushMessage implements PushMessage, Parcelable { @Override public void writeToParcel(Parcel dest, int flags) { + dest.writeInt(type); dest.writeString(source); dest.writeStringList(destinations); dest.writeInt(message.length); diff --git a/src/org/thoughtcrime/securesms/sms/RawTransportDetails.java b/library/src/org/whispersystems/textsecure/push/RawTransportDetails.java similarity index 93% rename from src/org/thoughtcrime/securesms/sms/RawTransportDetails.java rename to library/src/org/whispersystems/textsecure/push/RawTransportDetails.java index 17b0fe372a..6384ad9a32 100644 --- a/src/org/thoughtcrime/securesms/sms/RawTransportDetails.java +++ b/library/src/org/whispersystems/textsecure/push/RawTransportDetails.java @@ -1,4 +1,4 @@ -package org.thoughtcrime.securesms.sms; +package org.whispersystems.textsecure.push; import org.whispersystems.textsecure.crypto.TransportDetails; diff --git a/library/src/org/whispersystems/textsecure/storage/Record.java b/library/src/org/whispersystems/textsecure/storage/Record.java index 95cc66d64f..00634a8d75 100644 --- a/library/src/org/whispersystems/textsecure/storage/Record.java +++ b/library/src/org/whispersystems/textsecure/storage/Record.java @@ -68,7 +68,13 @@ public abstract class Record { } private static File getAddressFile(Context context, String directory, String address) { - return new File(context.getFilesDir().getAbsolutePath() + File.separatorChar + directory, address); + File parent = new File(context.getFilesDir(), directory); + + if (!parent.exists()) { + parent.mkdirs(); + } + + return new File(parent, address); } protected byte[] readBlob(FileInputStream in) throws IOException { diff --git a/src/org/thoughtcrime/securesms/gcm/GcmIntentService.java b/src/org/thoughtcrime/securesms/gcm/GcmIntentService.java index 4e29af7b90..dd437e29eb 100644 --- a/src/org/thoughtcrime/securesms/gcm/GcmIntentService.java +++ b/src/org/thoughtcrime/securesms/gcm/GcmIntentService.java @@ -8,6 +8,7 @@ import com.google.android.gcm.GCMBaseIntentService; import org.thoughtcrime.securesms.service.RegistrationService; import org.thoughtcrime.securesms.service.SendReceiveService; import org.thoughtcrime.securesms.sms.IncomingTextMessage; +import org.thoughtcrime.securesms.sms.SmsTransportDetails; import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.whispersystems.textsecure.crypto.InvalidVersionException; import org.whispersystems.textsecure.push.IncomingEncryptedPushMessage; @@ -76,7 +77,8 @@ public class GcmIntentService extends GCMBaseIntentService { private void handleIncomingTextMessage(Context context, IncomingPushMessage message) { ArrayList messages = new ArrayList(); - messages.add(new IncomingTextMessage(message)); + String encodedBody = new String(new SmsTransportDetails().getEncodedMessage(message.getBody())); + messages.add(new IncomingTextMessage(message, encodedBody)); Intent receivedIntent = new Intent(context, SendReceiveService.class); receivedIntent.setAction(SendReceiveService.RECEIVE_SMS_ACTION); diff --git a/src/org/thoughtcrime/securesms/mms/IncomingMediaMessage.java b/src/org/thoughtcrime/securesms/mms/IncomingMediaMessage.java index 65636b2550..636b41bef6 100644 --- a/src/org/thoughtcrime/securesms/mms/IncomingMediaMessage.java +++ b/src/org/thoughtcrime/securesms/mms/IncomingMediaMessage.java @@ -46,9 +46,9 @@ public class IncomingMediaMessage { this.headers.setLongInteger(message.getTimestampMillis() / 1000, PduHeaders.DATE); - if (message.getMessageText() != null && message.getMessageText().length() > 0) { + if (message.getBody() != null && message.getBody().length > 0) { PduPart text = new PduPart(); - text.setData(message.getMessageText().getBytes()); + text.setData(message.getBody()); text.setContentType("text/plain".getBytes(CharacterSets.MIMENAME_ISO_8859_1)); body.addPart(text); } diff --git a/src/org/thoughtcrime/securesms/service/SmsReceiver.java b/src/org/thoughtcrime/securesms/service/SmsReceiver.java index 643b901c4f..418b160989 100644 --- a/src/org/thoughtcrime/securesms/service/SmsReceiver.java +++ b/src/org/thoughtcrime/securesms/service/SmsReceiver.java @@ -36,15 +36,16 @@ import org.thoughtcrime.securesms.sms.IncomingKeyExchangeMessage; import org.thoughtcrime.securesms.sms.IncomingPreKeyBundleMessage; import org.thoughtcrime.securesms.sms.IncomingTextMessage; import org.thoughtcrime.securesms.sms.MultipartSmsMessageHandler; +import org.thoughtcrime.securesms.sms.SmsTransportDetails; import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.whispersystems.textsecure.crypto.InvalidKeyException; import org.whispersystems.textsecure.crypto.InvalidVersionException; import org.whispersystems.textsecure.crypto.MasterSecret; import org.whispersystems.textsecure.crypto.protocol.PreKeyBundleMessage; -import org.whispersystems.textsecure.push.OutgoingPushMessage; import org.whispersystems.textsecure.push.PushMessage; import org.whispersystems.textsecure.storage.InvalidKeyIdException; +import java.io.IOException; import java.util.List; public class SmsReceiver { @@ -120,14 +121,16 @@ public class SmsReceiver { Log.w("SmsReceiver", "Processing prekey message..."); try { - Recipient recipient = new Recipient(null, message.getSender(), null, null); - KeyExchangeProcessor processor = new KeyExchangeProcessor(context, masterSecret, recipient); - PreKeyBundleMessage preKeyExchange = new PreKeyBundleMessage(message.getMessageBody()); + Recipient recipient = new Recipient(null, message.getSender(), null, null); + KeyExchangeProcessor processor = new KeyExchangeProcessor(context, masterSecret, recipient); + SmsTransportDetails transportDetails = new SmsTransportDetails(); + PreKeyBundleMessage preKeyExchange = new PreKeyBundleMessage(transportDetails.getDecodedMessage(message.getMessageBody().getBytes())); if (processor.isTrusted(preKeyExchange)) { processor.processKeyExchangeMessage(preKeyExchange); - IncomingEncryptedMessage bundledMessage = new IncomingEncryptedMessage(message, preKeyExchange.getBundledMessage()); + String bundledMessageBody = new String(transportDetails.getEncodedMessage(preKeyExchange.getBundledMessage())); + IncomingEncryptedMessage bundledMessage = new IncomingEncryptedMessage(message, bundledMessageBody); Pair messageAndThreadId = storeSecureMessage(masterSecret, bundledMessage); Intent intent = new Intent(KeyExchangeProcessor.SECURITY_UPDATE_EVENT); @@ -136,6 +139,8 @@ public class SmsReceiver { context.sendBroadcast(intent, KeyCachingService.KEY_PERMISSION); return messageAndThreadId; + } else { + /// XXX } } catch (InvalidKeyException e) { Log.w("SmsReceiver", e); @@ -146,6 +151,9 @@ public class SmsReceiver { } catch (InvalidKeyIdException e) { Log.w("SmsReceiver", e); message.setStale(true); + } catch (IOException e) { + Log.w("SmsReceive", e); + message.setCorrupted(true); } return storeStandardMessage(masterSecret, message); diff --git a/src/org/thoughtcrime/securesms/sms/IncomingTextMessage.java b/src/org/thoughtcrime/securesms/sms/IncomingTextMessage.java index fba40c865d..5e478e8016 100644 --- a/src/org/thoughtcrime/securesms/sms/IncomingTextMessage.java +++ b/src/org/thoughtcrime/securesms/sms/IncomingTextMessage.java @@ -40,8 +40,8 @@ public class IncomingTextMessage implements Parcelable { this.sentTimestampMillis = message.getTimestampMillis(); } - public IncomingTextMessage(IncomingPushMessage message) { - this.message = message.getMessageText(); + public IncomingTextMessage(IncomingPushMessage message, String encodedBody) { + this.message = encodedBody; this.sender = message.getSource(); this.protocol = 31337; this.serviceCenterAddress = "GCM"; diff --git a/src/org/thoughtcrime/securesms/transport/PushTransport.java b/src/org/thoughtcrime/securesms/transport/PushTransport.java index 368202e2e7..4f6623cf98 100644 --- a/src/org/thoughtcrime/securesms/transport/PushTransport.java +++ b/src/org/thoughtcrime/securesms/transport/PushTransport.java @@ -9,7 +9,7 @@ import org.thoughtcrime.securesms.crypto.KeyExchangeProcessor; import org.thoughtcrime.securesms.database.model.SmsMessageRecord; import org.thoughtcrime.securesms.mms.PartParser; import org.thoughtcrime.securesms.recipients.Recipient; -import org.thoughtcrime.securesms.sms.RawTransportDetails; +import org.whispersystems.textsecure.push.RawTransportDetails; import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.thoughtcrime.securesms.util.Util; import org.whispersystems.textsecure.crypto.IdentityKey; diff --git a/src/org/thoughtcrime/securesms/transport/SmsTransport.java b/src/org/thoughtcrime/securesms/transport/SmsTransport.java index 39e6c46f53..b4b709e677 100644 --- a/src/org/thoughtcrime/securesms/transport/SmsTransport.java +++ b/src/org/thoughtcrime/securesms/transport/SmsTransport.java @@ -4,10 +4,9 @@ import android.app.PendingIntent; import android.content.Context; import android.telephony.SmsManager; import android.util.Log; -import android.util.Pair; import org.thoughtcrime.securesms.crypto.IdentityKeyUtil; -import org.thoughtcrime.securesms.sms.OutgoingPrekeyBundleMessage;import org.thoughtcrime.securesms.sms.RawTransportDetails; +import org.thoughtcrime.securesms.sms.OutgoingPrekeyBundleMessage;import org.whispersystems.textsecure.push.RawTransportDetails; import org.whispersystems.textsecure.crypto.IdentityKeyPair; import org.whispersystems.textsecure.crypto.KeyUtil; import org.whispersystems.textsecure.crypto.MasterSecret;