{{url}} is bypassing the security checks in the download allow/deny function

[tanis2000]

I have the following allow/deny rules for a collectionFS:

MessagesFS.allow({
  insert: function (userId, file) {
    return userId && file.owner === userId;
  },
  update: function (userId, file, fields, modifier) {
    return userId && file.owner === userId;
  },
  remove: function (userId, file) {
    return userId && file.owner === userId;
  },
  download: function (userId, file) {
    return userId && file.owner === userId;
  }
});

In my template I use the {{url}} helper to get an URL to the file and it works fine for the currently logged in user.

But if I copy that URL and paste it in another browser with no user signed in, I can still access the file.

Is there a way to block this behavior?

[raix]

Actually the user is bypassing by passing on the extracted authenticated url... One way is to create a token that expires a minute after its created by {{url}} much like s3 is using signed urls. The url contains the logged in user auth token - mind that meteor can revoke the auth token.

We have been investigating setting the x-auth header - there is already a branch on this in the http-methods.

We would like to use web sockets instead and have an authenticated line - much like ddp just for data. We are currently discussing this as client-side streams - it would be an improvement in both speed, security etc.

1.4.0 https://github.com/CollectionFS/Meteor-CollectionFS/issues?milestone=8&state=open

btw. you could use the ddp upload/download for data - its found as packages - this is very slow and not optimized for anything - we had to cut it off due to performance - but its safer?

[tanis2000]

@raix for what I need right now, a token that has an expiration time set to one minute would be a good solution.

So, as I understand it, right now you're appending the user auth token instead of creating one just for that URL. I guess that generating a new token is pretty much just a matter of adding a new metadata to that object with a Random.id(). But then how could I hook to the URL generation and access so that I can add that token to the URL and check that it's not expired when accessed?

Thanks!

[raix]

The http-methods allow a mount point to have its own way of resolving the user id.

server The cfs-access-point defines the mount point in access-point-server.js ln 108-198 add auth here. https://github.com/CollectionFS/Meteor-http-methods#using-custom-authentication

client In access-point-common.js ln 105 we currently set the loginToken directly

1. Create an expiration token on the client

authToken = btoa(JSON.stringify({
  authToken: Accounts._storedLoginToken() || '',
  expiration: Date.now + 60000 // We actually need the server time
}));

1. Check this on the server

  // My auth will return the userId
  var expirationAuth = function() {
    // Read the token from '/hello?token=base64'
    var encodedToken = self.query.token;
    // Check the userToken before adding it to the db query
    // Set the this.userId
    if (encodedToken) {

      var tokenString = atob(encodedToken);

      try {
        var tokenObject = JSON.parse(tokenString);

        // XXX: Do some check here of the object
        var userToken = tokenObject.authToken;

       // check if its too old
       if (tokenObject.expiration < Date.now()) {
         throw new Meteor.Error(500, 'Expired token');
       }

        // We are not on a secure line - so we have to look up the user...
        var user = Meteor.users.findOne({ 'services.resume.loginTokens.token': userToken });

        // Set the userId in the scope
        return user && user._id;

      } catch(err) {
        if (err instance of Meteor.Error) {
          // Pass on error
          throw err;
        } else {
         // Not formatted correctly
         throw new Meteor.Error(500, 'Internal error or something...');
        }

      }

    }  
  };

  // Add "auth: expirationAuth," to the ln 108

The code is not tested at all - I just wrote it here - but we'll take a pr if you want to give it a try,

The only thing here is to have an elegant way of setting the expiration time correctly, estimating the servertime - again - any body could modify this and gain access, but it would be a lot harder.

[tanis2000]

I've been toying a bit with this. If I add the auth to the FS.HTTP.mount function in access-point-server.js, I end up having that same auth method for PUT and POST as well, so if I try to upload a file it fails as the PUT query contains a token built the old way. Where is that token being built? Is there any other place you are aware of that would need to be changed? Thanks as usual :)

[aldeed]

You're probably looking for this bit of code in the cfs-upload-http package.

[tanis2000]

@aldeed yeah! Thanks! Do you reckon there might be others in different modules?

[tanis2000]

It's interesting that Accounts._storedLoginToken() differs from the content of the field services.resume.loginTokens.token in the Meteor.users collection..

[aldeed]

I think the difference has to do with the token hashing introduced a couple Meteor releases back? Not sure.

I don't think there are any other places where we use the PUT URL. Use of the GET URL should be confined to FS.File.prototype.url function. We don't use the DEL URL anywhere since we do deletion through DDP.

[tanis2000]

Thanks @aldeed.

I wonder if it would be the same to just use the Meteor.userId() to lookup the user in the db. Given that the token expires after one minute, I might as well just use the _id instead of a volatile login token as it would really make no difference unless I'm forgetting about something.

That way it's one less "private" Meteor method to call as well and it won't break if there's any change to the way the token is hashed in the future.

[aldeed]

I'm not really sure. @raix is more familiar with the token code.

[tanis2000]

Ok, I've got some code working but I'll wait for @raix to jump in the conversation to make sure I'm not sending over some crap code :)

[raix]

@tanis2000 yeah, the token is a bit off since its hashed in newer versions of Meteor, I think its best to use the token only since its revokable and id is not (in case of tampering)

The current http-methods getUserId actually looks like:

// This method retrieves the userId from the token and makes sure that the token
// is valid and not expired
_methodHTTP.getUserId = function() {
  var self = this;

  // // Get ip, x-forwarded-for can be comma seperated ips where the first is the
  // // client ip
  // var ip = self.req.headers['x-forwarded-for'] &&
  //         // Return the first item in ip list
  //         self.req.headers['x-forwarded-for'].split(',')[0] ||
  //         // or return the remoteAddress
  //         self.req.connection.remoteAddress;

  // Check authentication
  var userToken = self.query.token;

  // Check if we are handed strings
  try {
    userToken && check(userToken, String);
  } catch(err) {
    throw new Meteor.Error(404, 'Error user token and id not of type strings, Error: ' + (err.stack || err.message));
  }

  // Set the this.userId
  if (userToken) {
    // Look up user to check if user exists and is loggedin via token
    var user = Meteor.users.findOne({
        $or: [
          {'services.resume.loginTokens.hashedToken': Accounts._hashLoginToken(userToken)},
          {'services.resume.loginTokens.token': userToken}
        ]
      });
    // TODO: check 'services.resume.loginTokens.when' to have the token expire

    // Set the userId in the scope
    return user && user._id;
  }

  return null;
};

[tanis2000]

@raix thanks for the code, I'll go the same route, that's pretty easy. There are only two concerns now: 1) the date/time on the client: it's pretty much impossible to guess the time of the server unless it's the server that sends it's datetime to the client. 2) the btoa() function in access-point-common.js: it's only available in the browser. If it's called from the server, it doesn't exist and a Buffer() should be used. I'm already doing that on the server side code for atob() but I suppose I should check if btoa() is available in the common lib and use that or Buffer() accordingly.

[raix]

Buffer is fine - Buffer('foo').toString('base64');

[raix]

The server time - could perhaps sync this on startup + store it in local storage. I did some code in GroundDB ref: https://github.com/mizzao/meteor-timesync/issues/1#issuecomment-38024106

[tanis2000]

I've got this working but there's one more thing that we didn't take into consideration. It would be nice to let the programmer decide wether to have the link expire or not. In my case it's good to have an expiration on that link.. but just imagine a typical case history, where the user leaves a page open and that page has a link to a file to download. After one minute, that link won't be valid anymore.

Have you got any suggestion about how to handle that case? Thanks!

[aldeed]

The url function already accepts an auth option that is null by default. You could add support for setting it to a Number:

• When null, don't expire, same as current code.
• When false, don't add any auth token to the URL, same as current code.
• When typeof is "string", use that as the auth token, same as current code.
• When typeof is "number", use an auth token that expires in that many minutes (or seconds?)

[raix]

I'm just thinking here having second thoughts on the expire part - sorry,

If we set a dep auto run on userid and run a HEAD put to login - we could perhaps have it simply set the x-auth? still using base64 wrapped json.

I did add support for basic auth login in basic-auth-playground branch on http-methods (don't have the streaming support but could be merged to master) - it will keep the the token in the whole session.

This would be a general secure solution (if on https)?

I think all changes are basically present in the http-methods branch, I'll just try and merge the two into a new branch - moment...

[tanis2000]

@raix this is getting way too complicate for me, I'm kind of getting lost but I'll happily have a look at the code.

If you guys want to have a look at what I'm playing with, you can head over to: https://github.com/tanis2000/Meteor-cfs-upload-http and https://github.com/tanis2000/Meteor-cfs-access-point

It's pretty much all there without the last suggestions from @aldeed

[raix]

@tanis2000 just did a diff, your code looks good - maybe we should do as @aldeed suggests for now eg. if (auth === +auth) then set expiration - yep its a bit complicated in the http-methods - I've just merged the auth playground and master into auth-stream branch - if somebody feels like playing with it.

Both patterns are ok by me - maybe have the server time as a http get? (this filters some of the EJSON + ddp overhead... :)

simply replace Meteor.methods with HTTP.methods - and make a HTTP.get to the mount point maybe mount at /cfs/servertime ?

[raix]

Note: I like the base64 pattern of token better - maybe we should have an expire option in url, if we set it to 0 the server would allow this as long as the token is valid?

[tanis2000]

For the time being I've implemented the solution suggested by @aldeed. I might send a PR if you feel brave enough to give this a try :)

[aldeed]

PR sounds fine to me, but I defer to @raix since he knows more about this area of the code.

[tanis2000]

I've submitted the two PR.

[adamgins]

HI @tanis2000 @aldeed @raix how goes? Just wondering did this actually get implemented... ie is there a way I can expire the token easily or a way to stop/limit folk coping and pasting the URL?

I do understand that in theory, this is really just a deterrent for folk as it's the person with access who may send a link to some info and there is really nothing stopping them downloading the doc and giving it to another unauthorized user.

[tanis2000]

Hey @adamgins, this got implemented in a way that lets you set an expire token. I useitto provide links that expire after one minute as they contain sensitive information but the data is actually being provided theough the s3 adapter and to anonymous users who got the link through email. The link I'm sending is set to transfer the user to a landing page where I can do a couple of checks and afterwards I give them the actual link to the file with the expiration time set.

If you need a sample I'll dig it up as soon as I get in front of a desktop. Cheers!

[adamgins]

@tanis2000 thanks. Yep a sample would be great.

As I mentioned, I am not sure it solves the sensitive info issue... ie if someone downloads the file and forwards the link the info is still out there... also if they forward/tweet the link and it's clicked within a minute, if I am understanding correctly they'd get access, right? Or do the other checks prevent this? Just wondering if there's a way to share/force login on the link so only the user gets access to the s3 file?

[tanis2000]

@adamgins no, this solution doesn't cover security as the link will be good for everyone until it expires.

What I'm adding here is the expiration property of the token. But you can instead set the authToken property to that same token and set it to the loginToken of the user that should have access to that file.

If you do it that way, CFS automatically checks that the user can actually access the file. You can learn more by having a look at the code there: https://github.com/CollectionFS/Meteor-CollectionFS/blob/master/packages/access-point/access-point-server.js#L313

Adding a link with the expiration token is as easy as adding this to your template:

<a href="{{url auth=60}}" class="btn btn-primary btn-mini">{{name}}</a>

If you set auth to a string and actually set it to your loginToken, you get what I was referring to above.

Again, you can learn more about it by having a look at the code there: https://github.com/CollectionFS/Meteor-CollectionFS/blob/master/packages/access-point/access-point-common.js#L115