Skip to content
This repository has been archived by the owner on Jan 1, 2024. It is now read-only.

/var/tmp/ruby-uuid assumed to be writable #5

Closed
stouset opened this issue Feb 11, 2010 · 21 comments
Closed

/var/tmp/ruby-uuid assumed to be writable #5

stouset opened this issue Feb 11, 2010 · 21 comments

Comments

@stouset
Copy link

stouset commented Feb 11, 2010

Deploying the app to Heroku, and /var/tmp/ruby-uuid isn't writable because somebody else already is running it on their user account.

uuid checks to see if the /var/tmp directory is writable, but it never does a similar sanity check to ensure the file itself is writable.

@assaf
Copy link
Owner

assaf commented Feb 16, 2010

Do you have a patch that will work on Heroku?

Separately, sharing /var/tmp directory between apps is dumb. It's a source for bugs and security leaks. Might want to open an issue and ask them to change to per-instance temp directory.

If you're running OS X, you can echo $TMPDIR. You'll notice the temporary directory is unique to your account and changes on each boot. That's how it should work on any shared system.

@assaf
Copy link
Owner

assaf commented Feb 16, 2010

That /var/tmp/ruby-uuid might be owned by Heroku (i.e. no one gets to write to it)
http://docs.heroku.com/constraints#read-only-filesystem

Can you look at what TMPDIR/TMP/TEMP point at? Hopefully one of these would point to a useable temp directory that UUID can use.

@stouset
Copy link
Author

stouset commented Feb 16, 2010

That file could be owned by Heroku, but regardless the code should be written to be more fault-tolerant. It's getting past the check that the directory is writable, but it needs to check to see if the file already exists and is writable.

If those fail, the current approach of a dotfile in $HOMEDIR should work.

Of additional concern for an environment like this is locking/contention on the file. Is the code written to handle multiple processes writing state to the file? If not, another possible solution would be to include the uid and/or pid in the filename. That would both prevent contention of the state file and reduce the likelihood of encountering an existing file with no write access in the first place.

@assaf
Copy link
Owner

assaf commented Feb 16, 2010

Updates to the state file use exclusive lock, so only one process can update it at a time. The state file's sole purpose is to maintain state across server restarts: you want one per server, not one per process.

@stouset
Copy link
Author

stouset commented Feb 16, 2010

That makes sense. I imagine the best solution then would be to write it into $HOME if the one in a tempdir isn't writable. I don't think the root cause behind the directory is writable, just the file isn't.

But going to a different tempdir won't work, because if someone else creates the file it'll be the same SNAFU.

@stouset
Copy link
Author

stouset commented Feb 16, 2010

Hopefully that last comment made sense. I appear to have been incapable of stringing together a coherent sentence for the duration of the post.

@stouset
Copy link
Author

stouset commented Feb 17, 2010

So is there an issue with the solution as first described? Check for directory and file writability before attempting to write to /var/tmp/ruby-uuid, with the current $HOME fallback?

@assaf
Copy link
Owner

assaf commented Feb 17, 2010

  1. If you're running on Heroku, then you shouldn't be writing to /var/tmp at all, irrespective of what the directory contains. Even if there was no uuid.state file.

  2. The solution cannot depend on looking at the file permission. In the general case, if the file is not writeable, that's an error that should not be silently ignored.

@stouset
Copy link
Author

stouset commented Feb 17, 2010

But UUID doesn't currently give me any options here. It attempts to write in /var/tmp, but if the directory isn't writable it falls back to $HOME. If the file isn't writable even if the directory is, it ought to still use the same fall-back mechanism.

I agree if UUID can't find any file to write to it shouldn't be ignored, but if one particular file is owned by another user, it should try the others.

I'd be glad to write somewhere other than /var/tmp, but I don't think there's any way to override this in the gem as it's written currently. What are my options here?

@assaf
Copy link
Owner

assaf commented Feb 17, 2010

The uuid.state file is a shared resource. All processes have to agree on the location of the file. A fallback should take that into account (i.e. have them all fall back on the same page).

If one process has opened the file with the wrong permission (either preventing others from, or itself not able to access it), that's a configuration issue.

For Heroku, UUID needs to write some other place then /var/tmp, regardless of what's in that directory, which is why I asked if you can find out where TMPDIR/TMP/TEMP point at?

@stouset
Copy link
Author

stouset commented Feb 17, 2010

As far as I can tell, none of those environment variables are set inside the Ruby environment provided by Heroku. Is it really disastrous (as opposed to non-ideal) if multiple processes owned by separate users use separate statefiles? It seems like making it writable by any user could open up security implications (i.e., untrusted users toying with the sequence number, or even someone maliciously opening a lock and not releasing it).

@stouset
Copy link
Author

stouset commented Feb 17, 2010

ENV.keys
=> ["LM_GROUP_NAME", "INLINEDIR", "APP_NAME", "MEM_LIMIT", "RACK_ENV", "HEROKU_UPID", "PATH", "_", "PWD", "LM_CONTAINER_NAME", "REQUESTED_STACK", "MERB_ENV", "STACK", "URL", "HOME", "SHLVL", "COMMIT_HASH", "HEROKU_SLUG", "RAILS_ENV", "GEM_PATH", "SCHEMA", "LAST_GIT_BY", "HEROKU_TYPE", "DATABASE_URL", "HEROKU_PORT", "RUBYOPT"]

@assaf
Copy link
Owner

assaf commented Feb 17, 2010

Processes share the state file so they can share the same system clock and MAC address, but pick up consecutive sequence numbers. That helps them avoid collisions. Typically a server has one user account that all processes run under.

@stouset
Copy link
Author

stouset commented Feb 17, 2010

I don't mean to sound argumentative, but that's only typical from the perspective of, say, Ruby webapps. In more widespread practice, servers run separate processes under multiple accounts (e.g., Postfix => postfix, Apache => www-data, MySQL => mysql, etc.).

I believe UUID should be able to support those kinds of use-cases (not to mention in a shared environment like Heroku). And I'm concerned that having multiple users share a statefile opens up a whole host of potential security issues that at least need to be considered.

Would you be open to having a configurable location for the statefile? If so, what about for both instance-based (e.g., UUID.new.generate) and class-based (UUID.generate) uses of the gem?

I think even ignoring the case of trusted background-daemons (where you could arguably chmod 666 the file), shared hosting opens a whole host of security issues that need to be considered. I wouldn't want another user to be able to DOS my server process by refusing to unlock the file, which points towards having an option that doesn't use shared access.

@assaf
Copy link
Owner

assaf commented Feb 17, 2010

This is a Ruby library, so I'm only concerned with Ruby processes. A shared state file helps to avoid collisions, that's generally a Good Thing. Having multiple state files all over your computer doesn't help the UUID generator do its job better.

From what I gathered so far (reading the Heroku documentation), you're going to get one state file per process, and it's not going to persist across restarts. So essentially, the state file does nothing to prevent collisions under Heroku.

I which case the better option is having a way to disable it.

@stouset
Copy link
Author

stouset commented Feb 18, 2010

I'm alright with that.

For the record, though, I was opening up the idea that Ruby can be and is used for system background daemons, and they could conceivably rely on the UUID library. I could have picked some better examples, like actual Ruby daemons.

@assaf
Copy link
Owner

assaf commented Feb 18, 2010

Can you be more specific? I run all my apps as daemons (Unicorn processes behind Nginx, Resque processes, etc). Not seeing any fundamental problem.

Pushing out UUID 2.2.0. You can disable the state file on environments that don't support it, using:

UUID.state_file = false

@stouset
Copy link
Author

stouset commented Feb 18, 2010

Thanks for the fix!

I simply meant that it might be a bad assumption that all Ruby processes on a box will be run by the same user. It's generally considered bad system administration practice, and exposes unrelated services to risk if one service has a vulnerability.

I was just trying to make an analogy to system daemons like MySQL, Apache, BIND, Postfix, and countless others which all default to running as separate unprivileged users. A long time ago system processes used to run as nobody:nogroup, but that practice has largely been phased out in favor of the more secure separate users approach.

Ruby could conceivably be used for such unprivileged system daemons, and making the assumption that all Ruby processes on a machine share one user prevents UUID from being used in those kinds of applications.

For what it's worth, being able to disable the state file helps a lot in that regard, so I suppose the point is moot.

@assaf
Copy link
Owner

assaf commented Feb 19, 2010

You're welcome.

@stouset
Copy link
Author

stouset commented Feb 19, 2010

Apologies if I came across at all as being argumentative. Just saw a use-case that prohibited a shared state-file, and was trying to get that case across.

@assaf
Copy link
Owner

assaf commented Feb 20, 2010

Not at all. I like strong opinions, enjoyed the back & forth. Apologize if my short and sporadic responses came as too strong. Just really busy.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants