From 2305a648fb9ce1c86f961f7f22d5960c137d6de4 Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Fri, 3 May 2013 16:09:06 -0700 Subject: [PATCH] Minor refactoring --- .../securesms/crypto/SessionCipher.java | 12 +- .../securesms/crypto/TransportDetails.java | 3 +- .../database/model/DisplayRecord.java | 1 - .../database/model/MessageRecord.java | 12 -- .../securesms/service/SmsReceiver.java | 4 +- ...r.java => MultipartSmsMessageHandler.java} | 36 +++--- ...java => MultipartSmsTransportMessage.java} | 23 ++-- ...ultipartSmsTransportMessageFragments.java} | 6 +- .../securesms/sms/SmsTransportDetails.java | 106 +++++++----------- .../securesms/transport/SmsTransport.java | 13 +-- .../util/EncryptedCharacterCalculator.java | 10 -- 11 files changed, 92 insertions(+), 134 deletions(-) rename src/org/thoughtcrime/securesms/sms/{MultipartMessageHandler.java => MultipartSmsMessageHandler.java} (69%) rename src/org/thoughtcrime/securesms/sms/{MultipartTransportMessage.java => MultipartSmsTransportMessage.java} (90%) rename src/org/thoughtcrime/securesms/sms/{MultipartTransportMessageFragments.java => MultipartSmsTransportMessageFragments.java} (82%) diff --git a/src/org/thoughtcrime/securesms/crypto/SessionCipher.java b/src/org/thoughtcrime/securesms/crypto/SessionCipher.java index 321f67f5b4..ca9058bc33 100644 --- a/src/org/thoughtcrime/securesms/crypto/SessionCipher.java +++ b/src/org/thoughtcrime/securesms/crypto/SessionCipher.java @@ -79,11 +79,13 @@ public class SessionCipher { public byte[] encryptMessage(byte[] messageText) { Log.w("SessionCipher", "Encrypting message..."); try { - SessionKey sessionKey = getSessionKey(Cipher.ENCRYPT_MODE, localRecord.getCurrentKeyPair().getId(), remoteRecord.getCurrentRemoteKey().getId()); - byte[] paddedMessage = transportDetails.getPaddedMessageBody(messageText); - byte[] cipherText = getCiphertext(paddedMessage, sessionKey.getCipherKey()); - byte[] message = buildMessageFromCiphertext(cipherText); - byte[] messageWithMac = MessageMac.buildMessageWithMac(message, sessionKey.getMacKey()); + int localId = localRecord.getCurrentKeyPair().getId(); + int remoteId = remoteRecord.getCurrentRemoteKey().getId(); + SessionKey sessionKey = getSessionKey(Cipher.ENCRYPT_MODE, localId, remoteId); + byte[]paddedMessage = transportDetails.getPaddedMessageBody(messageText); + byte[]cipherText = getCiphertext(paddedMessage, sessionKey.getCipherKey()); + byte[]message = buildMessageFromCiphertext(cipherText); + byte[]messageWithMac = MessageMac.buildMessageWithMac(message, sessionKey.getMacKey()); sessionRecord.setSessionKey(sessionKey); sessionRecord.incrementCounter(); diff --git a/src/org/thoughtcrime/securesms/crypto/TransportDetails.java b/src/org/thoughtcrime/securesms/crypto/TransportDetails.java index 5797a7de32..c5976d6d6a 100644 --- a/src/org/thoughtcrime/securesms/crypto/TransportDetails.java +++ b/src/org/thoughtcrime/securesms/crypto/TransportDetails.java @@ -20,8 +20,9 @@ import java.io.IOException; public interface TransportDetails { - public byte[] stripPaddedMessage(byte[] messageWithPadding); + public byte[] stripPaddedMessage(byte[] messageWithPadding); public byte[] getPaddedMessageBody(byte[] messageBody); + public byte[] encodeMessage(byte[] messageWithMac); public byte[] decodeMessage(byte[] encodedMessageBytes) throws IOException; } diff --git a/src/org/thoughtcrime/securesms/database/model/DisplayRecord.java b/src/org/thoughtcrime/securesms/database/model/DisplayRecord.java index a58f4cf6c3..ef60bec8d3 100644 --- a/src/org/thoughtcrime/securesms/database/model/DisplayRecord.java +++ b/src/org/thoughtcrime/securesms/database/model/DisplayRecord.java @@ -56,7 +56,6 @@ public abstract class DisplayRecord { public Body getBody() { return body; -// return body == null ? "" : body; } public abstract SpannableString getDisplayBody(); diff --git a/src/org/thoughtcrime/securesms/database/model/MessageRecord.java b/src/org/thoughtcrime/securesms/database/model/MessageRecord.java index 2e3d2e026a..2f40fe8524 100644 --- a/src/org/thoughtcrime/securesms/database/model/MessageRecord.java +++ b/src/org/thoughtcrime/securesms/database/model/MessageRecord.java @@ -118,16 +118,4 @@ public abstract class MessageRecord extends DisplayRecord { return spannable; } - -// public static class GroupData { -// public final int groupSize; -// public final int groupSentCount; -// public final int groupSendFailedCount; -// -// public GroupData(int groupSize, int groupSentCount, int groupSendFailedCount) { -// this.groupSize = groupSize; -// this.groupSentCount = groupSentCount; -// this.groupSendFailedCount = groupSendFailedCount; -// } -// } } diff --git a/src/org/thoughtcrime/securesms/service/SmsReceiver.java b/src/org/thoughtcrime/securesms/service/SmsReceiver.java index 6374a8f91c..1251bb6aab 100644 --- a/src/org/thoughtcrime/securesms/service/SmsReceiver.java +++ b/src/org/thoughtcrime/securesms/service/SmsReceiver.java @@ -38,13 +38,13 @@ import org.thoughtcrime.securesms.protocol.WirePrefix; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.sms.IncomingKeyExchangeMessage; import org.thoughtcrime.securesms.sms.IncomingTextMessage; -import org.thoughtcrime.securesms.sms.MultipartMessageHandler; +import org.thoughtcrime.securesms.sms.MultipartSmsMessageHandler; import java.util.List; public class SmsReceiver { - private MultipartMessageHandler multipartMessageHandler = new MultipartMessageHandler(); + private MultipartSmsMessageHandler multipartMessageHandler = new MultipartSmsMessageHandler(); private final Context context; diff --git a/src/org/thoughtcrime/securesms/sms/MultipartMessageHandler.java b/src/org/thoughtcrime/securesms/sms/MultipartSmsMessageHandler.java similarity index 69% rename from src/org/thoughtcrime/securesms/sms/MultipartMessageHandler.java rename to src/org/thoughtcrime/securesms/sms/MultipartSmsMessageHandler.java index 15244a5447..3aaa21d262 100644 --- a/src/org/thoughtcrime/securesms/sms/MultipartMessageHandler.java +++ b/src/org/thoughtcrime/securesms/sms/MultipartSmsMessageHandler.java @@ -24,25 +24,25 @@ import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; -public class MultipartMessageHandler { +public class MultipartSmsMessageHandler { - private final HashMap partialMessages = - new HashMap(); + private final HashMap partialMessages = + new HashMap(); private final HashMap idMap = new HashMap(); - private IncomingTextMessage processMultipartMessage(MultipartTransportMessage message) { - Log.w("MultipartMessageHandler", "Processing multipart message..."); - MultipartTransportMessageFragments container = partialMessages.get(message.getKey()); + private IncomingTextMessage processMultipartMessage(MultipartSmsTransportMessage message) { + Log.w("MultipartSmsMessageHandler", "Processing multipart message..."); + MultipartSmsTransportMessageFragments container = partialMessages.get(message.getKey()); if (container == null) { - container = new MultipartTransportMessageFragments(message.getMultipartCount()); + container = new MultipartSmsTransportMessageFragments(message.getMultipartCount()); partialMessages.put(message.getKey(), container); } container.add(message); - Log.w("MultipartMessageHandler", "Filled buffer at index: " + message.getMultipartIndex()); + Log.w("MultipartSmsMessageHandler", "Filled buffer at index: " + message.getMultipartIndex()); if (!container.isComplete()) return null; @@ -50,18 +50,18 @@ public class MultipartMessageHandler { partialMessages.remove(message.getKey()); String strippedMessage = Base64.encodeBytesWithoutPadding(container.getJoined()); - if (message.getWireType() == MultipartTransportMessage.WIRETYPE_KEY) { + if (message.getWireType() == MultipartSmsTransportMessage.WIRETYPE_KEY) { return new IncomingKeyExchangeMessage(message.getBaseMessage(), strippedMessage); } else { return new IncomingEncryptedMessage(message.getBaseMessage(), strippedMessage); } } - private IncomingTextMessage processSinglePartMessage(MultipartTransportMessage message) { - Log.w("MultipartMessageHandler", "Processing single part message..."); + private IncomingTextMessage processSinglePartMessage(MultipartSmsTransportMessage message) { + Log.w("MultipartSmsMessageHandler", "Processing single part message..."); String strippedMessage = Base64.encodeBytesWithoutPadding(message.getStrippedMessage()); - if (message.getWireType() == MultipartTransportMessage.WIRETYPE_KEY) { + if (message.getWireType() == MultipartSmsTransportMessage.WIRETYPE_KEY) { return new IncomingKeyExchangeMessage(message.getBaseMessage(), strippedMessage); } else { return new IncomingEncryptedMessage(message.getBaseMessage(), strippedMessage); @@ -70,13 +70,13 @@ public class MultipartMessageHandler { public IncomingTextMessage processPotentialMultipartMessage(IncomingTextMessage message) { try { - MultipartTransportMessage transportMessage = new MultipartTransportMessage(message); + MultipartSmsTransportMessage transportMessage = new MultipartSmsTransportMessage(message); if (transportMessage.isInvalid()) return message; else if (transportMessage.isSinglePart()) return processSinglePartMessage(transportMessage); else return processMultipartMessage(transportMessage); } catch (IOException e) { - Log.w("MultipartMessageHandler", e); + Log.w("MultipartSmsMessageHandler", e); return message; } } @@ -88,19 +88,17 @@ public class MultipartMessageHandler { currentId = idMap.get(recipient); idMap.remove(recipient); } else { - currentId = Integer.valueOf(0); + currentId = 0; } byte id = currentId.byteValue(); - idMap.put(recipient, Integer.valueOf((currentId.intValue() + 1) % 255)); + idMap.put(recipient, (currentId + 1) % 255); return id; } public ArrayList divideMessage(OutgoingTextMessage message) { - - byte identifier = getIdForRecipient(message.getRecipients().getPrimaryRecipient().getNumber()); - return MultipartTransportMessage.getEncoded(message, identifier); + return MultipartSmsTransportMessage.getEncoded(message, identifier); } } diff --git a/src/org/thoughtcrime/securesms/sms/MultipartTransportMessage.java b/src/org/thoughtcrime/securesms/sms/MultipartSmsTransportMessage.java similarity index 90% rename from src/org/thoughtcrime/securesms/sms/MultipartTransportMessage.java rename to src/org/thoughtcrime/securesms/sms/MultipartSmsTransportMessage.java index 3d32520d67..5245e1fd75 100644 --- a/src/org/thoughtcrime/securesms/sms/MultipartTransportMessage.java +++ b/src/org/thoughtcrime/securesms/sms/MultipartSmsTransportMessage.java @@ -7,16 +7,19 @@ import org.thoughtcrime.securesms.protocol.SecureMessageWirePrefix; import org.thoughtcrime.securesms.protocol.WirePrefix; import org.thoughtcrime.securesms.util.Base64; import org.thoughtcrime.securesms.util.Conversions; -import org.thoughtcrime.securesms.util.Hex; import java.io.IOException; import java.util.ArrayList; -public class MultipartTransportMessage { - private static final String TAG = MultipartTransportMessage.class.getName(); +public class MultipartSmsTransportMessage { + private static final String TAG = MultipartSmsTransportMessage.class.getName(); private static final int MULTIPART_SUPPORTED_AFTER_VERSION = 1; + public static final int SINGLE_MESSAGE_MULTIPART_OVERHEAD = 1; + public static final int MULTI_MESSAGE_MULTIPART_OVERHEAD = 3; + public static final int FIRST_MULTI_MESSAGE_MULTIPART_OVERHEAD = 2; + public static final int WIRETYPE_SECURE = 1; public static final int WIRETYPE_KEY = 2; @@ -24,17 +27,16 @@ public class MultipartTransportMessage { private static final int MULTIPART_OFFSET = 1; private static final int IDENTIFIER_OFFSET = 2; - private final int wireType; - private final byte[] decodedMessage; + private final int wireType; + private final byte[] decodedMessage; private final IncomingTextMessage message; - public MultipartTransportMessage(IncomingTextMessage message) throws IOException { + public MultipartSmsTransportMessage(IncomingTextMessage message) throws IOException { this.message = message; this.wireType = WirePrefix.isEncryptedMessage(message.getMessageBody()) ? WIRETYPE_SECURE : WIRETYPE_KEY; this.decodedMessage = Base64.decodeWithoutPadding(message.getMessageBody().substring(WirePrefix.PREFIX_SIZE)); Log.w(TAG, "Decoded message with version: " + getCurrentVersion()); - Log.w(TAG, "Decoded message: " + Hex.toString(decodedMessage)); } public int getWireType() { @@ -141,10 +143,11 @@ public class MultipartTransportMessage { return message; } - public static ArrayList getEncoded(OutgoingTextMessage message, byte identifier) { + public static ArrayList getEncoded(OutgoingTextMessage message, byte identifier) + { try { byte[] decoded = Base64.decodeWithoutPadding(message.getMessageBody()); - int count = SmsTransportDetails.getMessageCountForBytes(decoded.length); + int count = new SmsTransportDetails().getMessageCountForBytes(decoded.length); WirePrefix prefix; @@ -177,7 +180,7 @@ public class MultipartTransportMessage { } private static ArrayList getMultiEncoded(byte[] decoded, WirePrefix prefix, - int segmentCount, byte id) + int segmentCount, byte id) { ArrayList list = new ArrayList(segmentCount); byte versionByte = decoded[VERSION_OFFSET]; diff --git a/src/org/thoughtcrime/securesms/sms/MultipartTransportMessageFragments.java b/src/org/thoughtcrime/securesms/sms/MultipartSmsTransportMessageFragments.java similarity index 82% rename from src/org/thoughtcrime/securesms/sms/MultipartTransportMessageFragments.java rename to src/org/thoughtcrime/securesms/sms/MultipartSmsTransportMessageFragments.java index a2687f0f23..b4ef13f2f4 100644 --- a/src/org/thoughtcrime/securesms/sms/MultipartTransportMessageFragments.java +++ b/src/org/thoughtcrime/securesms/sms/MultipartSmsTransportMessageFragments.java @@ -1,14 +1,14 @@ package org.thoughtcrime.securesms.sms; -public class MultipartTransportMessageFragments { +public class MultipartSmsTransportMessageFragments { private final byte[][] fragments; - public MultipartTransportMessageFragments(int count) { + public MultipartSmsTransportMessageFragments(int count) { this.fragments = new byte[count][]; } - public void add(MultipartTransportMessage fragment) { + public void add(MultipartSmsTransportMessage fragment) { this.fragments[fragment.getMultipartIndex()] = fragment.getStrippedMessage(); } diff --git a/src/org/thoughtcrime/securesms/sms/SmsTransportDetails.java b/src/org/thoughtcrime/securesms/sms/SmsTransportDetails.java index 0ad45dae12..258eaf8e79 100644 --- a/src/org/thoughtcrime/securesms/sms/SmsTransportDetails.java +++ b/src/org/thoughtcrime/securesms/sms/SmsTransportDetails.java @@ -29,104 +29,84 @@ public class SmsTransportDetails implements TransportDetails { public static final int SMS_SIZE = 160; public static final int MULTIPART_SMS_SIZE = 153; - - private static final int SINGLE_MESSAGE_MULTIPART_OVERHEAD = 1; - private static final int MULTI_MESSAGE_MULTIPART_OVERHEAD = 3; - private static final int FIRST_MULTI_MESSAGE_MULTIPART_OVERHEAD = 2; - - public static final int BASE_MAX_BYTES = Base64.getEncodedBytesForTarget(SMS_SIZE - WirePrefix.PREFIX_SIZE); - public static final int SINGLE_MESSAGE_MAX_BYTES = BASE_MAX_BYTES - SINGLE_MESSAGE_MULTIPART_OVERHEAD; - public static final int MULTI_MESSAGE_MAX_BYTES = BASE_MAX_BYTES - MULTI_MESSAGE_MULTIPART_OVERHEAD; - public static final int FIRST_MULTI_MESSAGE_MAX_BYTES = BASE_MAX_BYTES - FIRST_MULTI_MESSAGE_MULTIPART_OVERHEAD; - + + public static final int BASE_MAX_BYTES = Base64.getEncodedBytesForTarget(SMS_SIZE - WirePrefix.PREFIX_SIZE); + public static final int SINGLE_MESSAGE_MAX_BYTES = BASE_MAX_BYTES - MultipartSmsTransportMessage.SINGLE_MESSAGE_MULTIPART_OVERHEAD; + public static final int MULTI_MESSAGE_MAX_BYTES = BASE_MAX_BYTES - MultipartSmsTransportMessage.MULTI_MESSAGE_MULTIPART_OVERHEAD; + public static final int FIRST_MULTI_MESSAGE_MAX_BYTES = BASE_MAX_BYTES - MultipartSmsTransportMessage.FIRST_MULTI_MESSAGE_MULTIPART_OVERHEAD; + public static final int ENCRYPTED_SINGLE_MESSAGE_BODY_MAX_SIZE = SINGLE_MESSAGE_MAX_BYTES - SessionCipher.ENCRYPTED_MESSAGE_OVERHEAD; - // private final int encryptedMessageOverhead; - // private final int encryptedSingleMessageBodyMaxSize; - - // public SmsTransportDetails(int encryptedMessageOverhead) { - // this.encryptedMessageOverhead = encryptedMessageOverhead; - // this.encryptedSingleMessageBodyMaxSize = SINGLE_MESSAGE_MAX_BYTES - encryptedMessageOverhead; - // } - + + @Override public byte[] encodeMessage(byte[] messageWithMac) { String encodedMessage = Base64.encodeBytesWithoutPadding(messageWithMac); Log.w("SmsTransportDetails", "Encoded Message Length: " + encodedMessage.length()); return encodedMessage.getBytes(); } - + + @Override public byte[] decodeMessage(byte[] encodedMessageBytes) throws IOException { String encodedMessage = new String(encodedMessageBytes); return Base64.decodeWithoutPadding(encodedMessage); } - + + @Override public byte[] stripPaddedMessage(byte[] messageWithPadding) { int paddingBeginsIndex = 0; - + for (int i=1;i 0) messageCount++; - + return messageCount; } - - - - private int getMaxBodySizeForCurrentRecordCount(int bodyLength) { - int messageRecordsForBody = getMessageCountForBytes(bodyLength + SessionCipher.ENCRYPTED_MESSAGE_OVERHEAD); - - if (messageRecordsForBody == 1) - return ENCRYPTED_SINGLE_MESSAGE_BODY_MAX_SIZE; - else - return SmsTransportDetails.FIRST_MULTI_MESSAGE_MAX_BYTES + - (SmsTransportDetails.MULTI_MESSAGE_MAX_BYTES * (messageRecordsForBody-1)) - - SessionCipher.ENCRYPTED_MESSAGE_OVERHEAD; - } - - - } diff --git a/src/org/thoughtcrime/securesms/transport/SmsTransport.java b/src/org/thoughtcrime/securesms/transport/SmsTransport.java index b43dea6c24..b76fb9793f 100644 --- a/src/org/thoughtcrime/securesms/transport/SmsTransport.java +++ b/src/org/thoughtcrime/securesms/transport/SmsTransport.java @@ -15,7 +15,7 @@ import org.thoughtcrime.securesms.database.model.SmsMessageRecord; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.service.SendReceiveService; import org.thoughtcrime.securesms.service.SmsListener; -import org.thoughtcrime.securesms.sms.MultipartMessageHandler; +import org.thoughtcrime.securesms.sms.MultipartSmsMessageHandler; import org.thoughtcrime.securesms.sms.OutgoingTextMessage; import org.thoughtcrime.securesms.sms.SmsTransportDetails; @@ -40,14 +40,11 @@ public class SmsTransport { } private void deliverSecureMessage(SmsMessageRecord message) throws UndeliverableMessageException { - MultipartMessageHandler multipartMessageHandler = new MultipartMessageHandler(); - OutgoingTextMessage transportMessage = OutgoingTextMessage.from(message); + String encryptedMessage = getAsymmetricEncrypt(masterSecret, message.getBody().getBody(), + message.getIndividualRecipient()); - if (message.isSecure()) { - String encryptedMessage = getAsymmetricEncrypt(masterSecret, message.getBody().getBody(), - message.getIndividualRecipient()); - transportMessage = transportMessage.withBody(encryptedMessage); - } + OutgoingTextMessage transportMessage = OutgoingTextMessage.from(message).withBody(encryptedMessage); + MultipartSmsMessageHandler multipartMessageHandler = new MultipartSmsMessageHandler(); ArrayList messages = multipartMessageHandler.divideMessage(transportMessage); ArrayList sentIntents = constructSentIntents(message.getId(), message.getType(), messages); diff --git a/src/org/thoughtcrime/securesms/util/EncryptedCharacterCalculator.java b/src/org/thoughtcrime/securesms/util/EncryptedCharacterCalculator.java index df80b789b5..00ce9b054f 100644 --- a/src/org/thoughtcrime/securesms/util/EncryptedCharacterCalculator.java +++ b/src/org/thoughtcrime/securesms/util/EncryptedCharacterCalculator.java @@ -31,24 +31,14 @@ public class EncryptedCharacterCalculator extends CharacterCalculator { private CharacterState calculateMultiRecordCharacters(int charactersSpent) { int charactersInFirstRecord = SmsTransportDetails.ENCRYPTED_SINGLE_MESSAGE_BODY_MAX_SIZE; int spillover = charactersSpent - charactersInFirstRecord; - Log.w("EncryptedCharacterCalculator", "Spillover: " + spillover); - // int maxMultiMessageSize = SessionCipher.getMaxBodySizePerMultiMessage(charactersSpent); - // Log.w("EncryptedCharacterCalculator", "Maxmultimessagesize: " + maxMultiMessageSize); - // int spilloverMessagesSpent = spillover / maxMultiMessageSize; int spilloverMessagesSpent = spillover / SmsTransportDetails.MULTI_MESSAGE_MAX_BYTES; - Log.w("EncryptedCharacterCalculator", "Spillover messaegs spent: " + spilloverMessagesSpent); - // if ((spillover % maxMultiMessageSize) > 0) if ((spillover % SmsTransportDetails.MULTI_MESSAGE_MAX_BYTES) > 0) spilloverMessagesSpent++; - Log.w("EncryptedCharacterCalculator", "Spillover messaegs spent: " + spilloverMessagesSpent); - - // int charactersRemaining = (maxMultiMessageSize * spilloverMessagesSpent) - spillover; int charactersRemaining = (SmsTransportDetails.MULTI_MESSAGE_MAX_BYTES * spilloverMessagesSpent) - spillover; Log.w("EncryptedCharacterCalculator", "charactersRemaining: " + charactersRemaining); - // return new CharacterState(spilloverMessagesSpent+1, charactersRemaining, maxMultiMessageSize); return new CharacterState(spilloverMessagesSpent+1, charactersRemaining, SmsTransportDetails.MULTI_MESSAGE_MAX_BYTES); }