Skip to content
This repository has been archived by the owner on Mar 19, 2022. It is now read-only.

Use knife[:solo_path] as secure-ish repo location on server #145

Closed
wants to merge 11 commits into from

Conversation

matschaffer
Copy link
Owner

No description provided.

@tmatilai
Copy link
Collaborator

Overall looks good but I didn't test it too much yet.

Using hardcoded /tmp paths still leave the system vulnerable to symlink attacks. But Dir.mktmpdir is not an easy solution as the path should be usable for consecutive cook runs. Maybe a directory in $HOME would be more secure default? At least we could warn the users.

Hmm, how about knife solo init having an option to set "solo_path" in the generated solo.rb? And use it as a variable in all *_path settings to make it easier to change at a later time?

@tmatilai
Copy link
Collaborator

Of course my intention is not to expand the scope of this PR. ;)

@matschaffer
Copy link
Owner Author

Hrm... I wonder if relative paths would resolve to somewhere same on all
OSes. I'll give it a try. If it works we can generate a solo.rb with paths
relative to FILE

On Friday, December 28, 2012, Teemu Matilainen wrote:

Of course my intention is not to expand the scope of this PR. ;)


Reply to this email directly or view it on GitHubhttps://github.com//pull/145#issuecomment-11744733.

-Mat

about.me/matschaffer

@tmatilai
Copy link
Collaborator

rsync should use paths relative to home if they are not absolute. For the Chef settings it is possible to use #{ENV['HOME']} or maybe even ~.

Or maybe I didn't understand what you meant. =)

Since this may be set that way in existing solo.rb configs
@matschaffer
Copy link
Owner Author

So I realized there's a bit of a problem with the $HOME idea. Unless we do special work to see that solo_path is indeed intended to be a home directory, the dirs could be different between the work station (e.g., /User/mat) and chef-solo (e.g., /home/ubuntu)

I think this is part of the reason I was wondering if solo_path should be a knife.rb config, but I'm not sure if I want to introduce a .chef/knife.rb to the generation just yet. Seems like it could be too many big changes in one release.

I'm thinking we should leave this as-is then try to switch from /tmp to $HOME in the next release. Thoughts?

@matschaffer
Copy link
Owner Author

FWIW, I tried both of these solo.rb configurations https://gist.github.com/4432046

The first one has the different path problem. The second one almost works but we need the workstation to know the cookbook path so it can inject the patches at the right place.

@tmatilai
Copy link
Collaborator

tmatilai commented Jan 2, 2013

Might be too early in the morning... But as far as I understand knife[:solo_path] is the only setting that we need to evaluate at the work station. Others are evaluated by chef-solo at the node.

So shouldn't it be sufficient to configure knife[:solo_path] and all the others can be calculated at the node? Like this. (But with a helper to extract the File.join thing). Disclaimer: I didn't test it yet. =)

@tmatilai
Copy link
Collaborator

tmatilai commented Jan 2, 2013

And I agree in the attempt to release often with fewer changes. Now we are anyway breaking backwards compatibility of solo.rb so it would be nice to avoid doing it again on the next release.

@matschaffer
Copy link
Owner Author

Hrm... I'll give the relative thing a try.

The workstation currently also needs the cookbook path so it knows where to
rsync the patches cookbook to.

On Wednesday, January 2, 2013, Teemu Matilainen wrote:

And I agree in the attempt to release often with fewer changes. Now we are
anyway breaking backwards compatibility of solo.rb so it would be nice to
avoid doing it again on the next release.


Reply to this email directly or view it on GitHubhttps://github.com//pull/145#issuecomment-11806035.

-Mat

about.me/matschaffer

@tmatilai
Copy link
Collaborator

tmatilai commented Jan 2, 2013

Ah, I missed the cookbook_path part. That complicates things.

Could this kind of trick work? https://gist.github.com/4434030/fdee4e181cde1a3eb1206e02e8210f551470085c

@matschaffer
Copy link
Owner Author

Sadly, it looks like chef expects fully qualified paths. My next best guess
would be to use ruby over ssh to ask the server what it's cookbook path is.
Which would of course be one more step in an already sluggish process :(

-Mat

about.me/matschaffer

On Wed, Jan 2, 2013 at 8:47 AM, Teemu Matilainen
notifications@git.luolix.topwrote:

Ah, I missed the cookbook_path part. That complicates things.

Could this kind of trick work?
https://gist.github.com/4434030/fdee4e181cde1a3eb1206e02e8210f551470085c


Reply to this email directly or view it on GitHubhttps://github.com//pull/145#issuecomment-11808364.

@tmatilai
Copy link
Collaborator

tmatilai commented Jan 2, 2013

Oh, I thought File.dirname(__FILE__) expands to an absolute path. If not for sure there is a library call for it?

Or is the "cookbook_path" on the work station used for other things as just uploading the patches with rsync? Or do you mean that Chef does not even want to read the solo.rb with relative paths on the work station?

Uh, I have to test it myself before suggesting half baked "solutions". =)

@matschaffer
Copy link
Owner Author

FILE is relative unless you do File.expand_path. Then we run into the
cookbook_path problem since it tries to rsync the patches to
/User/mat/etc.../cookbooks rather than /home/ubuntu, /root or whatever.

So one more thought here. Should we maybe just get opinionated and generate
a solo.rb at cook time that dictates the paths to be inside the home
directory? The Berkshelf integration is already assuming "cookbooks" fwiw.

That may avoid a required breaking change since we'll start just ignoring
any existing solo.rb files. The biggest catch I see there is if people are
using custom cookbook paths it would break.

Makes me wish we had a mailing list so I could get a feel for what people
are typically doing.

On Wed, Jan 2, 2013 at 11:09 AM, Teemu Matilainen
notifications@git.luolix.topwrote:

Oh, I thought File.dirname(FILE) expands to an absolute path. If not
for sure there is a library call for it?

Or is the "cookbook_path" on the work station used for other things as
just uploading the patches with rsync?


Reply to this email directly or view it on GitHubhttps://github.com//pull/145#issuecomment-11813151.

@tmatilai
Copy link
Collaborator

tmatilai commented Jan 2, 2013

But in my later gist version the __FILE__ part should only ever be used on the node. The idea is that e.g. in #load_configuration before loading the solo.rb on the work station ENV['KNIFE_SOLO'] is set to something.
According to my quick testing it even works with this! =)

@tmatilai
Copy link
Collaborator

tmatilai commented Jan 2, 2013

Take a look at my solo_path_test branch.

@tmatilai
Copy link
Collaborator

tmatilai commented Jan 2, 2013

RE: being opinionated. I don't think we can silently default to any path at the home directory as there is always a change to override something. I guess the only (quite) secure way to be compatible with the old solo.rb would be to use the file_cache_path for knife[:solo_path] and then remove file_cache_path setting totally on node (or otherwise reset it to Chef's default). And warn the user of course.

I guess we could use the chef mailing list but I'm not sure how many knife-solo users would be covered...

@matschaffer
Copy link
Owner Author

Oh hey, that ENV thing isn't a bad idea. I think having it might even help
out a use case that @deepak had trying to use the same solo.rb for both
local and remote chef-solo runs.

@deepak
Copy link
Contributor

deepak commented Jan 3, 2013

my patch was at #77

@arosenhagen
Copy link

get the following error with the current version (0.2.0_beta) from master

Exception: NoMethodError: undefined method `remote_mkdir_p' for #<Chef::Knife::SoloCook:0x007ff9598fa5e8>

@tmatilai
Copy link
Collaborator

@arosenhagen remote_mkdir_p has never existed in master, it is only introduced in this pull request. Maybe you have multiple knife-solo gems/versions that knife is erroneously loading? (See #152 and CHEF-3255.) Add a -VV switch to get more debug information. The stack trace should have a hint which classes are used.

Btw, this PR will most likely still change quite a bit as it seems that the best move is to remove solo.rb from the client side and use knife.rb for configuration. And then generate the solo.rb for the node.

@arosenhagen
Copy link

got it thanks. was indeed an issue with multiple gems installed. switched back to version 0.1.0 but looking forward to have the option to re-run my recipes (at the moment i'm testing with vagrant locally, so no problem).

@tmatilai
Copy link
Collaborator

@arosenhagen, maybe I don't understand what you mean, but you can re-provision the node by calling knife solo cook as many times you want. Or do you have some issues with v0.1.0 that are fixed in master or this PR?

@arosenhagen
Copy link

currently I can't re-provision the vm due to permission issues. First run is ok, but second one fails due to sudo commands in my recipes. I thought this is related to this PR since it seems as if knife solo can't clear the temporary paths correctly.

@tmatilai
Copy link
Collaborator

@arosenhagen Ah, OK. That definitely is the target of this PR.

@teohm
Copy link

teohm commented Jan 31, 2013

👍 I have tried this PR to run rbenv cookbook that uses file_cache_path, it's working well in subsequence runs.

@matschaffer
Copy link
Owner Author

Thanks! I'll try to get it merged this afternoon. Sadly it's not gonna be a
clean one any more. Boo for long-lived branches.

On Thursday, January 31, 2013, Huiming Teo wrote:

[image: 👍] I have tried this PR to run rbenv cookbook that uses
file_cache_path, it's working well in subsequence runs.


Reply to this email directly or view it on GitHubhttps://github.com//pull/145#issuecomment-12943758.

-Mat

about.me/matschaffer

@matschaffer
Copy link
Owner Author

Started work on this again in issue/avoid-cache-path-1

@matschaffer
Copy link
Owner Author

Closing this in favor of #197

@tknerr
Copy link

tknerr commented Feb 16, 2013

Hey guys, I tried to follow this but I'm still confused.

Are the paths in solo.rb the ones on the local workstation or where they get rsync'ed to on the node?

In 0.2.0 these seemed to be the paths where the parts of the kitchen got uploaded to on the server.

Actually, I'd prefer to specify the workstation-local paths to the cookbooks, data bags and data bag secret so knife-solo knows what to upload. I'm totally not interested in where it rsyncs this stuff to on the node, that's an implementation detail knife-solo should just handle internally.

Comparing to Vagrant, it's the same concept, i.e. in you specify the local paths where to find cookbooks etc:

Vagrant::Config.run do |config|
  config.vm.provision :chef_solo do |chef|
    chef.cookbooks_path = ["cookbooks"]
    chef.data_bags_path = "data_bags"
    chef.roles_path = "roles"
  end
end

If you want, you can also override the path where the stuff gets uploaded (or rather mounted via fs share) to on the node, but in practice I actually never used it (an implementation detail I don't care about):

Vagrant::Config.run do |config|
  config.vm.provision :chef_solo do |chef|
    chef.provisioning_path = "/tmp/vagrant-chef"
  end
end

@matschaffer
Copy link
Owner Author

Funny you should mention vagrant @tknerr. @tmatilai and I were chatting yesterday about basically porting the code Vagrant uses to do exactly this. Thanks for confirming that the server side paths are basically an implementation detail too.

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

Successfully merging this pull request may close these issues.

6 participants