Cannot set permissions with file resource to quoted string

Chef Infra (archive)
1

Hello

I'd need to remove the setuid bit from certain files (as that was the finding of some security audit). On the command line, I'd simply do a "chmod u-s $file", as that would be the least interfering way and would do exactly, what I would need to do: "Remove the setuid bit from a file".

How can I do that in a recipe? I tried:

%w[/usr/lib/pt_chown /usr/lib/eject/dmcrypt-get-device /usr/bin/chfn].each do |setuidfile|
    file "Setuid Berechtigung anpassen: " + setuidfile do
        path setuidfile
        mode "u-s"
    end # of file "Setuid Berechtigung anpassen: " + setuidfile do
end # of %w[/usr/lib/pt_chown /usr/lib/eject/dmcrypt-get-device /usr/bin/chfn].each do |setuidfile|

But this fails:

================================================================================
Recipe Compile Error in /opt/kitchen/cookbooks/ew-hardening/recipes/default.rb
================================================================================


ArgumentError
-------------
invalid value for Integer(): "0u-s"


Cookbook Trace:
---------------
  /opt/kitchen/cookbooks/ew-hardening/recipes/default.rb:141:in `block (2 levels) in from_file'
  /opt/kitchen/cookbooks/ew-hardening/recipes/default.rb:139:in `block in from_file'
  /opt/kitchen/cookbooks/ew-hardening/recipes/default.rb:138:in `each'
  /opt/kitchen/cookbooks/ew-hardening/recipes/default.rb:138:in `from_file'


Relevant File Content:
----------------------
/opt/kitchen/cookbooks/ew-hardening/recipes/default.rb:

134:  #############################################
135:  # Audit
136:  # Setuid Dateien anpassen
137:  # https://wiki.ubuntu.com/Security/Investigation/Setuid
138:  %w[/usr/lib/pt_chown /usr/lib/eject/dmcrypt-get-device /usr/bin/chfn].each do |setuidfile|
139:      file "Setuid Berechtigung anpassen: " + setuidfile do
140:          path setuidfile
141>>         mode "u-s"
142:      end # of file "Setuid Berechtigung anpassen: " + setuidfile do
143:  end # of %w[/usr/lib/pt_chown /usr/lib/eject/dmcrypt-get-device /usr/bin/chfn].each do |setuidfile|
144:  
145:  # EOF
146:  


Chef Client failed. 0 resources updated

But according to the docs at "mode", it should be possible to use a string, I'd think:

mode
Ruby Types: Integer, String
…
UNIX- and Linux-based systems: A quoted 3-5 character string that defines the octal mode that is passed to chmod. For example: '755', '0755', or '00755'. If the value is specified as a quoted string, it works exactly as if the chmod command was passed.

Í am passing this as a quoted string (I'd think…). But what am I doing wrong?

I'm on Chef 11.8.2.

Thanks a lot,
Alexander

2

You probably need to translate that "u-s" into an actual mode, e.g. '0755'. Yes, "u-s" is a string but it doesn’t contain something that Chef is able to process.
https://docs.chef.io/ruby.html#file-modes (note these are guidelines, not requirements, but you might as well adopt them)

3

Hello

[quote="Shubby, post:2, topic:7259, full:true"]
You probably need to translate that "u-s" into an actual mode, e.g. '0755'. Yes, "u-s" is a string but it doesn't contain something that Chef is able to process.[/quote]

"u-s" cannot be translated into an actual mode. An actual mode also modifies the group and world permissions, whereas something like "u-s" does not. If I'm giving chef a valid string, why does it try to make an octal number out of it by prefixing a "0" to it? That's broken.

That's using chmod in a different way. It's kinda like trying to open a can with a sledge hammer, whereas I'd rather use a can opener to open a can. :wink:

Oh, well, guess that an "execute" resource is as good as it will get with Chef.

%w[/usr/lib/pt_chown /usr/lib/eject/dmcrypt-get-device /usr/bin/chfn].each do |setuidfile|
    execute "Setuid Berechtigung anpassen: " + setuidfile do
        command 'chmod u-s "' + setuidfile + '"'
        only_if {::File.stat(setuidfile).setuid?}
    end # of execute "Setuid Berechtigung anpassen: " + setuidfile do
end # of %w[/usr/lib/pt_chown /usr/lib/eject/dmcrypt-get-device /usr/bin/chfn].each do |setuidfile|

Feels a bit clumsy, though. I hate having to invoke shell tools.

Regards
Alexander

4

"u-s" cannot be translated into an actual mode. An actual mode also modifies the group and world permissions, whereas something like "u-s" does not.

What I meant by that is you should be able to set a mode that does not change the current group and world permissions because what you'd be setting is equal to what's already in place.

I think you have a misunderstanding around what a string is vs. what Chef expects. Just because the Chef docs tell you that the mode setting expects a string doesn't mean that it must accept the string you are passing. Like I said, yes, 'u-s' is a string, but so is '775' (as opposed to 775), but Chef is built to parse only the latter. That's not broken, that's by design. 'u-s' is not idempotent, which is against Chef's design philosophy.

5

I think you have a misconception around what a string is vs. what Chef expects. Just because the Chef docs tell you that the mode setting expects a string doesn't mean that it must accept the string you are passing.
[/quote]

Then the docs are broken. It's not mentioned there, that valid mode letters (like "u-s", or "ug=rwx,o=") are not supported. If a valid string is given, it should be accepted. "u-s" is a valid string. The docs say, that strings are supported, but valid chmod strings aren't supported. As I quoted already, the docs say this: "If the value is specified as a quoted string, it works exactly as if the chmod command was passed." That is not what is happening.

Okay, so it's broken by design. That makes it not better, I'd think.

[quote] 'u-s' is not idempotent, which is against Chef's design philosophy.
[/quote]

How's 'u-s' not idempotent, but eg. 0644 is? Both would remove the setuid bit from a file, if the perm used to be "u+s".

Regards,
Alexander

6

Chef is expecting a valid octal mode, passed as an integer or a string, that's all. It would also be unable to parse something more mundane like +x because that's not an octal mode. Perhaps the docs could be clearer about what is considered a valid string -- it sounds like you're assuming that anything enclosed in quotes is a valid string, but really you're trying to fit a square peg into a round hole.

Imagine you have two servers and both have this file, /tmp/foo.txt.
On server A, its permissions are 775.
On server B, they are 755.
Now, you run this Chef recipe on both nodes with the following code:

file '/tmp/foo.txt' do
  mode 'g-x'
end

Ignore the fact that this wouldn't work to begin with, it's just an example. At the end, the file on server A would have mode = 765, and the same file on server B would have mode = 745. That's what's not idempotent. You're supposed to end up with the same end state, every time, and only by specifying the octal mode can you get that result reliably, which is why I'm assuming that specifying the mode the way you're trying to specify it is not supported.

7

Quoting the doc, emphasis is mine:

UNIX- and Linux-based systems: A quoted 3-5 character string that defines the octal mode that is passed to chmod. For example: '755', '0755', or '00755'. If the value is specified as a quoted string, it works exactly as if the chmod command was passed. If the value is specified as an integer, prepend a zero (0) to the value to ensure that it is interpreted as an octal number. For example, to assign read, write, and execute rights for all users, use '0777' or '777'; for the same rights, plus the sticky bit, use '01777' or '1777'.

Reading only part of the doc won't help you. Both If goes together and are there to extend on the previous part saying it has to be a 3 to 5 digits string.

Relative mode changes on files are inherently unpredictable and does not describe a state, so obviously Chef do not allow it as you can't tell if the state of the permission is correct or not at the end of your action (Ok, you removed the setuid, so it should probably be executable, but by who ? only owner ? group too ? others ? Too much indetermination there.

When you use a configuration management system it is to ensure your system ends up in a predictable state, if you don't want to describe precisely your system state, Chef is probably not the right tool for you.

8

[quote="Shubby, post:6, topic:7259, full:true"]
Chef is expecting a valid octal mode, passed as an integer or a string, that's all. [/quote]

Okay. But that is not what the docs say. The docs say, that it accepts a string, just like if chmod were run.

On the contrary. Chef is doing that. I am trying to interfere with the environment as little as possible. Whereas Chef is doing the opposite - it's always using a sledge hammer to open a can.

I'm not assuming that "anything" enclosed in quotes is a valid string. "foo" would not be. But if it's a string, Chef should assume that it is valid. If not, the user will notice anyway. Currently, Chef is trying to be too clever and is failing to do so. It is interpreting the string as a number and fails.

Nope. That's just not the case, as I've shown. Even with "chmod 0775", you will only end up with identical results, if other preconditions are met. It's simply that the number of preconditions differ with "chmod 0775" vs. "chmod u-s".

Alexander

9
  • Got to use Chef.
  • I want describe precisely my system state.
  • Chef doesn't let me do this (if we ignore execute blocks)

Regards,
Alexander

10

No, the doc say that YOU have to prepend it with a 0 if you write it as an integer and not a quoted string, so it will be an octal number...

What should have be the point of a file being suid if it is not executable ?

It does not enforce a state, the permission on the file could be everything another process/user set them to, you just ensure the file is not suid, you're not sure it has the permission you assume would be after just removing the suid bit, which is not a precise description of the file state...

11

[quote="Tensibai, post:10, topic:7259, full:true"]

No, the doc say that YOU have to prepend it with a 0 if you write it as an integer and not a quoted string, so it will be an octal number...[/quote]

No, the doc does not say, that the user has to prepend a 0. It gives examples, where the 0 is left out. Eg.: "For example, to assign read, write, and execute rights for all users, use '0777' or '777';"

However, the doc does NOT say, that Chef prepends a 0 to a string. But it does say this: "If the value is specified as a quoted string, it works exactly as if the chmod command was passed. ". "chmod u-s" works. But mode "u-s" does not. How is "does not work" exactly the same as "does work"?

Yep, if you do a "chmod 0644 /tmp/foo/bar", you're also not sure what happens. For example, what happens, if /tmp/foo/bar isn't there? Or what happens, if it's set to "strange" rights (like in my /tmp/dir example above)?

If "chmod u-s" isn't idempotent, then "chmod 0644" also isn't. Because in both cases, it's easy to make up examples, which support the case. One just has to leave out bits of data, that support the own case.

"chmod u-s file" is a very precise description of the file state. It does not have the setuid bit set anymore. Cannot be much more precise.

Octals, on the other hand, are too broad. They describe things, which I don't care about (at this point).

12

The docs clearly do say that the user has to prepend a 0 if you're passing the value as an integer, and if using a string instead, that it must be three to five digits, e.g. '777', '0777', or '00777'. It is neither mentioned nor implied that a relative statement like +x is acceptable. It does not matter whether or not Chef prepends a zero to a string, because you're not supposed to pass something like +x or u-s, because it's not supported.

The first line of the same paragraph clearly states what the mode setting expects.
It must be a mode. That prevails over everything else. It cannot be something else. If it's a string, it can be 3, 4 or 5 digits and is passed to chmod as-is. Or apparently, it may be prepending a zero to your string behind the scenes for some reason, but it doesn't matter since u-s wouldn't be supported here whether Chef prepends the zero or not. It doesn't say that Chef passes that value to chmod as is, only that it works the same way as if you'd passed it to chmod, provided that it fits within the first and main parameter of the setting, which is that it requires an absolute mode made of 3 to 5 digits. Again, that's not broken, nor is it broken by design, it's just by design.

Actually it's quite certain what would happen if the file does not exist: an empty file would be created with mode 644.

13

https://tickets.opscode.com/browse/CHEF-1857

I cut that ticket back in Nov 2010, its just not easy to implement due to changes needed deep in the bowels of Chef::ScanAccessControl and Chef::FileAccessControl, which are scary bits of code to touch.

14

The first line of the same paragraph clearly states what the mode setting expects.
It must be a mode.[/quote]

"u+s" or "ugo=rwx" are modes. Those mode specifications aren't supported.

[quote][quote="Alexander_Skwar, post:11, topic:7259"]
Yep, if you do a "chmod 0644 /tmp/foo/bar", you're also not sure what happens. For example, what happens, if /tmp/foo/bar isn't there?
[/quote]

Actually it's quite certain what would happen if the file does not exist: an empty file would be created with mode 644.
[/quote]

But not, if the directory /tmp/foo doesn't exist.

[2015-11-04T08:19:30+01:00] ERROR: file[/tmp/foo/bar] (ew::test line 10) had an error: Chef::Exceptions::EnclosingDirectoryDoesNotExist: Parent directory /tmp/foo does not exist.
[2015-11-04T08:19:30+01:00] FATAL: Chef::Exceptions::ChildConvergeError: Chef run process exited unsuccessfully (exit code 1)

So, no, it's not quite certain what would happen.

15

I suppose the problem boils down to the fact, that the Ruby File.chmod function does only support "mode_int". And thus, somebody would need to interprete the string passed. Up to now, maybe nobody has done so, it seems.

I haven't looked at it, but maybe it might be doable to copy the parsing from Gnu chmod?

Cheers
Alexander

16

What you're asking for go against a predictable state of the file at end as any external mode change out of the suid bit won't be checked nor enforced.

This is an as bad pattern as modifying files instead of managing its entire content in my opinion, maybe explaining why it has never be done too.

17

[quote="Tensibai, post:16, topic:7259, full:true"]

What you're asking for go against a predictable state of the file at end as any external mode change out of the suid bit won't be checked nor enforced. [/quote]

How is "chmod u-s $file" not specifying a predictable state? Afterwards, the setuid bit is gone.

If I only look at the setuid bit, I know how the state is afterwards: Gone.

Or if we'd talk about "chmod +x $file", we also know the state afterwards: The "x" bit for user, group and others is set. Sure, we do not know how the r and/or w bits are set. But we don't need to. Even with a "chmod 0700 /tmp/foo/bar", we do not know if the user as able to read-write-execute the file, because we do not know the permissions of the /tmp/foo directory.

So, in that respect, chmod 0700 $file also doesn't describe a predictable state regarding the accessability of the file.

Because of?

Regards
Alexander

18

This is just part of the state, not the state in my point of view.

I disagreee

I don't see how this is relevant to the start of the talk about string mode in file/template resources. (but I agree the parent directory should be managed too with mode enforced)

You won't convince me it's a correct practice to not manage the whole permission set, it get opens to too much DoS, errors, compromission or any other problem caused by out of band operation on this file.

I've said what I think about it, I'll stop there before we end up on a GodWin point.

19

[quote="Tensibai, post:18, topic:7259, full:true"]

This is just part of the state, not the state in my point of view.[/quote]

This is the whole part of the state that is important here. With chmod 0644 $file, you're also just looking at parts of the "file accessibility state". You're happy ignoring the permissions of directories, for example. In that respect, chmod 0644 also is not good, I assume.

[quote]

I disagreee[/quote]

For the question "is setuid set?", the state of the r/w/x bits is of no matter.

[quote]

I don't see how this is relevant to the start of the talk about string mode in file/template resources. [/quote]

It is important, because someone brought up the point, that "chmod u-s" somehow would not describe the state of some parts of file access. That was not my point.

Hopefully this will never happen.

But you already do. The file resource does not manage the permissions of directories (in the case of "file "/tmp/foo/bar" …"). So already now, not the whole permission set is managed. And that's somehow fine? Because of… … it's always been like that? Is that the only reason? Mind you, I would not want this. But I'm not after Chef forcing the user to manage the "complete set of permissions" (whatever that might be…) of a file.

Regards
Alexander