[epiphany/wip/sync: 11/12] sync: Better error handling



commit 71170a5f81c1ee364cff92a848e24c3a63ba0330
Author: Gabriel Ivascu <ivascu gabriel59 gmail com>
Date:   Wed Apr 5 17:01:58 2017 +0300

    sync: Better error handling

 src/bookmarks/ephy-bookmark.c |    6 -
 src/prefs-dialog.c            |  124 ++++++++++----
 src/sync/ephy-sync-crypto.c   |  110 ++++++-------
 src/sync/ephy-sync-crypto.h   |    2 +-
 src/sync/ephy-sync-service.c  |  376 ++++++++++++++++++++++++++++-------------
 src/sync/ephy-sync-service.h  |    3 +-
 6 files changed, 396 insertions(+), 225 deletions(-)
---
diff --git a/src/bookmarks/ephy-bookmark.c b/src/bookmarks/ephy-bookmark.c
index c3b9a2e..5774740 100644
--- a/src/bookmarks/ephy-bookmark.c
+++ b/src/bookmarks/ephy-bookmark.c
@@ -426,18 +426,12 @@ ephy_bookmark_synchronizable_to_bso (EphySynchronizable  *synchronizable,
 
   serialized = json_gobject_to_data (G_OBJECT (bookmark), NULL);
   payload = ephy_sync_crypto_encrypt_record (serialized, bundle);
-  if (!payload) {
-    g_warning ("Failed to encrypt bookmark");
-    goto free_serialized;
-  }
-
   bso = ephy_sync_utils_build_json_string (FALSE,
                                            "id", bookmark->id,
                                            "payload", payload,
                                            NULL);
 
   g_free (payload);
-free_serialized:
   g_free (serialized);
 
   return bso;
diff --git a/src/prefs-dialog.c b/src/prefs-dialog.c
index a404961..b69e5c7 100644
--- a/src/prefs-dialog.c
+++ b/src/prefs-dialog.c
@@ -259,7 +259,7 @@ sync_tokens_store_finished_cb (EphySyncService *service,
     g_free (account);
   } else {
     /* Destroy the current session. */
-    ephy_sync_service_destroy_session (service, NULL);
+    ephy_sync_service_destroy_session (service);
 
     /* Unset the email and tokens. */
     ephy_sync_service_set_user_email (service, NULL);
@@ -306,69 +306,121 @@ sync_fxa_server_message_cb (WebKitUserContentManager *manager,
   JsonNode *node;
   JsonObject *json;
   JsonObject *detail;
+  JsonObject *data;
+  GError *error = NULL;
   char *json_string;
   const char *type;
   const char *command;
+  const char *email;
+  const char *uid;
+  const char *sessionToken;
+  const char *keyFetchToken;
+  const char *unwrapBKey;
+  gboolean verified;
+  gboolean is_internal_error = TRUE;
 
   json_string = ephy_embed_utils_get_js_result_as_string (result);
-  node = json_from_string (json_string, NULL);
+  if (!json_string) {
+    g_warning ("Failed to get JavaScript result as string");
+    goto out;
+  }
+  node = json_from_string (json_string, &error);
+  if (error) {
+    g_warning ("Response is not a valid JSON: %s", error->message);
+    g_error_free (error);
+    goto free_string;
+  }
+  if (!JSON_NODE_HOLDS_OBJECT (node)) {
+    g_warning ("JSON node does not hold a JSON object");
+    goto free_node;
+  }
   json = json_node_get_object (node);
+  if (!json_object_has_member (json, "type") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "type"))) {
+    g_warning ("JSON object has invalid 'type' member");
+    goto free_node;
+  }
   type = json_object_get_string_member (json, "type");
-
-  /* The only message type we can receive is FirefoxAccountsCommand. */
-  if (g_strcmp0 (type, "FirefoxAccountsCommand") != 0) {
+  /* The only message type expected is FirefoxAccountsCommand. */
+  if (g_strcmp0 (type, "FirefoxAccountsCommand")) {
     g_warning ("Unknown command type: %s", type);
-    goto out;
+    goto free_node;
+  }
+  if (!json_object_has_member (json, "detail") ||
+      !JSON_NODE_HOLDS_OBJECT (json_object_get_member (json, "detail"))) {
+    g_warning ("JSON object has invalid 'detail' member");
+    goto free_node;
   }
-
   detail = json_object_get_object_member (json, "detail");
-  command = json_object_get_string_member (detail, "command");
+  if (!json_object_has_member (detail, "command") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (detail, "command"))) {
+    g_warning ("JSON object has invalid 'command' member");
+    goto free_node;
+  }
 
-  if (g_strcmp0 (command, "loaded") == 0) {
+  command = json_object_get_string_member (detail, "command");
+  if (!g_strcmp0 (command, "loaded")) {
     LOG ("Firefox Accounts iframe loaded");
-  } else if (g_strcmp0 (command, "can_link_account") == 0) {
+    is_internal_error = FALSE;
+  } else if (!g_strcmp0 (command, "can_link_account")) {
     /* We need to confirm a relink. */
     sync_send_data_to_fxa_server (dialog, "message", "can_link_account", "{'ok': true}");
-  } else if (g_strcmp0 (command, "login") == 0) {
-    JsonObject *data = json_object_get_object_member (detail, "data");
-    const char *email = json_object_get_string_member (data, "email");
-    const char *uid = json_object_get_string_member (data, "uid");
-    const char *sessionToken = json_object_get_string_member (data, "sessionToken");
-    const char *keyFetchToken = json_object_get_string_member (data, "keyFetchToken");
-    const char *unwrapBKey = json_object_get_string_member (data, "unwrapBKey");
+    is_internal_error = FALSE;
+  } else if (!g_strcmp0 (command, "login")) {
+    if (!json_object_has_member (detail, "data") ||
+        !JSON_NODE_HOLDS_OBJECT (json_object_get_member (detail, "data"))) {
+      g_warning ("JSON object has invalid 'data' member");
+      goto free_node;
+    }
 
     gtk_widget_set_visible (dialog->sync_firefox_iframe_label, FALSE);
     sync_send_data_to_fxa_server (dialog, "message", "login", NULL);
 
-    /* Cannot retrieve the sync keys without keyFetchToken or unwrapBKey. */
-    if (keyFetchToken == NULL || unwrapBKey == NULL) {
-      g_warning ("Ignoring login with keyFetchToken or unwrapBKey missing!"
-                 "Cannot retrieve sync keys with one of them missing.");
-
-      ephy_sync_service_destroy_session (dialog->sync_service, sessionToken);
-      sync_sign_in_details_show (dialog, _("Something went wrong, please try again."));
-      webkit_web_view_load_uri (dialog->fxa_web_view, FXA_IFRAME_URL);
-
-      goto out;
+    data = json_object_get_object_member (detail, "data");
+    if (!json_object_has_member (data, "uid") ||
+        !JSON_NODE_HOLDS_VALUE (json_object_get_member (data, "uid")) ||
+        !json_object_has_member (data, "email") ||
+        !JSON_NODE_HOLDS_VALUE (json_object_get_member (data, "email")) ||
+        !json_object_has_member (data, "sessionToken") ||
+        !JSON_NODE_HOLDS_VALUE (json_object_get_member (data, "sessionToken")) ||
+        !json_object_has_member (data, "keyFetchToken") ||
+        !JSON_NODE_HOLDS_VALUE (json_object_get_member (data, "keyFetchToken")) ||
+        !json_object_has_member (data, "unwrapBKey") ||
+        !JSON_NODE_HOLDS_VALUE (json_object_get_member (data, "unwrapBKey")) ||
+        !json_object_has_member (data, "verified") ||
+        !JSON_NODE_HOLDS_VALUE (json_object_get_member (data, "verified"))) {
+      g_warning ("JSON object has missing or invalid members");
+      goto free_node;
     }
 
-    if (json_object_get_boolean_member (data, "verified") == FALSE)
+    email = json_object_get_string_member (data, "email");
+    uid = json_object_get_string_member (data, "uid");
+    sessionToken = json_object_get_string_member (data, "sessionToken");
+    keyFetchToken = json_object_get_string_member (data, "keyFetchToken");
+    unwrapBKey = json_object_get_string_member (data, "unwrapBKey");
+    verified = json_object_get_boolean_member (data, "verified");
+
+    if (!verified) {
       sync_sign_in_details_show (dialog, _("Please don’t leave this page until "
                                            "you have completed the verification."));
+    }
 
     ephy_sync_service_do_sign_in (dialog->sync_service, email, uid,
                                   sessionToken, keyFetchToken, unwrapBKey);
-  } else if (g_strcmp0 (command, "session_status") == 0) {
-    /* We are not signed in at this time, which we signal by returning an error. */
-    sync_send_data_to_fxa_server (dialog, "message", "error", NULL);
-  } else if (g_strcmp0 (command, "sign_out") == 0) {
-    /* We are not signed in at this time. We should never get a sign out message! */
-    sync_send_data_to_fxa_server (dialog, "message", "error", NULL);
+    is_internal_error = FALSE;
+  } else {
+    g_warning ("Unexepected command: %s", command);
   }
 
-out:
-  g_free (json_string);
+free_node:
   json_node_unref (node);
+free_string:
+  g_free (json_string);
+out:
+  if (is_internal_error) {
+    sync_sign_in_details_show (dialog, _("Something went wrong, please try again."));
+    webkit_web_view_load_uri (dialog->fxa_web_view, FXA_IFRAME_URL);
+  }
 }
 
 static void
diff --git a/src/sync/ephy-sync-crypto.c b/src/sync/ephy-sync-crypto.c
index 2c7fcde..172a607 100644
--- a/src/sync/ephy-sync-crypto.c
+++ b/src/sync/ephy-sync-crypto.c
@@ -772,60 +772,67 @@ ephy_sync_crypto_process_session_token (const char  *sessionToken,
   g_free (info);
 }
 
-void
-ephy_sync_crypto_compute_sync_keys (const char    *bundle,
+gboolean
+ephy_sync_crypto_compute_sync_keys (const char    *bundle_hex,
                                     const guint8  *respHMACkey,
                                     const guint8  *respXORkey,
                                     const guint8  *unwrapBKey,
                                     guint8       **kA,
                                     guint8       **kB)
 {
-  guint8 *bdl;
+  guint8 *bundle;
   guint8 *ciphertext;
   guint8 *respMAC;
   guint8 *respMAC2;
   guint8 *xored;
   guint8 *wrapKB;
   char *respMAC2_hex;
+  gboolean retval = TRUE;
 
-  g_return_if_fail (bundle);
-  g_return_if_fail (respHMACkey);
-  g_return_if_fail (respXORkey);
-  g_return_if_fail (unwrapBKey);
-  g_return_if_fail (kA);
-  g_return_if_fail (kB);
+  g_return_val_if_fail (bundle_hex, FALSE);
+  g_return_val_if_fail (respHMACkey, FALSE);
+  g_return_val_if_fail (respXORkey, FALSE);
+  g_return_val_if_fail (unwrapBKey, FALSE);
+  g_return_val_if_fail (kA, FALSE);
+  g_return_val_if_fail (kB, FALSE);
 
-  bdl = ephy_sync_crypto_decode_hex (bundle);
+  bundle = ephy_sync_crypto_decode_hex (bundle_hex);
   ciphertext = g_malloc (2 * EPHY_SYNC_TOKEN_LENGTH);
   respMAC = g_malloc (EPHY_SYNC_TOKEN_LENGTH);
-  wrapKB = g_malloc (EPHY_SYNC_TOKEN_LENGTH);
-  *kA = g_malloc (EPHY_SYNC_TOKEN_LENGTH);
 
   /* Compute the MAC and compare it to the expected value. */
-  memcpy (ciphertext, bdl, 2 * EPHY_SYNC_TOKEN_LENGTH);
-  memcpy (respMAC, bdl + 2 * EPHY_SYNC_TOKEN_LENGTH, EPHY_SYNC_TOKEN_LENGTH);
+  memcpy (ciphertext, bundle, 2 * EPHY_SYNC_TOKEN_LENGTH);
+  memcpy (respMAC, bundle + 2 * EPHY_SYNC_TOKEN_LENGTH, EPHY_SYNC_TOKEN_LENGTH);
   respMAC2_hex = g_compute_hmac_for_data (G_CHECKSUM_SHA256,
                                           respHMACkey, EPHY_SYNC_TOKEN_LENGTH,
                                           ciphertext, 2 * EPHY_SYNC_TOKEN_LENGTH);
   respMAC2 = ephy_sync_crypto_decode_hex (respMAC2_hex);
-  g_assert (ephy_sync_crypto_equals (respMAC, respMAC2, EPHY_SYNC_TOKEN_LENGTH));
+  if (!ephy_sync_crypto_equals (respMAC, respMAC2, EPHY_SYNC_TOKEN_LENGTH)) {
+    g_warning ("HMAC values differs from the one expected");
+    retval = FALSE;
+    goto out;
+  }
 
   /* XOR the extracted ciphertext with the respXORkey, then split in into the
    * separate kA and wrap(kB) values. */
   xored = ephy_sync_crypto_xor (ciphertext, respXORkey, 2 * EPHY_SYNC_TOKEN_LENGTH);
+  *kA = g_malloc (EPHY_SYNC_TOKEN_LENGTH);
   memcpy (*kA, xored, EPHY_SYNC_TOKEN_LENGTH);
+  wrapKB = g_malloc (EPHY_SYNC_TOKEN_LENGTH);
   memcpy (wrapKB, xored + EPHY_SYNC_TOKEN_LENGTH, EPHY_SYNC_TOKEN_LENGTH);
-
   /* Finally, XOR wrap(kB) with unwrapBKey to obtain kB. There is no MAC on wrap(kB). */
   *kB = ephy_sync_crypto_xor (unwrapBKey, wrapKB, EPHY_SYNC_TOKEN_LENGTH);
 
-  g_free (bdl);
-  g_free (ciphertext);
-  g_free (respMAC);
-  g_free (respMAC2);
-  g_free (xored);
   g_free (wrapKB);
+  g_free (xored);
+out:
+  g_free (respMAC2);
   g_free (respMAC2_hex);
+  g_free (respMAC);
+  g_free (ciphertext);
+  g_free (bundle);
+
+  return retval;
 }
 
 SyncCryptoKeyBundle *
@@ -926,10 +933,6 @@ ephy_sync_crypto_decrypt_record (const char          *payload,
   /* Get the encryption key and the HMAC key. */
   aes_key = ephy_sync_crypto_decode_hex (bundle->aes_key_hex);
   hmac_key = ephy_sync_crypto_decode_hex (bundle->hmac_key_hex);
-  if (!aes_key || !hmac_key) {
-    g_warning ("The key bundle does not hold valid keys");
-    goto free_node;
-  }
 
   /* Under no circumstances should a client try to decrypt a record
    * if the HMAC verification fails. */
@@ -958,7 +961,7 @@ char *
 ephy_sync_crypto_encrypt_record (const char          *cleartext,
                                  SyncCryptoKeyBundle *bundle)
 {
-  char *payload = NULL;
+  char *payload;
   char *iv_b64;
   char *ciphertext_b64;
   char *hmac;
@@ -974,10 +977,6 @@ ephy_sync_crypto_encrypt_record (const char          *cleartext,
   /* Get the encryption key and the HMAC key. */
   aes_key = ephy_sync_crypto_decode_hex (bundle->aes_key_hex);
   hmac_key = ephy_sync_crypto_decode_hex (bundle->hmac_key_hex);
-  if (!aes_key || !hmac_key) {
-    g_warning ("The key bundle does not hold valid keys");
-    goto free_keys;
-  }
 
   /* Generate a random 16 bytes initialization vector. */
   iv = g_malloc (IV_LEN);
@@ -1003,7 +1002,6 @@ ephy_sync_crypto_encrypt_record (const char          *cleartext,
   g_free (ciphertext_b64);
   g_free (ciphertext);
   g_free (iv);
-free_keys:
   g_free (aes_key);
   g_free (hmac_key);
 
@@ -1136,7 +1134,7 @@ ephy_sync_crypto_generate_rsa_key_pair (void)
 {
   struct rsa_public_key public;
   struct rsa_private_key private;
-  int retval;
+  int success;
 
   rsa_public_key_init (&public);
   rsa_private_key_init (&private);
@@ -1145,15 +1143,11 @@ ephy_sync_crypto_generate_rsa_key_pair (void)
   mpz_set_ui (public.e, 65537);
 
   /* Key sizes below 2048 are considered breakable and should not be used. */
-  retval = rsa_generate_keypair (&public, &private,
-                                 NULL, ephy_sync_crypto_random_bytes_gen,
-                                 NULL, NULL, 2048, 0);
-  if (retval == 0) {
-    g_warning ("Failed to generate RSA key pair");
-    rsa_public_key_clear (&public);
-    rsa_private_key_clear (&private);
-    return NULL;
-  }
+  success = rsa_generate_keypair (&public, &private,
+                                  NULL, ephy_sync_crypto_random_bytes_gen,
+                                  NULL, NULL, 2048, 0);
+  /* Given correct parameters, this never fails. */
+  g_assert (success);
 
   return ephy_sync_crypto_rsa_key_pair_new (public, private);
 }
@@ -1170,14 +1164,15 @@ ephy_sync_crypto_create_assertion (const char           *certificate,
   char *body_b64;
   char *header_b64;
   char *to_sign;
-  char *sig_b64 = NULL;
-  char *assertion = NULL;
+  char *sig_b64;
+  char *assertion;
   char *digest_hex;
   guint8 *digest;
-  guint8 *sig = NULL;
+  guint8 *sig;
   guint64 expires_at;
   gsize expected_size;
   gsize count;
+  int success;
 
   g_return_val_if_fail (certificate, NULL);
   g_return_val_if_fail (audience, NULL);
@@ -1196,27 +1191,22 @@ ephy_sync_crypto_create_assertion (const char           *certificate,
 
   /* Use the provided key pair to RSA sign the message. */
   mpz_init (signature);
-  if (rsa_sha256_sign_digest_tr (&keypair->public, &keypair->private,
-                                 NULL, ephy_sync_crypto_random_bytes_gen,
-                                 digest, signature) == 0) {
-    g_warning ("Failed to sign the message. Giving up.");
-    goto out;
-  }
+  success = rsa_sha256_sign_digest_tr (&keypair->public, &keypair->private,
+                                       NULL, ephy_sync_crypto_random_bytes_gen,
+                                       digest, signature);
+  /* Given correct parameters, this never fails. */
+  g_assert (success);
 
   expected_size = (mpz_sizeinbase (signature, 2) + 7) / 8;
   sig = g_malloc (expected_size);
   mpz_export (sig, &count, 1, sizeof (guint8), 0, 0, signature);
-
-  if (count != expected_size) {
-    g_warning ("Expected %lu bytes, got %lu. Giving up.", count, expected_size);
-    goto out;
-  }
+  /* Given correct parameters, this never fails. */
+  g_assert (count == expected_size);
 
   /* Finally, join certificate, header, body and signed message to create the assertion. */
   sig_b64 = ephy_sync_crypto_base64_urlsafe_encode (sig, count, TRUE);
   assertion = g_strdup_printf ("%s~%s.%s.%s", certificate, header_b64, body_b64, sig_b64);
 
-out:
   g_free (body);
   g_free (body_b64);
   g_free (header_b64);
@@ -1316,14 +1306,12 @@ guint8 *
 ephy_sync_crypto_decode_hex (const char *hex)
 {
   guint8 *retval;
-  gsize hex_len = strlen (hex);
 
   g_return_val_if_fail (hex, NULL);
-  g_return_val_if_fail (hex_len % 2 == 0, NULL);
 
-  retval = g_malloc (hex_len / 2);
-  for (gsize i = 0, j = 0; i < hex_len; i += 2, j++)
-    sscanf(hex + i, "%2hhx", retval + j);
+  retval = g_malloc (strlen (hex) / 2);
+  for (gsize i = 0, j = 0; i < strlen (hex); i += 2, j++)
+    sscanf (hex + i, "%2hhx", retval + j);
 
   return retval;
 }
diff --git a/src/sync/ephy-sync-crypto.h b/src/sync/ephy-sync-crypto.h
index bcdb179..420aebf 100644
--- a/src/sync/ephy-sync-crypto.h
+++ b/src/sync/ephy-sync-crypto.h
@@ -91,7 +91,7 @@ void                    ephy_sync_crypto_process_session_token    (const char
                                                                    guint8                **tokenID,
                                                                    guint8                **reqHMACkey,
                                                                    guint8                **requestKey);
-void                    ephy_sync_crypto_compute_sync_keys        (const char             *bundle,
+gboolean                ephy_sync_crypto_compute_sync_keys        (const char             *bundle_hex,
                                                                    const guint8           *respHMACkey,
                                                                    const guint8           *respXORkey,
                                                                    const guint8           *unwrapBKey,
diff --git a/src/sync/ephy-sync-service.c b/src/sync/ephy-sync-service.c
index 11bb82d..2bd0da1 100644
--- a/src/sync/ephy-sync-service.c
+++ b/src/sync/ephy-sync-service.c
@@ -52,7 +52,6 @@ struct _EphySyncService {
 
   char        *user_email;
   double       sync_time;
-  gint64       auth_at;
 
   gboolean     locked;
   char        *storage_endpoint;
@@ -381,11 +380,12 @@ ephy_sync_service_certificate_is_valid (EphySyncService *self,
   JsonParser *parser;
   JsonObject *json;
   JsonObject *principal;
+  GError *error = NULL;
   SoupURI *uri;
   char **pieces;
   char *header;
   char *payload;
-  char *uid_email = NULL;
+  char *expected;
   const char *alg;
   const char *email;
   gsize len;
@@ -394,46 +394,73 @@ ephy_sync_service_certificate_is_valid (EphySyncService *self,
   g_assert (EPHY_IS_SYNC_SERVICE (self));
   g_assert (certificate);
 
-  /* Check if the certificate is something that we were expecting, i.e.
-   * if the algorithm and email fields match the expected values. */
-
   uri = soup_uri_new (MOZILLA_FXA_SERVER_URL);
   pieces = g_strsplit (certificate, ".", 0);
   header = (char *)ephy_sync_crypto_base64_urlsafe_decode (pieces[0], &len, TRUE);
   payload = (char *)ephy_sync_crypto_base64_urlsafe_decode (pieces[1], &len, TRUE);
-
   parser = json_parser_new ();
-  json_parser_load_from_data (parser, header, -1, NULL);
+
+  json_parser_load_from_data (parser, header, -1, &error);
+  if (error) {
+    g_warning ("Header is not a valid JSON: %s", error->message);
+    g_error_free (error);
+    goto out;
+  }
+  if (!JSON_NODE_HOLDS_OBJECT (json_parser_get_root (parser))) {
+    g_warning ("JSON node does not hold a JSON object");
+    goto out;
+  }
   json = json_node_get_object (json_parser_get_root (parser));
+  if (!json_object_has_member (json, "alg") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "alg"))) {
+    g_warning ("JSON object has missing or invalid 'alg' member");
+    goto out;
+  }
   alg = json_object_get_string_member (json, "alg");
-
   if (g_strcmp0 (alg, "RS256")) {
-    g_warning ("Expected algorithm RS256, found %s. Giving up.", alg);
+    g_warning ("Expected algorithm RS256, found %s", alg);
+    goto out;
+  }
+  json_parser_load_from_data (parser, payload, -1, &error);
+  if (error) {
+    g_warning ("Payload is not a valid JSON: %s", error->message);
+    g_error_free (error);
+    goto out;
+  }
+  if (!JSON_NODE_HOLDS_OBJECT (json_parser_get_root (parser))) {
+    g_warning ("JSON node does not hold a JSON object");
     goto out;
   }
-
-  json_parser_load_from_data (parser, payload, -1, NULL);
   json = json_node_get_object (json_parser_get_root (parser));
+  if (!json_object_has_member (json, "principal") ||
+      !JSON_NODE_HOLDS_OBJECT (json_object_get_member (json, "principal"))) {
+    g_warning ("JSON object has missing or invalid 'principal' member");
+    goto out;
+  }
   principal = json_object_get_object_member (json, "principal");
-  email = json_object_get_string_member (principal, "email");
-  uid_email = g_strdup_printf ("%s@%s",
-                               ephy_sync_service_get_token (self, TOKEN_UID),
-                               soup_uri_get_host (uri));
-
-  if (g_strcmp0 (uid_email, email)) {
-    g_warning ("Expected email %s, found %s. Giving up.", uid_email, email);
+  if (!json_object_has_member (principal, "email") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (principal, "email"))) {
+    g_warning ("JSON object has misisng or invalid 'email' member");
     goto out;
   }
+  email = json_object_get_string_member (principal, "email");
+  expected = g_strdup_printf ("%s@%s",
+                              ephy_sync_service_get_token (self, TOKEN_UID),
+                              soup_uri_get_host (uri));
+  if (g_strcmp0 (email, expected)) {
+    g_warning ("Expected email %s, found %s", expected, email);
+    goto free_expected;
+  }
 
-  self->auth_at = json_object_get_int_member (json, "fxa-lastAuthAt");
   retval = TRUE;
 
+free_expected:
+  g_free (expected);
 out:
-  g_free (header);
+  g_object_unref (parser);
   g_free (payload);
-  g_free (uid_email);
+  g_free (header);
   g_strfreev (pieces);
-  g_object_unref (parser);
   soup_uri_free (uri);
 
   return retval;
@@ -447,28 +474,51 @@ obtain_storage_credentials_cb (SoupSession *session,
   EphySyncService *self;
   JsonNode *node;
   JsonObject *json;
-
-  self = ephy_shell_get_sync_service (ephy_shell_get_default ());
+  GError *error = NULL;
+  gboolean is_internal_error = TRUE;
 
   if (msg->status_code != 200) {
-    g_warning ("Failed to talk to the Token Server, status code %u. "
-               "See https://docs.services.mozilla.com/token/apis.html#error-responses";,
-               msg->status_code);
-    self->locked = FALSE;
-    return;
+    g_warning ("Failed to get storage credentials from Token Server. Status code: %u, response: %s",
+               msg->status_code, msg->response_body->data);
+    goto out;
   }
 
-  node = json_from_string (msg->response_body->data, NULL);
+  node = json_from_string (msg->response_body->data, &error);
+  if (error) {
+    g_warning ("Response is not a valid JSON: %s", error->message);
+    g_error_free (error);
+    goto out;
+  }
+  if (!JSON_NODE_HOLDS_OBJECT (node)) {
+    g_warning ("JSON node does not hold a JSON object");
+    goto free_node;
+  }
   json = json_node_get_object (node);
+  if (!json_object_has_member (json, "api_endpoint") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "api_endpoint")) ||
+      !json_object_has_member (json, "id") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "id")) ||
+      !json_object_has_member (json, "key") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "key")) ||
+      !json_object_has_member (json, "duration") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "duration"))) {
+    g_warning ("JSON object has missing or invalid members");
+    goto free_node;
+  }
+
+  self = ephy_shell_get_sync_service (ephy_shell_get_default ());
   self->storage_endpoint = g_strdup (json_object_get_string_member (json, "api_endpoint"));
   self->storage_credentials_id = g_strdup (json_object_get_string_member (json, "id"));
   self->storage_credentials_key = g_strdup (json_object_get_string_member (json, "key"));
-  self->storage_credentials_expiry_time = json_object_get_int_member (json, "duration") +
-                                             ephy_sync_utils_current_time_seconds ();
-  self->locked = FALSE;
-  ephy_sync_service_send_next_storage_request (self);
+  self->storage_credentials_expiry_time = json_object_get_int_member (json, "duration") + 
ephy_sync_utils_current_time_seconds ();
+  is_internal_error = FALSE;
 
+free_node:
   json_node_unref (node);
+out:
+  self->locked = FALSE;
+  if (!is_internal_error)
+    ephy_sync_service_send_next_storage_request (self);
 }
 
 static void
@@ -489,8 +539,6 @@ ephy_sync_service_obtain_storage_credentials (EphySyncService *self)
   audience = ephy_sync_utils_make_audience (MOZILLA_TOKEN_SERVER_URL);
   assertion = ephy_sync_crypto_create_assertion (self->certificate, audience,
                                                  ASSERTION_DURATION, self->keypair);
-  g_assert (assertion);
-
   kB = ephy_sync_crypto_decode_hex (ephy_sync_service_get_token (self, TOKEN_KB));
   hashed_kB = g_compute_checksum_for_data (G_CHECKSUM_SHA256, kB, EPHY_SYNC_TOKEN_LENGTH);
   client_state = g_strndup (hashed_kB, EPHY_SYNC_TOKEN_LENGTH);
@@ -519,52 +567,78 @@ obtain_signed_certificate_cb (SoupSession *session,
   EphySyncService *self;
   JsonNode *node;
   JsonObject *json;
+  GError *error = NULL;
   const char *certificate;
+  const char *suggestion = NULL;
+  char *message = NULL;
+  gboolean is_internal_error = TRUE;
 
   self = ephy_shell_get_sync_service (ephy_shell_get_default ());
 
-  node = json_from_string (msg->response_body->data, NULL);
+  node = json_from_string (msg->response_body->data, &error);
+  if (error) {
+    g_warning ("Response is not a valid JSON: %s", error->message);
+    g_error_free (error);
+    goto out;
+  }
+  if (!JSON_NODE_HOLDS_OBJECT (node)) {
+    g_warning ("JSON node does not hold a JSON object");
+    goto free_node;
+  }
   json = json_node_get_object (node);
 
-  /* Since a new Firefox Account password implies new tokens, this will fail
-   * with an error code 110 (Invalid authentication token in request signature)
-   * if the user has changed his password since the last time he signed in.
-   * When this happens, notify the user to sign in with the new password. */
-  if (msg->status_code == 401 && json_object_get_int_member (json, "errno") == 110) {
-    char *error = g_strdup_printf (_("The password of your Firefox account %s "
-                                     "seems to have been changed."),
-                                   ephy_sync_service_get_user_email (self));
-    const char *suggestion = _("Please visit Preferences and sign in with "
-                               "the new password to continue the sync process.");
+  if (msg->status_code != 200) {
+    if (!json_object_has_member (json, "errno") ||
+        !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "errno"))) {
+      g_warning ("JSON object has missing or invalid 'errno' member");
+      goto free_node;
+    }
 
-    ephy_notification_show (ephy_notification_new (error, suggestion));
+    /* Since a new Firefox Account password implies new tokens, this will fail
+     * with an error code 110 (Invalid authentication token in request signature)
+     * if the user has changed his password since the last time he signed in.
+     * When this happens, notify the user to sign in with the new password. */
+    if (msg->status_code == 401 && json_object_get_int_member (json, "errno") == 110) {
+      message = g_strdup_printf (_("The password of your Firefox account %s seems to have been changed."),
+                                 ephy_sync_service_get_user_email (self));
+      suggestion = _("Please visit Preferences and sign in with the new password to continue syncing.");
+      goto free_node;
+    }
 
-    g_free (error);
-    self->locked = FALSE;
-    goto out;
+    g_warning ("Failed to sign certificate. Status code: %u, response: %s",
+               msg->status_code, msg->response_body->data);
+    goto free_node;
   }
 
-  if (msg->status_code != 200) {
-    g_warning ("FxA server errno: %ld, errmsg: %s",
-               json_object_get_int_member (json, "errno"),
-               json_object_get_string_member (json, "message"));
-    self->locked = FALSE;
-    goto out;
+  if (!json_object_has_member (json, "cert") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "cert"))) {
+    g_warning ("JSON object has missing or invalid 'cert' member");
+    goto free_node;
   }
 
   certificate = json_object_get_string_member (json, "cert");
-
   if (!ephy_sync_service_certificate_is_valid (self, certificate)) {
+    g_warning ("Invalid certificate");
     ephy_sync_crypto_rsa_key_pair_free (self->keypair);
-    self->locked = FALSE;
-    goto out;
+    goto free_node;
   }
 
   self->certificate = g_strdup (certificate);
-  ephy_sync_service_obtain_storage_credentials (self);
+  is_internal_error = FALSE;
 
-out:
+free_node:
   json_node_unref (node);
+out:
+  if (is_internal_error) {
+    self->locked = FALSE;
+    ephy_notification_show (ephy_notification_new (message ? message
+                                                           : _("Something went wrong while syncing."),
+                                                   suggestion ? suggestion
+                                                              : _("Please visit Preferences and sign in 
again.")));
+    g_free (message);
+  } else {
+    ephy_sync_service_obtain_storage_credentials (self);
+  }
 }
 
 static void
@@ -581,12 +655,10 @@ ephy_sync_service_obtain_signed_certificate (EphySyncService *self)
 
   g_assert (EPHY_IS_SYNC_SERVICE (self));
 
-  /* Generate a new RSA key pair that is going to be used to sign the new certificate. */
+  /* Generate a new RSA key pair to sign the new certificate. */
   if (self->keypair)
     ephy_sync_crypto_rsa_key_pair_free (self->keypair);
-
   self->keypair = ephy_sync_crypto_generate_rsa_key_pair ();
-  g_assert (self->keypair);
 
   /* Derive tokenID, reqHMACkey and requestKey from the sessionToken. */
   ephy_sync_crypto_process_session_token (ephy_sync_service_get_token (self, TOKEN_SESSIONTOKEN),
@@ -945,8 +1017,7 @@ destroy_session_cb (SoupSession *session,
 }
 
 void
-ephy_sync_service_destroy_session (EphySyncService *self,
-                                   const char      *sessionToken)
+ephy_sync_service_destroy_session (EphySyncService *self)
 {
   SyncCryptoHawkOptions *hoptions;
   SyncCryptoHawkHeader *hheader;
@@ -957,21 +1028,14 @@ ephy_sync_service_destroy_session (EphySyncService *self,
   char *tokenID_hex;
   char *url;
   const char *content_type = "application/json";
-  const char *endpoint = "session/destroy";
   const char *request_body = "{}";
 
   g_return_if_fail (EPHY_IS_SYNC_SERVICE (self));
+  g_return_if_fail (ephy_sync_service_get_token (self, TOKEN_SESSIONTOKEN));
 
-  if (!sessionToken) {
-    sessionToken = ephy_sync_service_get_token (self, TOKEN_SESSIONTOKEN);
-    if (!sessionToken) {
-      g_warning ("Cannot destroy session: missing sessionToken");
-      return;
-    }
-  }
-
-  url = g_strdup_printf ("%s%s", MOZILLA_FXA_SERVER_URL, endpoint);
-  ephy_sync_crypto_process_session_token (sessionToken, &tokenID, &reqHMACkey, &requestKey);
+  url = g_strdup_printf ("%ssession/destroy", MOZILLA_FXA_SERVER_URL);
+  ephy_sync_crypto_process_session_token (ephy_sync_service_get_token (self, TOKEN_SESSIONTOKEN),
+                                          &tokenID, &reqHMACkey, &requestKey);
   tokenID_hex = ephy_sync_crypto_encode_hex (tokenID, 0);
 
   msg = soup_message_new (SOUP_METHOD_POST, url);
@@ -1004,7 +1068,7 @@ ephy_sync_service_report_sign_in_error (EphySyncService *self,
   g_assert (message);
 
   g_signal_emit (self, signals[SIGN_IN_ERROR], 0, message);
-  ephy_sync_service_destroy_session (self, NULL);
+  ephy_sync_service_destroy_session (self);
 
   if (clear_tokens) {
     ephy_sync_service_set_user_email (self, NULL);
@@ -1020,48 +1084,82 @@ check_storage_version_cb (SoupSession *session,
   EphySyncService *self;
   JsonParser *parser;
   JsonObject *json;
+  GError *error = NULL;
   char *payload;
-  char *message;
+  char *message = NULL;
   int storage_version;
+  gboolean is_internal_error = TRUE;
 
   self = ephy_shell_get_sync_service (ephy_shell_get_default ());
 
   if (msg->status_code != 200) {
     g_warning ("Failed to check storage version. Status code: %u, response: %s",
                msg->status_code, msg->response_body->data);
-    ephy_sync_service_report_sign_in_error (self,
-                                            _("Something went wrong, please try again."),
-                                            TRUE);
     goto out;
   }
 
   parser = json_parser_new ();
-  json_parser_load_from_data (parser, msg->response_body->data, -1, NULL);
+  json_parser_load_from_data (parser, msg->response_body->data, -1, &error);
+  if (error) {
+    g_warning ("Response is not a valid JSON: %s", error->message);
+    g_error_free (error);
+    goto free_parser;
+  }
+  if (!JSON_NODE_HOLDS_OBJECT (json_parser_get_root (parser))) {
+    g_warning ("JSON node does not hold a JSON object");
+    goto free_parser;
+  }
+
   json = json_node_get_object (json_parser_get_root (parser));
+  if (!json_object_has_member (json, "payload") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "payload"))) {
+    g_warning ("JSON object has missing or invalid 'payload' member");
+    goto free_parser;
+  }
   payload = g_strdup (json_object_get_string_member (json, "payload"));
-  json_parser_load_from_data (parser, payload, -1, NULL);
+  json_parser_load_from_data (parser, payload, -1, &error);
+  if (error) {
+    g_warning ("Payload is not a valid JSON: %s", error->message);
+    g_error_free (error);
+    goto free_payload;
+  }
+  if (!JSON_NODE_HOLDS_OBJECT (json_parser_get_root (parser))) {
+    g_warning ("JSON node does not hold a JSON object");
+    goto free_payload;
+  }
   json = json_node_get_object (json_parser_get_root (parser));
+  if (!json_object_has_member (json, "storageVersion") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "storageVersion"))) {
+    g_warning ("JSON object has missing or invalid 'storageVersion' member");
+    goto free_payload;
+  }
   storage_version = json_object_get_int_member (json, "storageVersion");
-
-  /* If the storage version is correct, proceed to store the tokens.
-   * Otherwise, signal the error to the user. */
-  if (storage_version == STORAGE_VERSION) {
-    ephy_sync_secret_store_tokens (self);
-  } else {
+  if (storage_version != STORAGE_VERSION) {
     LOG ("Unsupported storage version: %d", storage_version);
     /* Translators: the %d is the storage version, the \n is a newline character. */
-    message = g_strdup_printf (_("Your Firefox Account uses a storage version "
-                                       "that Epiphany does not support, namely v%d.\n"
-                                       "Create a new account to use the latest storage version."),
-                                     storage_version);
-    ephy_sync_service_report_sign_in_error (self, message, TRUE);
-    g_free (message);
+    message = g_strdup_printf (_("Your Firefox Account uses storage version %d "
+                                 "which Epiphany does not support.\n"
+                                 "Create a new account to use the latest storage version."),
+                               storage_version);
+    goto free_payload;
   }
 
+  /* Proceed to store tokens. */
+  ephy_sync_secret_store_tokens (self);
+  is_internal_error = FALSE;
+
+free_payload:
   g_free (payload);
+free_parser:
   g_object_unref (parser);
-
 out:
+  if (is_internal_error) {
+    ephy_sync_service_report_sign_in_error (self,
+                                            message ? message
+                                                    : _("Something went wrong, please try again."),
+                                            TRUE);
+    g_free (message);
+  }
   ephy_sync_service_send_next_storage_request (self);
 }
 
@@ -1091,9 +1189,15 @@ ephy_sync_service_conclude_sign_in (EphySyncService *self,
 
   /* Derive the master sync keys form the key bundle. */
   unwrapKB = ephy_sync_crypto_decode_hex (data->unwrapBKey);
-  ephy_sync_crypto_compute_sync_keys (bundle, data->respHMACkey,
-                                      data->respXORkey, unwrapKB,
-                                      &kA, &kB);
+  if (!ephy_sync_crypto_compute_sync_keys (bundle, data->respHMACkey,
+                                           data->respXORkey, unwrapKB,
+                                           &kA, &kB)) {
+    ephy_sync_service_report_sign_in_error (self,
+                                            _("Failed to retrieve the Sync Key"),
+                                            FALSE);
+    goto out;
+  }
+
   kB_hex = ephy_sync_crypto_encode_hex (kB, 0);
 
   /* Save the email and the tokens. */
@@ -1104,9 +1208,10 @@ ephy_sync_service_conclude_sign_in (EphySyncService *self,
 
   ephy_sync_service_check_storage_version (self);
 
-  g_free (kA);
-  g_free (kB);
   g_free (kB_hex);
+  g_free (kB);
+  g_free (kA);
+out:
   g_free (unwrapKB);
   sign_in_async_data_free (data);
 }
@@ -1120,32 +1225,65 @@ get_account_keys_cb (SoupSession *session,
   SignInAsyncData *data;
   JsonNode *node;
   JsonObject *json;
+  GError *error = NULL;
+  gboolean is_internal_error = TRUE;
 
   self = ephy_shell_get_sync_service (ephy_shell_get_default ());
   data = (SignInAsyncData *)user_data;
-  node = json_from_string (msg->response_body->data, NULL);
+  node = json_from_string (msg->response_body->data, &error);
+  if (error) {
+    g_warning ("Response is not a valid JSON: %s", error->message);
+    g_error_free (error);
+    goto out;
+  }
+  if (!JSON_NODE_HOLDS_OBJECT (node)) {
+    g_warning ("JSON node does not hold a JSON object");
+    goto free_node;
+  }
   json = json_node_get_object (node);
 
-  if (msg->status_code == 200) {
-    /* Extract the master sync keys from the bundle and save tokens. */
-    ephy_sync_service_conclude_sign_in (self, data,
-                                        json_object_get_string_member (json, "bundle"));
-  } else if (msg->status_code == 400 && json_object_get_int_member (json, "errno") == 104) {
-    /* Poll the Firefox Accounts Server until the user verifies the account. */
-    LOG ("Account not verified, retrying...");
-    ephy_sync_service_fxa_hawk_get_async (self, "account/keys", data->tokenID_hex,
-                                          data->reqHMACkey, EPHY_SYNC_TOKEN_LENGTH,
-                                          get_account_keys_cb, data);
-  } else {
+  if (msg->status_code != 200) {
+    if (!json_object_has_member (json, "errno") ||
+        !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "errno"))) {
+      g_warning ("JSON objetc has invalid 'errno' member");
+      goto free_node;
+    }
+
+    /* If account in not verified, poll the Firefox Accounts Server until the
+     * verification has completed. */
+    if (json_object_get_int_member (json, "errno") == 104) {
+      LOG ("Account not verified, retrying...");
+      ephy_sync_service_fxa_hawk_get_async (self, "account/keys", data->tokenID_hex,
+                                            data->reqHMACkey, EPHY_SYNC_TOKEN_LENGTH,
+                                            get_account_keys_cb, data);
+      is_internal_error = FALSE;
+      goto free_node;
+    }
+
     g_warning ("Failed to GET /account/keys. Status code: %u, response: %s",
                msg->status_code, msg->response_body->data);
+    goto free_node;
+  }
+
+  if (!json_object_has_member (json, "bundle") ||
+      !JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "bundle"))) {
+    g_warning ("JSON object has invalid or missing 'bundle' member");
+    goto free_node;
+  }
+  /* Extract the master sync keys from the bundle and save tokens. */
+  ephy_sync_service_conclude_sign_in (self, data,
+                                      json_object_get_string_member (json, "bundle"));
+  is_internal_error = FALSE;
+
+free_node:
+  json_node_unref (node);
+out:
+  if (is_internal_error) {
     ephy_sync_service_report_sign_in_error (self,
                                             _("Failed to retrieve the Sync Key"),
                                             FALSE);
     sign_in_async_data_free (data);
   }
-
-  json_node_unref (node);
 }
 
 void
@@ -1198,7 +1336,7 @@ ephy_sync_service_do_sign_out (EphySyncService *self)
 
   /* Destroy session and delete tokens. */
   ephy_sync_service_stop_periodical_sync (self);
-  ephy_sync_service_destroy_session (self, NULL);
+  ephy_sync_service_destroy_session (self);
   ephy_sync_service_clear_storage_credentials (self);
   ephy_sync_service_clear_tokens (self);
   ephy_sync_secret_forget_tokens ();
@@ -1526,7 +1664,7 @@ ephy_sync_service_sync_collection (EphySyncService           *self,
   is_initial = ephy_synchronizable_manager_is_initial_sync (manager);
   data = sync_collection_async_data_new (manager, is_initial, collection_index, num_collections);
 
-  LOG ("Syncing %s collection...", collection);
+  LOG ("Syncing %s collection%s...", collection, is_initial ? " first time" : "");
   ephy_sync_service_queue_storage_request (self, endpoint, SOUP_METHOD_GET, NULL,
                                            is_initial ? -1 : ephy_synchronizable_manager_get_sync_time 
(manager),
                                            -1, sync_collection_cb, data);
diff --git a/src/sync/ephy-sync-service.h b/src/sync/ephy-sync-service.h
index 1988457..ce3522e 100644
--- a/src/sync/ephy-sync-service.h
+++ b/src/sync/ephy-sync-service.h
@@ -47,8 +47,7 @@ SyncCryptoKeyBundle *ephy_sync_service_get_key_bundle             (EphySyncServi
                                                                    const char                *collection);
 void                 ephy_sync_service_clear_storage_credentials  (EphySyncService           *self);
 void                 ephy_sync_service_clear_tokens               (EphySyncService           *self);
-void                 ephy_sync_service_destroy_session            (EphySyncService           *self,
-                                                                   const char                *sessionToken);
+void                 ephy_sync_service_destroy_session            (EphySyncService           *self);
 void                 ephy_sync_service_do_sign_in                 (EphySyncService           *self,
                                                                    const char                *email,
                                                                    const char                *uid,



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]