En avril 2021, alors que nous cherchions comment développer un plugin de chiffrement de bout en bout pour OnlyOffice web en utilisant le SDK E2EE de Seald, nous, chez Seald, avons examiné le code source du plugin développé pour OnlyOffice Desktop et avons trouvé quelques faiblesses dans la façon dont l'E2EE était implémenté.

Avertissement : Cette revue n'était en aucun cas une revue complète du code ni un audit de sécurité formel. Certaines faiblesses peuvent ne pas avoir été trouvées au cours de cette revue, et les recommandations peuvent ne pas être parfaites.

Résumé : Nous avons trouvé de multiples vulnérabilités critiques dans la mise en œuvre d'E2EE de la dernière version au moment de l'audit (en avril 2021), qui était 6.3.0 :

  • le PRNG n'était pas sûr
  • la dérivation du mot de passe n'avait pas de sel
  • le chiffrement symétrique n'était pas authentifié et réutilisait des IV
  • il y avait un potentiel MITM en cas d'attaque active sur le serveur.

Politique de divulgation : la politique de divulgation du Google Project Zero (qui est pour nous une référence dans le domaine) est de publier publiquement après un maximum de 120 jours après le rapport initial. Dans ce cas, le rapport initial a été fait il y a 433 jours. Nous avons ainsi donc décidé de publier ce rapport publiquement.

Historique

  • 23/04/2021: Nous avons contacté plusieurs membres de l'équipe de développement d'OnlyOffice pour avoir plus de détails concernant la fonctionnalité Private Rooms, qui met en œuvre le chiffrement de bout en bout avec un plugin.
  • 26/04/2021: un responsable marketing d'OnlyOffice m'a envoyé le lien GitHub vers le code source du plugin E2EE.
  • 27/04/2021:  nous avons recontacté cette personne par email pour la prévenir que nous voulions envoyer ce rapport. Cette personne m'a renvoyé vers le développeur en charge du développement du plugin de E2EE. Le développeur en charge du plugin a demandé à l'équipe sécurité de nous envoyer les instructions sur la façon d'envoyer un rapport.
  • 30/04/2021: nous avons envoyé le rapport conformément à leurs instructions.
  • 03/05/2021: nous avons reçu confirmation que le rapport avait été bien reçu et des correctifs priorisés.
  • 23/06/2021: nous avons reçu la confirmation que dans la version 6.3.1, le RNG avait été modifié que le l'AES-CBC avait été changé en de l'AES-GCM. Nous n'avons pas revu leurs modifications, et ils nous ont assuré que les autres problèmes seraient corrigés dans une prochaine version du DocumentServer, ce que nous n'avons pas vérifié.
  • 07/07/2022: Étant donné que l'échéance de divulgation du Google Project Zero est de 120 jours au maximum et que cela fait plus d'un an que notre rapport a été transmis à OnlyOffice, il nous semble raisonnable de publier ce rapport publiquement aujourd'hui.

Résultats

Les résultats sont classés par type d'opération plutôt que par criticité.

Ils concernent tous la version 6.3.0, qui était la plus récente au moment de la rédaction de ce rapport (en avril 2021).

Chiffrement symétrique

Générateur de nombres pseudo-aléatoires

Le code utilisé par OnlyOffice pour, in fine, générer un mot de passe est le suivant :

window.AscCrypto.CryptoWorker.createPasswordNew = function() {
  function s4() {
    return Math.floor((1 + Math.random()) * 0x10000).toString(16).substring(1);
  }
  return s4() + s4() + '-' + s4() + '-' + s4() + '-' + s4() + '-' + s4() + s4() + s4();
}

Il utilise comme source d'aléatoire la fonction Math.random() qui n'est pas cryptographiquement sûre.

Des faiblesses de cette fonction ont été exploitées par le passé, comme exposé ici. La version de V8 utilisée est plus résistante à ce type d'exploits, mais elle n'est toujours pas cryptographiquement sûre, comme expliqué sur le blog de V8 : https://v8.dev/blog/math-random puisque cela utilise xorshift128+.

Ce problème est CRITIQUE car il pourrait conduire un attaquant à deviner quelles clés de chiffrement sont utilisées.

Nous avions recommandé soit de :

  • utiliser les APIs webcrypto si Chromium Embedded Framework le permet, ou ;
  • utiliser node-forge qui possède une solution de repli JS pour générer un aléatoire plus sûr, ou ;
  • utiliser les bindings Cpp vers le PRNG sécurisé d'OpenSSL comme pour les autres primitives.

Dérivation de mot de passe

Lors de l'initialisation de la "session" AES, OnlyOffice appelait CryptoAES_Init avec un argument password depuis worker.js, ce qui est défini ici:

else if (name == "CryptoAES_Init")
{
    std::string sPassword = arguments[0]->GetStringValue().ToString();
    std::string sSalt = "";
    if (arguments.size() > 1)
        sSalt = arguments[0]->GetStringValue().ToString();
    if (NULL != m_pAES_KeyIv)
        NSOpenSSL::openssl_free(m_pAES_KeyIv);
    m_pAES_KeyIv = NSOpenSSL::PBKDF2_desktop(sPassword, sSalt);
    return true;
}

Cette fonction a deux problèmes :

  • le premier est que sSalt n'est jamais donné (puisque toujours appelé avec un seul argument);
  • le second est que si elle était appelée avec un deuxième argument, il ne serait pas utilisé: sSalt est initialisé avec arguments[0] au lieu de arguments[1], donc PBKDF2_desktop serait appelé avec sPassword en tant que sPassword et sSalt.

PBKDF2_desktop est défini ici:

unsigned char* PBKDF2_desktop(const std::string& pass, const std::string& salt)
{
    unsigned char* key_iv = NULL;
    if (salt.empty())
    {
        unsigned int pass_salt_len = 0;
        unsigned char* pass_salt = NSOpenSSL::GetHash((unsigned char*)pass.c_str(), (unsigned int)pass.length(), OPENSSL_HASH_ALG_SHA512, pass_salt_len);
        key_iv = PBKDF2(pass.c_str(), (int)pass.length(), pass_salt, pass_salt_len, OPENSSL_HASH_ALG_SHA256, 32 + 16);
        openssl_free(pass_salt);
    }
    else
    {
        key_iv = PBKDF2(pass.c_str(), (int)pass.length(), (const unsigned char*)salt.c_str(), (unsigned int)salt.length(), OPENSSL_HASH_ALG_SHA256, 32 + 16);
    }
    return key_iv;
}

Puisque le salt est vide dans ce codepath, OnlyOffice dérive le salt du mot de passe directement en utilisant SHA512.

Cela conduit à toujours avoir la même clé pour un mot de passe donné, ce qui est une mauvaise pratique en soi.

Ce problème est MINEUR, puisque cela serait réellement problématique si le mot de passe était un véritable "mot de passe" défini par l'utilisateur, mais il s'agit plutôt d'une clé générée aléatoirement.

Nous avions recommandé de générer un seul aléatoire et de le stocker dans les informations du document, là où les clés symétriques chiffrées sont stockées, et de les récupérer via readPassword qui parse déjà docInfo.

Chiffrement / déchiffrement

Le chiffrement était effectué via worker.js qui appelle CryptoAES_Encrypt, qui appelle du code Cpp qui appelle AES_Encrypt_desktop défini ailleurs:

bool AES_Encrypt_desktop(const unsigned char* key_iv, const std::string& input, std::string& output)
{
    unsigned char* data_crypt = NULL;
    unsigned int data_crypt_len = 0;
    bool bRes = AES_Encrypt(OPENSSL_AES_256_CBC, key_iv, key_iv + 32, (unsigned char*)input.c_str(), (unsigned int)input.length(), data_crypt, data_crypt_len);
    if (!bRes)
        return false;
    output = Serialize(data_crypt, data_crypt_len, OPENSSL_SERIALIZE_TYPE_BASE64);
    openssl_free(data_crypt);
    return true;
}

Cela a deux problèmes majeurs:

1/ L'IV est réutilisé entre plusieurs chiffrements:

L'IV provient de la dérivation du mot de passe (avec le PBKDF2 décrit au paragraphe du dessus), qui donne toujours le même IV pour un mot de passe donné.

Ce problème est CRITIQUE puisque le mode CBC est utilisé, ce qui peut faire une fuite d'information entre différentes versions d'un document qui sont toutes un peu similaires.

Nous avions recommandé de générer un IV aléatoire de 16 octets à chaque opération de chiffrement, et de préfixer le cipherText de cet IV.

2/ L'algorithme de chiffrement n'est pas authentifié

AES-CBC n'est pas un algorithme de chiffrement authentifié (contrairement à ChaCha20 ou AES-GCM), ce qui veut dire qu'un attaquant pourrait injecter des blocs malveillants dans le cipherText, et la donnée serait déchiffrée sans avertissement. Cela a conduit (en combinaison avec d'autres vulnérabilités) à des attaques comme efail.de, qui ont réussi à exflitrer des données chiffrées par l'injection de CBC gadgets, qui seront déchiffrés en un payload qui exploite une vulnérabilité XSS présente dans le client affichant les données déchiffrées. Cela semble tiré par les cheveux, mais il y a eu des exploits de ce genre d'attaques.

Ce problème est CRITIQUE, puisqu'un attaquant pourrait altérer un fichier chiffré de façon à ce que quand l'utilisateur le déchiffre, l'attaquant puisse injecter un payload au début du document qui serait la base d'un exploit en plusieurs étapes.

Nous avions recommandé de soit:

  • Changer d'algorithme de chiffrement pour utiliser ChaCha20 ou AES-GCM, or;
  • Implémenter un mécanisme de MAC (comme HMAC-SHA256) sur le {IV + cipherText}, calculer ce MAC avec une autre clé, concaténer le résultat du MAC avec le cipherText et IV, et vérifier le MAC avant déchiffrement. Si la vérification du MAC échoue, cela indique que le cipherText a été altéré, il faut donc cesser le déchiffrement. Pour générer l'autre clé, on recommande de dériver 64 octets à partir du mot de passe et non 32 (en supposant que l'IV n'est plus dérivé du mot de passe), et utiliser les 32 premiers octets pour la clé AES et les 32 suivants comme la clé de MAC.

La fonction sous-jacente qui execute l'AES AES_Encrypt est étrange:

    bool AES_Encrypt(int type, const unsigned char* key, const unsigned char* iv, const unsigned char* data, const unsigned int& size, unsigned char*& data_crypt, unsigned int& data_crypt_len)
    {
        EVP_CIPHER_CTX* ctx = EVP_CIPHER_CTX_new();
        EVP_CIPHER_CTX_init(ctx);
        EVP_EncryptInit_ex(ctx, _get_cipher_aes(type), NULL, key, iv);
        EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, 16, NULL);
        int out_len1 = (int)size + AES_BLOCK_SIZE;
        int out_len2 = 0;
        data_crypt = openssl_alloc(out_len1);
        EVP_EncryptUpdate(ctx, data_crypt, &out_len1, data, (int)size);
        EVP_EncryptFinal_ex(ctx, data_crypt + out_len1, &out_len2);
        data_crypt_len = out_len1 + out_len2;
        EVP_CIPHER_CTX_free(ctx);
        EVP_cleanup();
        return true;
    }

Cela instancie un Cipher conext avec CBC et utilise une fonction pour définir la longueur de l'IV avec une constante spécifique à un autre mode de chiffrement EVP_CTRL_GCM_SET_IVLEN (spécifique à AES-GCM). On ne comprend pas exactement l'effet de cet appel de fonction ce qui n'est jamais bon signe lorsque l'on fait de la cryptographie.

Ce problème nécessite plus de recherches, étant donné que nous sommes incapables de déterminer les effets secondaires d'un tel appel de fonction.

Chiffrement asymétrique

Génération de clés

OnlyOffice utilise RSA_generate_multi_prime_key avec l'arguement primes avec la valeur 2 ce qui est le comportement par défaut de RSA_generate_key_ex.

Ce n'est pas un problème, simplement une bizarrerie.

Chiffrement / déchiffrement

Si le code path USE_DEPRECATED est utilisé, cela utilise RSA_NO_PADDING ce qui serait un problème CRITIQUE, mais cela a apparemment déjà été détecté.

L'autre code path qui utilise RSA_PKCS1_OAEP_PADDING n'a pas de souci (même si Victor Shoup a proposé OAEP+ qui est encore plus "optimal" en 2001, mais il n'a jamais été implémenté parce que les attaques contre OAEP ne sont que théoriques jusqu'à aujourd'hui).

Protection des clés privées

Les clés privées semblent protégées par le même chiffrement symétrique que décrit ci-dessus, avec les mêmes problèmes (qui sont CRITIQUES), avec un mot de passe m_sPassword provenant de tmpInfo (ce que l'on a trouvé ici).

Sur ce sujet, nous ne sommes pas familiers avec le mécanisme utilisé pour passer le mot de passe (utilisant tmpInfo), nous ne sommes pas certains que cela soit correctement géré.

Récupération des clés publiques

Les clés publiques proviennet de la commande  'getsharingkeys' sur le cloudCryptoCommandMainFrame qui ensuite semble déclencher  getEncryptionAccess ici, qui ensuite semble faire une requête au serveur de clés de la privacyRoom:

  return request({
    method: "get",
    url: `privacyroom/access/${fileId}`,
    data: fileId,
  });

Nous n'avons pas vu de mécanisme empêchant les serveurs d'OnlyOffice d'introduire un nouveau participant à une room. C'est une backdoor, litérallement: si le serveur devenait malveillant, il pourrait modifier les clés dans ce tableau avec une nouvelle clé publique pour laquelle l'attaquant aurait la clé privée, et de façon quasi silencieuse (il pourrait déchiffrer et rechiffrer pour les autres participants à la volée pour maintenir le même nombre de participants).

Ce problème est MAJEUR car cela peut mener à une backdoor littéralement si les serveurs étaient activement compromis.

La stratégie pour régler ce problème serait de signer la liste des participants (métadonnées et clés publiques) de façon à ce que le serveur ne puisse pas mentir, ou a minima qu'il ne puisse pas mentir une fois que les participants auront vérifié un hash de la liste des participants côté client via un canal séparé à un moment dans le cycle de vie de la room (c'est le paradigme Trust On First Use — TOFU). Si des signatures sont utilisées, pensez à utiliser une paire de clés différente pour chiffrer et signer, réutiliser la même peut mener à des attaques.

Conclusion

Ce rapport est basé sur une revue partielle du code uniquement, nous avons peut-être mal compris une partie du code, les résultats doivent être confirmés par l'équipe de développement d'OnlyOffice. Les recommandations que nous avons faites peuvent avoir elles-mêmes des défauts dont nous ne sommes pas conscients, leur robustesse doit également être confirmée avant de les mettre en œuvre.