834886 - RSBAC policy change review request for train-2013.01.18

Status: RESOLVED FIXED

(Security Assurance :: General, task)

Description

[Gene Wood [:gene]]

Guillaume,
  New functionality in the browserid app have caused a mismatch with the existing RSBAC policy. Please take a look at the attached policy patch and review it.

Here are the debug lines that I used to create the patch :

<7>0003161399|check_comp_rc(): pid 23228 (node), owner 450, rc_role 50005, FILE rc_type 50001, request MAP_EXEC -> NOT_GRANTED!
<6>0003161400|rsbac_adf_request(): request MAP_EXEC, pid 23228, ppid 7730, prog_name node, uid 450, target_type FILE, tid Device 08:03 Inode 8407900 Path /opt/browserid/node_modules/toobusy/build/Release/toobusy.node, attr prot_bits, value 5, result NOT_GRANTED (Softmode) by RC
<7>0003161401|check_comp_rc(): pid 23228 (node), owner 450, rc_role 50005, FILE rc_type 50001, request MAP_EXEC -> NOT_GRANTED!
<6>0003161402|rsbac_adf_request(): request MAP_EXEC, pid 23228, ppid 7730, prog_name node, uid 450, target_type FILE, tid Device 08:03 Inode 8407900 Path /opt/browserid/node_modules/toobusy/build/Release/toobusy.node, attr prot_bits, value 7, result NOT_GRANTED (Softmode) by RC

Comment 1

[Gene Wood [:gene]]

Attachment: Patch to browserid.custom.policy puppet file

Comment 2

[Gene Wood [:gene]]

Talked with Guillaume and one of these will look better :

If this is a rare instance of a c library in node_module then :

attr_set_file_dir -V 66565 FD "//opt/browserid/node_modules/toobusy/build/Release/toobusy.node" rc_type_fd 50003

if we're going to be doing this more generally then this :

attr_set_file_dir -V 66565 FD "//opt/browserid/node_modules" rc_type_fd 50003

Comment 3

[Gene Wood [:gene]]

I'll talk to the developer to find out if this is a one off or if we expect to see more of these to know which of the above policy changes we should implement.

Comment 4

[Gene Wood [:gene]]

Attachment: New patch which recategorizes node_modules as BrowserID-lib

This patch ignores the fact that previously "/opt/browserid/node_modules/jwcrypto/node_modules/bigint/build/Release/bigint.node" had enjoyed the permissions of being a "sys-libs" file and now it's only a "BrowserID-lib" file. I'm guessing that the BrowserID-lib permissions will satisfy but I'll have to look when the app is running.

Comment 5

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 707211 [details] [diff] [review]
New patch which recategorizes node_modules as BrowserID-lib

Looks good to me. There's an additional thing that you wanna do, it's to reset the FD type of files that don't need it anymore, otherwise, RSBAC doesn't know (it stops applying it, but it wont remove anything previously set)

That's ok for access requests because you actually rewrite them.

I use the "fixups.policy" file in puppet for this, for example here what you want in fixups.policy is:

attr_set_file_dir -v 66565 FD "//opt/browserid/node_modules/jwcrypto/node_modules/bigint/build/Release/bigint.node" rc_type_fd 4294967294

(that's "inherit")

Comment 6

[Gene Wood [:gene]]

Attachment: Adding in kang's policy fixup suggestion

Comment 7

[Gene Wood [:gene]]

Cool, I'll plan to deploy this today at 2pm to stage, test it out and then move it onto prod.

Comment 8

[Gene Wood [:gene]]

Attachment: Adding in MAP_EXEC for all browserid node ROLES and creating a new ROLE for "static"

Comment 9

[Gene Wood [:gene]]

I've run out of time for my prod rsbac change. I will roll back the staging change, disable rsbac in stage (so as to not break staging) and redeploy to stage and prod tomorrow.

Comment 10

[Gene Wood [:gene]]

Side note : I've seen this error which probably isn't a show stopper and can be ignored but I thought I'd drop it here so I could look at it tomorrow :

<7>0000046070|check_comp_rc(): pid 6766 (nginx), owner 0, rc_role 10010, DIR rc_type 201, request SEARCH -> NOT_GRANTED!
<6>0000046071|rsbac_adf_request(): request SEARCH, pid 6766, ppid 6765, prog_name nginx, prog_file /bin/bash, uid 0, target_type DIR, tid Device 08:03 Inode 6435073 Path /sbin, attr none, value none, result NOT_GRANTED by RC

Comment 11

[John Morrison [:jrgm]]

(In reply to Gene Wood [:gene] from comment #9)
> I've run out of time for my prod rsbac change. I will roll back the staging
> change, disable rsbac in stage (so as to not break staging) and redeploy to
> stage and prod tomorrow.

So,actually the sweb{1,2,3} appear to have rsbac still enabled (the dbwriter log/main/current is showing a "failed to map segment from shared object: Operation not permitted" startup crash, and the daemon is just flapping.

Comment 12

[Guillaume Destuynder [:kang] [this account is no longer active]]

@comment 10 you can add  SEARCH GET_STATUS_DATA CLOSE CHDIR for this (201=sys-exec)
@comment 11 rsbac generally stays enabled on stage, but rules are probably not back to default (?)

Comment 13

[Gene Wood [:gene]]

I've deployed the changes (the patch not the ones you metion in Comment 12) along with a handful of additional changes which I'll detail tomorrow.

Comment 14

[Gene Wood [:gene]]

Attachment: Final actually deployed changes

Guillaume, here are the final set of changes that I implemented and deployed. Let me know if it's not clear from the patch what was changed and why.

Comment 15

[Guillaume Destuynder [:kang] [this account is no longer active]]

I would also add:

+rc_set_item -V 66565 ROLE 300 admin_roles 50007 1
+rc_set_item -V 66565 ROLE 300 admin_roles 50008 1
+rc_set_item -V 66565 ROLE 300 admin_roles 50009 1
 rc_set_item -V 66565 ROLE 300 assign_roles 50000 1
 rc_set_item -V 66565 ROLE 300 assign_roles 50001 1
 rc_set_item -V 66565 ROLE 300 assign_roles 50002 1
@@ -15,6 +18,9 @@
 rc_set_item -V 66565 ROLE 300 assign_roles 50004 1
 rc_set_item -V 66565 ROLE 300 assign_roles 50005 1
 rc_set_item -V 66565 ROLE 300 assign_roles 50006 1
+rc_set_item -V 66565 ROLE 300 assign_roles 50007 1
+rc_set_item -V 66565 ROLE 300 assign_roles 50008 1
+rc_set_item -V 66565 ROLE 300 assign_roles 50009 1

To ROLE 1 (security), so that we can interactively make those changes as well (300 being puppet)

Comment 16

[Gene Wood [:gene]]

done and comitted