6425: CommunicationsManager is validating account identifiers too rigorously

6516: CentralRepo accounts should be created with a normalized account identifier.
6507: CR validation does not include a length check

Core/src/org/sleuthkit/autopsy/centralrepository/datamodel/CentralRepoAccount.java

@@ -24,10 +24,9 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Objects;
+import org.apache.commons.lang.StringUtils;
 import org.sleuthkit.datamodel.Account;
-import org.sleuthkit.datamodel.CommunicationsUtils;
+import org.sleuthkit.datamodel.InvalidAccountIDException;
-import static org.sleuthkit.datamodel.CommunicationsUtils.normalizeEmailAddress;
-import org.sleuthkit.datamodel.TskCoreException;
 
 /**
  * This class abstracts an Account as stored in the CR database.
@@ -242,16 +241,9 @@ public final class CentralRepoAccount {
      * @throws CentralRepoException If there is an error in getting the
      * accounts.
      */
-    public static Collection<CentralRepoAccount> getAccountsWithIdentifier(String accountIdentifier) throws CentralRepoException {
+    public static Collection<CentralRepoAccount> getAccountsWithIdentifier(String accountIdentifier) throws InvalidAccountIDException, CentralRepoException {
-
-        String normalizedAccountIdentifier;
-
-        try {
-            normalizedAccountIdentifier = normalizeAccountIdentifier(accountIdentifier);
-        } catch (TskCoreException ex) {
-            throw new CentralRepoException("Failed to normalize account identifier.", ex);
-        }
 
+        String normalizedAccountIdentifier = normalizeAccountIdentifier(accountIdentifier);
         String queryClause = ACCOUNTS_QUERY_CLAUSE
                 + " WHERE LOWER(accounts.account_unique_identifier) = LOWER('" + normalizedAccountIdentifier + "')";
 
@@ -287,14 +279,24 @@ public final class CentralRepoAccount {
      * @param accountIdentifier Account identifier to be normalized.
      * @return normalized identifier
      *
-     * @throws TskCoreException
+     * @throws InvalidAccountIDException If the account identifier is not valid.
      */
-    private static String normalizeAccountIdentifier(String accountIdentifier) throws TskCoreException {
+    private static String normalizeAccountIdentifier(String accountIdentifier) throws InvalidAccountIDException {
-        String normalizedAccountIdentifier = accountIdentifier;
+        if (StringUtils.isEmpty(accountIdentifier)) {
-        if (CommunicationsUtils.isValidPhoneNumber(accountIdentifier)) {
+            throw new InvalidAccountIDException("Account id is null or empty.");
-            normalizedAccountIdentifier = CommunicationsUtils.normalizePhoneNum(accountIdentifier);
+        }
-        } else if (CommunicationsUtils.isValidEmailAddress(accountIdentifier)) {
+
-            normalizedAccountIdentifier = normalizeEmailAddress(accountIdentifier);
+        String normalizedAccountIdentifier;
+        try {
+            if (CorrelationAttributeNormalizer.isValidPhoneNumber(accountIdentifier)) {
+                normalizedAccountIdentifier = CorrelationAttributeNormalizer.normalizePhone(accountIdentifier);
+            } else if (CorrelationAttributeNormalizer.isValidEmailAddress(accountIdentifier)) {
+                normalizedAccountIdentifier = CorrelationAttributeNormalizer.normalizeEmail(accountIdentifier);
+            } else {
+                normalizedAccountIdentifier = accountIdentifier.toLowerCase().trim();
+            }
+        } catch (CorrelationAttributeNormalizationException ex) {
+            throw new InvalidAccountIDException("Failed to normalize the account idenitier.", ex);
         }
         return normalizedAccountIdentifier;
     }

Core/src/org/sleuthkit/autopsy/centralrepository/datamodel/CorrelationAttributeNormalizer.java

@@ -19,12 +19,14 @@
  */
 package org.sleuthkit.autopsy.centralrepository.datamodel;
 
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
+import org.apache.commons.lang.StringUtils;
 import org.apache.commons.validator.routines.DomainValidator;
 import org.apache.commons.validator.routines.EmailValidator;
-import org.sleuthkit.datamodel.CommunicationsUtils;
-import org.sleuthkit.datamodel.TskCoreException;
 
 /**
  * Provides functions for normalizing data by attribute type before insertion or
@@ -155,25 +157,43 @@ final public class CorrelationAttributeNormalizer {
 
     /**
      * Verify and normalize email address.
+     *
+     * @param emailAddress Address to normalize.
+     * @return Normalized email address.
+     * @throws CorrelationAttributeNormalizationExceptions If the input is not a
+     * valid email address.
+     *
      */
-    private static String normalizeEmail(String data) throws CorrelationAttributeNormalizationException {
+    static String normalizeEmail(String emailAddress) throws CorrelationAttributeNormalizationException {
-       try {
+        if (isValidEmailAddress(emailAddress)) {
-           return CommunicationsUtils.normalizeEmailAddress(data);
+            return emailAddress.toLowerCase().trim();
-       }
+        } else {
-       catch(TskCoreException ex) {
+            throw new CorrelationAttributeNormalizationException(String.format("Data was expected to be a valid email address: %s", emailAddress));
-           throw new CorrelationAttributeNormalizationException(String.format("Data was expected to be a valid email address: %s", data), ex);
         }
     }
 
     /**
      * Verify and normalize phone number.
+     *
+     * @param phoneNumber Phone number to normalize.
+     * @return Normalized phone number.
+     * @throws CorrelationAttributeNormalizationExceptions If the input is not a
+     * valid phone number.
+     *
      */
-    private static String normalizePhone(String data) throws CorrelationAttributeNormalizationException {
+    static String normalizePhone(String phoneNumber) throws CorrelationAttributeNormalizationException {
-        try {
+        if (isValidPhoneNumber(phoneNumber)) {
-             return CommunicationsUtils.normalizePhoneNum(data);
+            String normalizedNumber = phoneNumber.replaceAll("\\s+", ""); // remove spaces.	
+            normalizedNumber = normalizedNumber.replaceAll("[\\-()]", ""); // remove parens & dashes.
+
+            // ensure a min length
+            if (normalizedNumber.length() < MIN_PHONENUMBER_LEN) {
+                throw new CorrelationAttributeNormalizationException(String.format("Phone number string %s is too short ", phoneNumber));
             }
-        catch(TskCoreException ex) {
+            return normalizedNumber;
-            throw new CorrelationAttributeNormalizationException(String.format("Data was expected to be a valid phone number: %s", data));
+
+        } else {
+            throw new CorrelationAttributeNormalizationException(String.format("Data was expected to be a valid phone number: %s", phoneNumber));
         }
     }
 
@@ -318,6 +338,60 @@ final public class CorrelationAttributeNormalizer {
         }
     }
 
+    // These symbols are allowed in written form of phone numbers.
+    // A '+' is allowed only as a leading digit and hence not inlcuded here.
+    // While a dialed sequence may have additonal special characters, such as #, * or ',',
+    // CR attributes represent accounts and hence those chatracter are not allowed.
+    private static final Set<String> PHONENUMBER_CHARS = new HashSet<>(Arrays.asList(
+            "-", "(", ")"
+    ));
+
+    private static final int MIN_PHONENUMBER_LEN = 5;
+
+    /**
+     * Checks if the given string is a valid phone number.
+     *
+     * @param phoneNumber String to check.
+     *
+     * @return True if the given string is a valid phone number, false
+     * otherwise.
+     */
+    static boolean isValidPhoneNumber(String phoneNumber) {
+
+        // A phone number may have a leading '+', special telephony chars, or digits.
+        // Anything else implies an invalid phone number.
+        for (int i = 0; i < phoneNumber.length(); i++) {
+            if ((i == 0 && phoneNumber.charAt(i) == '+')
+                    || Character.isSpaceChar(phoneNumber.charAt(i))
+                    || Character.isDigit(phoneNumber.charAt(i))
+                    || PHONENUMBER_CHARS.contains(String.valueOf(phoneNumber.charAt(i)))) {
+                // continue
+            } else {
+                return false;
+            }
+        }
+
+        // ensure a min length
+        return phoneNumber.length() >= MIN_PHONENUMBER_LEN;
+    }
+
+    /**
+     * Checks if the given string is a valid email address.
+     *
+     * @param emailAddress String to check.
+     *
+     * @return True if the given string is a valid email address, false
+     * otherwise.
+     */
+    static boolean isValidEmailAddress(String emailAddress) {
+        if (!StringUtils.isEmpty(emailAddress)) {
+            EmailValidator validator = EmailValidator.getInstance(true, true);
+            return validator.isValid(emailAddress);
+        }
+
+        return false;
+    }
+
     /**
      * This is a utility class - no need for constructing or subclassing, etc...
      */

Core/src/org/sleuthkit/autopsy/centralrepository/datamodel/CorrelationAttributeUtil.java

@@ -184,7 +184,11 @@ public class CorrelationAttributeUtil {
                     makeCorrAttrsFromCommunicationArtifacts(correlationAttrs, sourceArtifact);
                 }
             }
-        } catch (CentralRepoException ex) {
+        } catch (CorrelationAttributeNormalizationException ex) {
+            logger.log(Level.SEVERE, String.format("Error normalizing correlation attribute (%s)", artifact), ex); // NON-NLS
+            return correlationAttrs;
+        } 
+        catch (CentralRepoException ex) {
             logger.log(Level.SEVERE, String.format("Error querying central repository (%s)", artifact), ex); // NON-NLS
             return correlationAttrs;
         } catch (TskCoreException ex) {
@@ -198,18 +202,19 @@ public class CorrelationAttributeUtil {
     }
 
     /**
-     * Makes a correlation attribute instance from a phone number attribute of an
+     * Makes a correlation attribute instance from a phone number attribute of
-     * artifact.
+     * an artifact.
      *
      * @param corrAttrInstances Correlation attributes will be added to this.
      * @param artifact An artifact with a phone number attribute.
      *
-     * @throws TskCoreException     If there is an error querying the case
+     * @throws TskCoreException If there is an error querying the case database.
-     *                              database.
      * @throws CentralRepoException If there is an error querying the central
      * repository.
+     * @throws CorrelationAttributeNormalizationException If there is an error
+     * in normalizing the attribute.
      */
-    private static void makeCorrAttrsFromCommunicationArtifacts(List<CorrelationAttributeInstance> corrAttrInstances, BlackboardArtifact artifact) throws TskCoreException, CentralRepoException {
+    private static void makeCorrAttrsFromCommunicationArtifacts(List<CorrelationAttributeInstance> corrAttrInstances, BlackboardArtifact artifact) throws TskCoreException, CentralRepoException, CorrelationAttributeNormalizationException {
         CorrelationAttributeInstance corrAttr = null;
 
         /*
@@ -228,8 +233,8 @@ public class CorrelationAttributeUtil {
          * Normalize the phone number.
          */
         if (value != null) {
-            if(CommunicationsUtils.isValidPhoneNumber(value)) {
+            if(CorrelationAttributeNormalizer.isValidPhoneNumber(value)) {
-                value = CommunicationsUtils.normalizePhoneNum(value);
+                value = CorrelationAttributeNormalizer.normalizePhone(value);
                 corrAttr = makeCorrAttr(artifact, CentralRepository.getInstance().getCorrelationTypeById(CorrelationAttributeInstance.PHONE_TYPE_ID), value);
                 if(corrAttr != null) {
                     corrAttrInstances.add(corrAttr);

Core/src/org/sleuthkit/autopsy/centralrepository/persona/Bundle.properties-MERGED

@@ -17,6 +17,8 @@ PersonaAccountDialog_get_types_exception_msg=Failed to access central repository
 PersonaAccountDialog_get_types_exception_Title=Central Repository failure
 PersonaAccountDialog_identifier_empty_msg=The identifier field cannot be empty.
 PersonaAccountDialog_identifier_empty_Title=Empty identifier
+PersonaAccountDialog_invalid_account_msg=Account identifier is not valid.
+PersonaAccountDialog_invalid_account_Title=Invalid account identifier
 PersonaAccountDialog_search_empty_msg=Account not found for given identifier and type.
 PersonaAccountDialog_search_empty_Title=Account not found
 PersonaAccountDialog_search_failure_msg=Central Repository account search failed.

Core/src/org/sleuthkit/autopsy/centralrepository/persona/PersonaAccountDialog.java

@@ -36,6 +36,7 @@ import org.sleuthkit.autopsy.centralrepository.datamodel.CentralRepoException;
 import org.sleuthkit.autopsy.centralrepository.datamodel.CentralRepository;
 import org.sleuthkit.autopsy.centralrepository.datamodel.Persona;
 import org.sleuthkit.autopsy.coreutils.Logger;
+import org.sleuthkit.datamodel.InvalidAccountIDException;
 
 /**
  * Configuration dialog for adding an account to a persona.
@@ -276,7 +277,10 @@ public class PersonaAccountDialog extends JDialog {
         "PersonaAccountDialog_search_failure_Title=Account add failure",
         "PersonaAccountDialog_search_failure_msg=Central Repository account search failed.",
         "PersonaAccountDialog_search_empty_Title=Account not found",
-        "PersonaAccountDialog_search_empty_msg=Account not found for given identifier and type.",})
+        "PersonaAccountDialog_search_empty_msg=Account not found for given identifier and type.",
+        "PersonaAccountDialog_invalid_account_Title=Invalid account identifier",
+        "PersonaAccountDialog_invalid_account_msg=Account identifier is not valid.",
+    })
     private void okBtnActionPerformed(java.awt.event.ActionEvent evt) {//GEN-FIRST:event_okBtnActionPerformed
         if (identifierTextField.getText().isEmpty()) {
             JOptionPane.showMessageDialog(this,
@@ -303,6 +307,14 @@ public class PersonaAccountDialog extends JDialog {
                     JOptionPane.ERROR_MESSAGE);
             return;
         }
+        catch (InvalidAccountIDException e) {
+            logger.log(Level.SEVERE, "Invalid account identifier", e);
+            JOptionPane.showMessageDialog(this,
+                    Bundle.PersonaAccountDialog_invalid_account_msg(),
+                    Bundle.PersonaAccountDialog_invalid_account_Title(),
+                    JOptionPane.ERROR_MESSAGE);
+            return;
+        }
         if (candidates.isEmpty()) {
             JOptionPane.showMessageDialog(this,
                     Bundle.PersonaAccountDialog_search_empty_msg(),

Core/src/org/sleuthkit/autopsy/communications/relationships/AccountSummary.java

@@ -33,6 +33,7 @@ import org.sleuthkit.datamodel.TskCoreException;
 import org.sleuthkit.datamodel.blackboardutils.attributes.MessageAttachments.FileAttachment;
 import org.sleuthkit.datamodel.blackboardutils.attributes.MessageAttachments;
 import org.sleuthkit.datamodel.CommunicationsUtils;
+import org.sleuthkit.datamodel.InvalidAccountIDException;
 import org.sleuthkit.datamodel.blackboardutils.attributes.BlackboardJsonAttrUtil;
 
 /**
@@ -113,7 +114,7 @@ class AccountSummary {
                                         isReference = true;
                                         break;
                                     }
-                                } catch (TskCoreException ex) {
+                                } catch (InvalidAccountIDException ex) {
                                     logger.log(Level.WARNING, String.format("Exception thrown "
                                             + "in trying to normalize attribute value: %s",
                                             attributeValue), ex); //NON-NLS

Core/src/org/sleuthkit/autopsy/datasourceprocessors/xry/XRYCallsFileParser.java

@@ -23,12 +23,14 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.logging.Level;
+import org.openide.util.Exceptions;
 import org.sleuthkit.autopsy.coreutils.Logger;
 import org.sleuthkit.datamodel.Account;
 import org.sleuthkit.datamodel.Blackboard.BlackboardException;
 import org.sleuthkit.datamodel.BlackboardArtifact;
 import org.sleuthkit.datamodel.BlackboardAttribute;
 import org.sleuthkit.datamodel.Content;
+import org.sleuthkit.datamodel.InvalidAccountIDException;
 import org.sleuthkit.datamodel.SleuthkitCase;
 import org.sleuthkit.datamodel.TskCoreException;
 import org.sleuthkit.datamodel.blackboardutils.CommunicationArtifactsHelper;
@@ -285,8 +287,12 @@ final class XRYCallsFileParser extends AbstractSingleEntityParser {
             // If both callerId and calleeList were non-null/non-empty, then 
             // it would have been a valid combination.
             if (callerId != null) {
+                try {
                 currentCase.getCommunicationsManager().createAccountFileInstance(
                         Account.Type.PHONE, callerId, PARSER_NAME, parent);
+                } catch (InvalidAccountIDException ex) {
+                   logger.log(Level.WARNING, String.format("Invalid account identifier %s", callerId), ex);
+                }
 
                 otherAttributes.add(new BlackboardAttribute(
                         BlackboardAttribute.ATTRIBUTE_TYPE.TSK_PHONE_NUMBER,
@@ -294,8 +300,13 @@ final class XRYCallsFileParser extends AbstractSingleEntityParser {
             }
 
             for (String phone : calleeList) {
+                try {
                 currentCase.getCommunicationsManager().createAccountFileInstance(
                         Account.Type.PHONE, phone, PARSER_NAME, parent);
+                } catch (InvalidAccountIDException ex) {
+                    logger.log(Level.WARNING, String.format("Invalid account identifier %s", phone), ex);
+                }
+                
 
                 otherAttributes.add(new BlackboardAttribute(
                         BlackboardAttribute.ATTRIBUTE_TYPE.TSK_PHONE_NUMBER,

Core/src/org/sleuthkit/autopsy/datasourceprocessors/xry/XRYMessagesFileParser.java

@@ -34,6 +34,7 @@ import org.sleuthkit.datamodel.Account;
 import org.sleuthkit.datamodel.Blackboard.BlackboardException;
 import org.sleuthkit.datamodel.BlackboardAttribute;
 import org.sleuthkit.datamodel.Content;
+import org.sleuthkit.datamodel.InvalidAccountIDException;
 import org.sleuthkit.datamodel.SleuthkitCase;
 import org.sleuthkit.datamodel.TskCoreException;
 import org.sleuthkit.datamodel.blackboardutils.CommunicationArtifactsHelper;
@@ -307,8 +308,13 @@ final class XRYMessagesFileParser implements XRYFileParser {
                         } else if(namespace == XryNamespace.TO || direction == CommunicationDirection.OUTGOING) {
                             recipientIdsList.add(pair.getValue());
                         } else {
+                            try {
                                 currentCase.getCommunicationsManager().createAccountFileInstance(
                                         Account.Type.PHONE, pair.getValue(), PARSER_NAME, parent);
+                            } catch (InvalidAccountIDException ex) {
+                                logger.log(Level.WARNING, String.format("Invalid account identifier %s", pair.getValue()), ex);
+                            }
+
                             otherAttributes.add(new BlackboardAttribute(
                                         BlackboardAttribute.ATTRIBUTE_TYPE.TSK_PHONE_NUMBER,
                                         PARSER_NAME, pair.getValue()));

Core/src/org/sleuthkit/autopsy/datasourceprocessors/xry/XRYUtils.java

@@ -27,6 +27,7 @@ import java.time.format.DateTimeFormatter;
 import java.time.temporal.TemporalAccessor;
 import java.time.temporal.TemporalQueries;
 import org.sleuthkit.datamodel.CommunicationsUtils;
+import org.sleuthkit.datamodel.InvalidAccountIDException;
 import org.sleuthkit.datamodel.TskCoreException;
 
 /**
@@ -46,7 +47,7 @@ final class XRYUtils {
         try {
             CommunicationsUtils.normalizePhoneNum(phoneNumber);
             return true;
-        } catch (TskCoreException ex) {
+        } catch (InvalidAccountIDException ex) {
             return false;
         }
     }
@@ -55,7 +56,7 @@ final class XRYUtils {
         try {
             CommunicationsUtils.normalizeEmailAddress(email);
             return true;
-        } catch (TskCoreException ex) {
+        } catch (InvalidAccountIDException ex) {
             return false;
         }
     }

InternalPythonModules/android/whatsapp.py

@@ -177,16 +177,16 @@ class WhatsAppAnalyzer(general.AndroidComponentAnalyzer):
                 home_phone = contacts_parser.get_home_phone()
                 mobile_phone = contacts_parser.get_mobile_phone()
                 email = contacts_parser.get_email()
-
+                other_attributes = contacts_parser.get_other_attributes()
                 # add contact if we have at least one valid phone/email
-                if phone or home_phone or mobile_phone or email:
+                if phone or home_phone or mobile_phone or email or other_attributes:
                     helper.addContact( 
                         name, 
                         phone,
                         home_phone,
                         mobile_phone,
                         email,
-                        contacts_parser.get_other_attributes()
+                        other_attributes
                     )
             contacts_parser.close()
         except SQLException as ex:
@@ -443,10 +443,14 @@ class WhatsAppContactsParser(TskContactsParser):
         return (value if general.isValidEmailAddress(value) else None)
         
     def get_other_attributes(self):
+        value = self.result_set.getString("jid")
+        if value:
             return [BlackboardAttribute(
                     BlackboardAttribute.ATTRIBUTE_TYPE.TSK_ID, 
                     self._PARENT_ANALYZER, 
-                    self.result_set.getString("jid"))]
+                    value)]
+        else:
+             return []
 
 class WhatsAppMessagesParser(TskMessagesParser):
     """