vsivsi / meteor-file-collection

Extends Meteor Collections to handle file data using MongoDB gridFS.
http://atmospherejs.com/vsivsi/file-collection
Other
159 stars 37 forks source link

Adding mongoOptions as a pass through option object to mongo connecti… #127

Open oceddi opened 8 years ago

oceddi commented 8 years ago

…ons.

vsivsi commented 8 years ago

Hi, sorry it has taken me a couple of days to get to this...

Q: What mechanism is used by Meteor itself to specify TLS/SSL parameters (e.g. sslCA) for the default MongoDB connection used by a regular old Mongo.Collection?

You can easily use the MongoDB connection string in the MONGO_URL environment variable to specify ?ssl=true, but how do you then tell Meteor to use a given sslCA (or other ssl params) for that connection?

It seems from this issue/question that you still can't: https://github.com/meteor/meteor/issues/4812 http://stackoverflow.com/questions/26521138/can-meteor-connect-to-mongodb-over-ssl

Or rather, Meteor will use SSL for the connection, but the certificate is not validated (by default) for the connection over the old mongodb 1.4.x driver that Meteor uses internally. http://mongodb.github.io/node-mongodb-native/1.4/api-generated/server.html?highlight=sslValidate

Looking at the Meteor source code, there appears to be no way to accomplish the subject of this PR (passing in a sslCA) in Meteor itself. https://github.com/meteor/meteor/blob/devel/packages/mongo/mongo_driver.js#L127-L219

I understand that you are probably experiencing this as a bug, because in the newer MongoDB 2.1.x driver used by file-collection, the SSL cert validation is _enabled_ by default. http://mongodb.github.io/node-mongodb-native/2.1/reference/connecting/connection-settings/

So, the immediate issue is that a Meteor mongodb 1.4.x SSL connection ignores validation (and is required to), but the parallel file-collection mongodb 2.1.x SSL connection enforces validation.

So with that now understood: what is the best way in file-collection to fix this "bug"?

I am honestly a bit hesitant to just pass along a blanket mongoOptions object as you propose here. That potentially creates a very large testing surface for this package (by implying that all mongodb connection options are supported on the 2.1.x connection), and it is a bit misleading to the developer (by implying that the parallel Meteor 1.4.x connection will share those options, which it definitely won't...)

Given that Meteor itself does not support validation of a MongoDB SSL connection, I'm very tempted to say that file-collection also shouldn't take this on, and if that's the case, the solution to the issue you reported is as simple as:

@db = Meteor.wrapAsync(mongodb.MongoClient.connect)(process.env.MONGO_URL,{ sslValidate: false })

This has the virtue of solving your immediate problem without adding any additional complexity. It also may make it much simpler to migrate file-collection to use the Meteor mongoDB connection when they finally upgrade Meteor to use the up-to-date 2.1.x mongodb node.js driver.

That was a lot of text, but I fight hard to keep this package simple and maintainable, and I try to make changes that touch on security very seriously.

Please let me know what you think of this alternative solution.

vsivsi commented 8 years ago

@oceddi Any thoughts on my comments above?

oceddi commented 8 years ago

Your suggested solution fixes the "bug" but weakens the security of the connection. I understand that the Meteor SSL connection has a similar weakness which you are aligning with (and which I hope they address soon).

oceddi commented 7 years ago

So it seems Meteor has added support for user defined Mongo connection options:

https://github.com/meteor/meteor/pull/7277 https://github.com/meteor/meteor/blob/devel/packages/mongo/connection_options.js

They are offering an API call that sets the connection options object. That object is then passed along to the Mongo driver with very little validation by meteor (they check if its an object). This approach is similar to what I had originally proposed in my PR.

Not securing communications between Meteor and mongo with mutually authenticated SSL seems like a bad long term approach if you care about the sensitivity of your file data. Perhaps a compromise where we can set the 'sslCA' parameter as the only pass through option?

vsivsi commented 7 years ago

Hi, in the medium-term, I will be removing the separate file-collection mongodb driver/connection. That was only required in the first place because Meteor was using an antique version of the mongodb driver and it didn't properly support gridFS. Now that Meteor uses a 2.x series driver, file-collection can drop maintaining its own connection and just piggyback on the Meteor connection to the same DB/Collection. That should make whatever solution Meteor has devised for the issue raised in this PR equally applicable to file-collection.

I say "in the medium-term" because I probably won't be able to get to this work for several months (early July at the soonest).

juliomac commented 3 years ago

I am having issues with gridFS and SSL while trying to mo make a mandatory move from Mlab to Mongo Atlas.

I do not understand how to use this: @db = Meteor.wrapAsync(mongodb.MongoClient.connect)(process.env.MONGO_URL,{ sslValidate: false })

What does @db mean here?

I have seen a fork from @sakeena12 and/or @brucejo75 https://github.com/sakeena12/meteor-file-collection that seems to implement a way around, but not documentation around it is available nor a way to contact them.

Does anyone have succeed with it?