mailvelope / keyserver

A simple OpenPGP public key server that validates email address ownership of uploaded keys.
https://keys.mailvelope.com
GNU Affero General Public License v3.0
413 stars 62 forks source link

Support ferretdb to replace mongodb #142

Open pravi opened 5 months ago

pravi commented 5 months ago

FerretDB aims to replace MongoDB since the latter is no longer Free Software.

At present it fails with MongoServerError: Index option "expireAfterSeconds" is not implemented yet

But it could be worked around until this is implemented, https://github.com/FerretDB/FerretDB/issues/2415#issuecomment-2037868295 "include a check for the property in question in your actual query, so results that are too old just don't come back, and do your own purges of stuff older than that periodically."

It would be nice to be able to setup mailvelope keyserver with fully Free Software dependencies.

pravi commented 5 months ago

I was able to get the keyserver to start with this change,

diff --git a/src/modules/public-key.js b/src/modules/public-key.js
index f6210ba..dbb6d47 100644
--- a/src/modules/public-key.js
+++ b/src/modules/public-key.js
@@ -54,7 +54,7 @@ class PublicKey {

   async init() {
     // create time to live (TTL) index to purge unverified keys
-    await this._mongo.createIndexes([{key: {verifyUntil: 1}, expireAfterSeconds: 1}], DB_TYPE);
+    await this._mongo.createIndexes([{key: {verifyUntil: 1}}], DB_TYPE);
   }

   /**

I could upload keys and receive verification emails, but after verifying emails, search gives an error "unknown operator: email" and trying to remove a key results in "User ID not found" when clicking on the verification link received via email. Not sure if this is related to the above change or something missing in ferretdb.

pravi commented 5 months ago

npm test shows 6 failing tests


 1) Mongo Integration Tests
       "before all" hook in "Mongo Integration Tests":
     MongoParseError: authMechanism one of MONGODB-AWS,MONGODB-CR,DEFAULT,GSSAPI,PLAIN,SCR
AM-SHA-1,SCRAM-SHA-256,MONGODB-X509,MONGODB-OIDC, got PLAIN-int
      at transform (node_modules/mongodb/lib/connection_string.js:486:23)
      at setOption (node_modules/mongodb/lib/connection_string.js:458:36)
      at parseOptions (node_modules/mongodb/lib/connection_string.js:283:13)
      at new MongoClient (node_modules/mongodb/lib/mongo_client.js:51:63)
      at Mongo.init (src/modules/mongo.js:25:20)
      at Context.<anonymous> (test/integration/mongo-test.js:19:17)
      at process.processImmediate (node:internal/timers:476:21)

  2) Mongo Integration Tests
       "after all" hook in "Mongo Integration Tests":
     TypeError: Cannot read properties of undefined (reading 'collection')
      at Mongo.clear (src/modules/mongo.js:135:26)
      at Context.<anonymous> (test/integration/mongo-test.js:28:17)
      at process.processImmediate (node:internal/timers:476:21)

  3) Public Key Integration Tests
       "before all" hook in "Public Key Integration Tests":
     MongoParseError: authMechanism one of MONGODB-AWS,MONGODB-CR,DEFAULT,GSSAPI,PLAIN,SCRAM-SHA-1,SCRAM-SHA-256,MONGODB-X509,MONGODB-OIDC, got PLAIN-int
      at transform (node_modules/mongodb/lib/connection_string.js:486:23)
      at setOption (node_modules/mongodb/lib/connection_string.js:458:36)
      at parseOptions (node_modules/mongodb/lib/connection_string.js:283:13)
      at new MongoClient (node_modules/mongodb/lib/mongo_client.js:51:63)
      at Mongo.init (src/modules/mongo.js:25:20)
      at Context.<anonymous> (test/integration/public-key-test.js:42:17)

  4) Public Key Integration Tests
       "after all" hook in "Public Key Integration Tests":
     TypeError: Cannot read properties of undefined (reading 'collection')
      at Mongo.clear (src/modules/mongo.js:135:26)
      at Context.<anonymous> (test/integration/public-key-test.js:82:17)
      at process.processImmediate (node:internal/timers:476:21)

  5) Key Server Integration Tests
       "before all" hook in "Key Server Integration Tests":
     MongoParseError: authMechanism one of MONGODB-AWS,MONGODB-CR,DEFAULT,GSSAPI,PLAIN,SCRAM-SHA-1,SCRAM-SHA-256,MONGODB-X509,MONGODB-OIDC, got PLAIN-int
      at transform (node_modules/mongodb/lib/connection_string.js:486:23)
      at setOption (node_modules/mongodb/lib/connection_string.js:458:36)
      at parseOptions (node_modules/mongodb/lib/connection_string.js:283:13)
      at new MongoClient (node_modules/mongodb/lib/mongo_client.js:51:63)
      at Mongo.init (src/modules/mongo.js:25:20)
      at Context.<anonymous> (test/integration/server-test.js:31:17)

  6) Key Server Integration Tests
       "after all" hook in "Key Server Integration Tests":
     TypeError: Cannot read properties of undefined (reading 'collection')
      at Mongo.clear (src/modules/mongo.js:135:26)
      at Context.<anonymous> (test/integration/server-test.js:54:17)
      at process.processImmediate (node:internal/timers:476:21)
toberndo commented 5 months ago

But it could be worked around until this is implemented, FerretDB/FerretDB#2415 (comment) "include a check for the property in question in your actual query, so results that are too old just don't come back, and do your own purges of stuff older than that periodically."

Here you can revert https://github.com/mailvelope/keyserver/commit/bf2b856091dfb817f08aa840ca86aff54943c7a7 to bring back unverified key purges without requiring TTL.

toberndo commented 5 months ago

I could upload keys and receive verification emails, but after verifying emails, search gives an error "unknown operator: email" and trying to remove a key results in "User ID not found" when clicking on the verification link received via email. Not sure if this is related to the above change or something missing in ferretdb.

Does not sound like these errors are related to the change.

pravi commented 5 months ago

Does not sound like these errors are related to the change. There are no errors in the console even with LOG_LEVEL=debug passed to npm start. Any other way to get more verbose logs?

pravi commented 5 months ago

Here you can revert bf2b856 to bring back unverified key purges without requiring TTL.

git revert does not work for this commit, so I checked out its parent commit. But same error for searching. I created the recommended indexes too. Any ideas for further debugging?

pravi commented 5 months ago

When I run db.publickey.find() in mogosh, I see all uploaded keys have verified: false, so it seems the problem is in key verification.


_id: ObjectId('6645f71ce1fc4753fd1365f4'),
    keyId: '8f53e0193b294b75',
    fingerprint: 'd30863e26020e543f4719a838f53e0193b294b75',
    userIds: [
      {
        name: 'Praveen Arimbrathodiyil',
        email: 'praveen@onenetbeyond.org',
        verified: false,
        publicKeyArmored: '-----BEGIN PGP PUBLIC KEY BLOCK-----\n' +

Though clicking on the email link says "Email address praveen@onenetbeyond.org successfully verified"

image

pravi commented 5 months ago

We should probably double check verified field is actually set to true before showing the verification successful message. Could this be a problem with ferretdb missing some features or behaving differently?

toberndo commented 5 months ago

The verified field is set here: https://github.com/mailvelope/keyserver/blob/master/src/modules/public-key.js#L212 Maybe ferretdb does not work well with the userIds.$.verified semantic?

pravi commented 5 months ago

The verified field is set here: https://github.com/mailvelope/keyserver/blob/master/src/modules/public-key.js#L212 Maybe ferretdb does not work well with the userIds.$.verified semantic?

Searching for .$. gave this https://github.com/FerretDB/FerretDB/blob/def90a13a31e901361d04038792452f14e78a534/internal/handler/common/projection.go#L106 so it looks like positional projection can only be used at the end. Is there another way to express the same?

https://github.com/FerretDB/FerretDB/issues/835

pravi commented 5 months ago

Looking at https://www.mongodb.com/docs/manual/reference/operator/projection/positional/#positional-operator-placement-restriction this seems invalid

asdofindia commented 5 months ago

The update query matches nothing with ferretdb

ferretdb> db.publickey.updateOne({keyId: 'e19afa78dce1e3dc','userIds.nonce': '583499bb46c7f6177ec6ecf09274640b'}, {"$set": {verifyUntil: null}})
{
  acknowledged: true,
  insertedId: null,
  matchedCount: 0,
  modifiedCount: 0,
  upsertedCount: 0
}
ferretdb> db.publickey.updateOne({keyId: 'e19afa78dce1e3dc'}, {"$set": {verifyUntil: null}})
{
  acknowledged: true,
  insertedId: null,
  matchedCount: 1,
  modifiedCount: 1,
  upsertedCount: 0
}

This is likely because ferretdb does not support nested arrays.

And userIds is a nested array (an array of embedded documents).

So the query there which includes userIds.nonce is failing to match anything. (Please note that find, and findOne does seem to work with nested array with ferretdb, but only update fails).

This diff might help to get past this:

❯ git diff src/modules/public-key.js
diff --git a/src/modules/public-key.js b/src/modules/public-key.js
index f6210ba..8cc1810 100644
--- a/src/modules/public-key.js
+++ b/src/modules/public-key.js
@@ -209,12 +209,22 @@ class PublicKey {
       publicKeyArmored = await this._pgp.updateKey(key.publicKeyArmored, publicKeyArmored);
     }
     // flag the user ID as verified
-    await this._mongo.update(query, {
+    const updatedUserIds = key.userIds.map(uid => {
+      if (uid.nonce === nonce) {
+        return {
+          verified: true,
+          nonce: null,
+          publicKeyArmored: null,
+          name: uid.name,
+          email: uid.email
+        };
+      }
+      return uid;
+    });
+    await this._mongo.update({'_id': key['_id']}, {
       publicKeyArmored,
-      'userIds.$.verified': true,
-      'userIds.$.nonce': null,
-      'userIds.$.publicKeyArmored': null,
-      verifyUntil: null
+      verifyUntil: null,
+      userIds: updatedUserIds
     }, DB_TYPE);
     return {email};
   }

But that's again going to be heartbreak as when you search you'll get a new error: "unknown operator: email" which arises from

https://github.com/mailvelope/keyserver/blob/60da5dcb566cd50444c64c8139248a77999acceb/src/modules/public-key.js#L265-L266

The $elemMatch seems to have different behaviour in ferret supporting only comparison operators like "$eq".

You could perhaps work around that too, but at this point, it might be easier to just write a module to make keyserver store data directly in postgres instead of this mongo -> ferret -> postgres incompatibilities

pravi commented 5 months ago

@asdofindia thanks, with this, search by keyid works. For example, https://keys.puri.sm/pks/lookup?op=get&search=0xD30863E26020E543F4719A838F53E0193B294B75

pravi commented 5 months ago

@toberndo do you think switching directly to PostgreSQL is possible?

pravi commented 5 months ago

With the following change, search by email is working.

--- a/src/modules/public-key.js
+++ b/src/modules/public-key.js
@@ -268,11 +268,10 @@ class PublicKey {
     // query by user id
     if (userIds) {
       queries = queries.concat(userIds.map(uid => ({
-        userIds: {
-          $elemMatch: {
-            'email': util.normalizeEmail(uid.email),
-            'verified': true
-          }
+        'userIds.email': {
+            $eq: util.normalizeEmail(uid.email)},
+        'userIds.verified': {
+            $eq: true
         }
       })));
     }
pravi commented 5 months ago

Now removing a key is failing with 'User ID not found' when clicking on the link received by email.

We need to replace 'userIds.$.nonce' like before in

https://github.com/mailvelope/keyserver/blob/master/src/modules/public-key.js#L342

pravi commented 5 months ago

With this change, I'm able to remove by email as well,

--- a/src/modules/public-key.js
+++ b/src/modules/public-key.js
@@ -345,7 +345,19 @@ class PublicKey {
     // flag only the provided user id
     if (email) {
       const nonce = util.random();
-      await this._mongo.update(query, {'userIds.$.nonce': nonce}, DB_TYPE);
+      const flaggedUserIds = key.userIds.map(uid => {
+        if (uid.email === email) {
+          return {
+            nonce: nonce,
+            verified: uid.verified,
+            publicKeyArmored: uid.publicKeyArmored,
+            name: uid.name,
+            email: uid.email
+          }
+        }
+        return uid;
+      });
pravi commented 5 months ago

I have pushed these changes here https://source.puri.sm/Purism/keyserver/-/commits/ferretdb-compat/?ref_type=HEADS

I'd be happy if this is maintained as separate branch in this repo itself.

asdofindia commented 5 months ago

@pravi happy to hear you got it working. I've done only an eyeball review. In the remove part, as per the code the key can be removed by keyId as well. I'm not sure if your patch supports that. Other changes in https://source.puri.sm/Purism/keyserver/-/compare/master...ferretdb-compat?from_project_id=3240&straight=false looks like they will work (although I'm not sure whether that breaks compatibility with any mongo version)

pravi commented 5 months ago

@asdofindia I saw the remove by keyid part in the code as well, but there is no such option in the web ui. So I thought I could skip that for now.

toberndo commented 5 months ago

I have pushed these changes here https://source.puri.sm/Purism/keyserver/-/commits/ferretdb-compat/?ref_type=HEADS

I'd be happy if this is maintained as separate branch in this repo itself.

I pushed the changes to branch https://github.com/mailvelope/keyserver/tree/ferretdb-compat