Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(460)

Side by Side Diff: sandbox/linux/syscall_broker/broker_policy.cc

Issue 721553002: sandbox: Extend BrokerPolicy to support file creation (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: minor fix Created 6 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "sandbox/linux/syscall_broker/broker_policy.h" 5 #include "sandbox/linux/syscall_broker/broker_policy.h"
6 6
7 #include <fcntl.h> 7 #include <fcntl.h>
8 #include <stdint.h> 8 #include <stdint.h>
9 #include <string.h> 9 #include <string.h>
10 10
11 #include <string> 11 #include <string>
12 #include <vector> 12 #include <vector>
13 13
14 #include "base/logging.h" 14 #include "base/logging.h"
15 #include "sandbox/linux/syscall_broker/broker_common.h" 15 #include "sandbox/linux/syscall_broker/broker_common.h"
16 16
17 namespace sandbox { 17 namespace sandbox {
18 namespace syscall_broker { 18 namespace syscall_broker {
19 19
20 namespace { 20 namespace {
21 21
22 // Validate a path to be evaulated by this policy.
23 // Async signal safe - calls: strlen
24 bool ValidatePath(const char* path) {
25 if (!path)
26 return false;
27
28 size_t len = strlen(path);
29 // No empty paths
30 if (len == 0)
31 return false;
32 // Paths must be absolute and not relative
33 if (path[0] != '/')
34 return false;
35 // No trailing / (but "/" is valid)
36 if (len > 1 && path[len - 1] == '/')
37 return false;
38 // No trailing /..
39 if (len >= 3 && path[len - 3] == '/' && path[len - 2] == '.' &&
40 path[len - 1] == '.')
41 return false;
42 // No /../ anywhere
43 for (size_t i = 0; i < len; i++) {
44 if (path[i] == '/' && (len - i) > 3) {
45 if (path[i + 1] == '.' && path[i + 2] == '.' && path[i + 3] == '/') {
46 return false;
47 }
48 }
49 }
50 return true;
51 }
52
22 // We maintain a list of flags that have been reviewed for "sanity" and that 53 // We maintain a list of flags that have been reviewed for "sanity" and that
23 // we're ok to allow in the broker. 54 // we're ok to allow in the broker.
24 // I.e. here is where we wouldn't add O_RESET_FILE_SYSTEM. 55 // I.e. here is where we wouldn't add O_RESET_FILE_SYSTEM.
25 bool IsAllowedOpenFlags(int flags) { 56 // Async signal safe - No external calls
26 // First, check the access mode. 57 bool CheckFlags(const BrokerPermission& perm, int flags) {
58 // First, check the access mode is valid.
27 const int access_mode = flags & O_ACCMODE; 59 const int access_mode = flags & O_ACCMODE;
28 if (access_mode != O_RDONLY && access_mode != O_WRONLY && 60 if (access_mode != O_RDONLY && access_mode != O_WRONLY &&
29 access_mode != O_RDWR) { 61 access_mode != O_RDWR) {
30 return false; 62 return false;
31 } 63 }
32 64
33 // We only support a 2-parameters open, so we forbid O_CREAT. 65 // Check if read is allowed
34 if (flags & O_CREAT) { 66 if (!perm.allow_read && (access_mode == O_RDONLY || access_mode == O_RDWR)) {
67 return false;
68 }
69
70 // Check if write is allowed
71 if (!perm.allow_write && (access_mode == O_WRONLY || access_mode == O_RDWR)) {
72 return false;
73 }
74
75 // Check if file creation is allowed.
76 if (!perm.allow_create && (flags & O_CREAT)) {
77 return false;
78 }
79
80 // If O_CREAT is present, ensure O_EXCL
81 if ((flags & O_CREAT) && !(flags & O_EXCL)) {
82 return false;
83 }
84
85 // If this file is to be unlinked, ensure its created.
86 if (perm.unlink && !(flags & O_CREAT)) {
35 return false; 87 return false;
36 } 88 }
37 89
38 // Some flags affect the behavior of the current process. We don't support 90 // Some flags affect the behavior of the current process. We don't support
39 // them and don't allow them for now. 91 // them and don't allow them for now.
40 if (flags & kCurrentProcessOpenFlagsMask) 92 if (flags & kCurrentProcessOpenFlagsMask) {
41 return false; 93 return false;
94 }
42 95
43 // Now check that all the flags are known to us. 96 // Now check that all the flags are known to us.
44 const int creation_and_status_flags = flags & ~O_ACCMODE; 97 const int creation_and_status_flags = flags & ~O_ACCMODE;
45 98
46 const int known_flags = O_APPEND | O_ASYNC | O_CLOEXEC | O_CREAT | O_DIRECT | 99 const int known_flags = O_APPEND | O_ASYNC | O_CLOEXEC | O_CREAT | O_DIRECT |
47 O_DIRECTORY | O_EXCL | O_LARGEFILE | O_NOATIME | 100 O_DIRECTORY | O_EXCL | O_LARGEFILE | O_NOATIME |
48 O_NOCTTY | O_NOFOLLOW | O_NONBLOCK | O_NDELAY | 101 O_NOCTTY | O_NOFOLLOW | O_NONBLOCK | O_NDELAY |
49 O_SYNC | O_TRUNC; 102 O_SYNC | O_TRUNC;
50 103
51 const int unknown_flags = ~known_flags; 104 const int unknown_flags = ~known_flags;
52 const bool has_unknown_flags = creation_and_status_flags & unknown_flags; 105 const bool has_unknown_flags = creation_and_status_flags & unknown_flags;
53 return !has_unknown_flags; 106 return !has_unknown_flags;
54 } 107 }
55 108
56 // Needs to be async signal safe if |file_to_open| is NULL. 109 // Sets the filename.
57 // TODO(jln): assert signal safety. 110 // Async signal safe if file_to_open == NULL
58 bool GetFileNameInWhitelist(const std::vector<std::string>& allowed_file_names, 111 void SetFileName(const BrokerPermission& perm,
59 const char* requested_filename, 112 const char* requested_filename,
60 const char** file_to_open) { 113 const char** file_to_open) {
61 if (file_to_open && *file_to_open) { 114 if (file_to_open) {
62 // Make sure that callers never pass a non-empty string. In case callers 115 if (perm.recursive)
63 // wrongly forget to check the return value and look at the string 116 *file_to_open = requested_filename;
64 // instead, this could catch bugs. 117 else
65 RAW_LOG(FATAL, "*file_to_open should be NULL"); 118 *file_to_open = perm.path.c_str();
119 }
120 }
121
122 bool ValidatePermission(const BrokerPermission& perm) {
123 // Don't allow unlinking on creation without create permission
124 if (perm.unlink && !perm.allow_create)
66 return false; 125 return false;
67 } 126 // Recursive paths must have a trailing slash
68 127 if (perm.recursive && perm.path.back() != '/')
69 // Look for |requested_filename| in |allowed_file_names|. 128 return false;
70 // We don't use ::find() because it takes a std::string and 129 // Whitelisted paths must be absolute.
71 // the conversion allocates memory. 130 if (perm.path.front() != '/')
72 for (const auto& allowed_file_name : allowed_file_names) { 131 return false;
73 if (strcmp(requested_filename, allowed_file_name.c_str()) == 0) { 132 return true;
74 if (file_to_open)
75 *file_to_open = allowed_file_name.c_str();
76 return true;
77 }
78 }
79 return false;
80 } 133 }
81 134
82 } // namespace 135 } // namespace
83 136
84 BrokerPolicy::BrokerPolicy(int denied_errno, 137 BrokerPolicy::BrokerPolicy(int denied_errno,
85 const std::vector<std::string>& allowed_r_files, 138 const std::vector<BrokerPermission>& permissions)
86 const std::vector<std::string>& allowed_w_files)
87 : denied_errno_(denied_errno), 139 : denied_errno_(denied_errno),
88 allowed_r_files_(allowed_r_files), 140 permissions_(permissions),
89 allowed_w_files_(allowed_w_files) { 141 num_of_permissions_(permissions.size()) {
142 // The spec guarantees vectors store their elements contiguously
143 // so set up a pointer to array of element so it can be used
144 // in async signal safe code instead of vector operations.
145 permissions_array_ = &permissions_[0];
146 for (size_t i = 0; i < num_of_permissions_; i++) {
147 if (!ValidatePermission(permissions_array_[i])) {
148 // TODO: DIE HERE
149 // SANDBOX_DIE("Failed to Validate Broker permissions");
150 }
151 }
90 } 152 }
91 153
92 BrokerPolicy::~BrokerPolicy() { 154 BrokerPolicy::~BrokerPolicy() {
93 } 155 }
94 156
95 // Check if calling access() should be allowed on |requested_filename| with 157 // Check if calling access() should be allowed on |requested_filename| with
96 // mode |requested_mode|. 158 // mode |requested_mode|.
97 // Note: access() being a system call to check permissions, this can get a bit 159 // Note: access() being a system call to check permissions, this can get a bit
98 // confusing. We're checking if calling access() should even be allowed with 160 // confusing. We're checking if calling access() should even be allowed with
99 // the same policy we would use for open(). 161 // the same policy we would use for open().
100 // If |file_to_access| is not NULL, we will return the matching pointer from 162 // If |file_to_access| is not NULL, we will return the matching pointer from
101 // the whitelist. For paranoia a caller should then use |file_to_access|. See 163 // the whitelist. For paranoia a caller should then use |file_to_access|. See
102 // GetFileNameIfAllowedToOpen() for more explanation. 164 // GetFileNameIfAllowedToOpen() for more explanation.
103 // return true if calling access() on this file should be allowed, false 165 // return true if calling access() on this file should be allowed, false
104 // otherwise. 166 // otherwise.
105 // Async signal safe if and only if |file_to_access| is NULL. 167 // Async signal safe if and only if |file_to_access| is NULL.
106 bool BrokerPolicy::GetFileNameIfAllowedToAccess( 168 bool BrokerPolicy::GetFileNameIfAllowedToAccess(
107 const char* requested_filename, 169 const char* requested_filename,
108 int requested_mode, 170 int requested_mode,
109 const char** file_to_access) const { 171 const char** file_to_access) const {
172 if (file_to_access && *file_to_access) {
173 // Make sure that callers never pass a non-empty string. In case callers
174 // wrongly forget to check the return value and look at the string
175 // instead, this could catch bugs.
176 RAW_LOG(FATAL, "*file_to_access should be NULL");
177 return false;
178 }
179 if (!ValidatePath(requested_filename))
180 return false;
181
110 // First, check if |requested_mode| is existence, ability to read or ability 182 // First, check if |requested_mode| is existence, ability to read or ability
111 // to write. We do not support X_OK. 183 // to write. We do not support X_OK.
112 if (requested_mode != F_OK && requested_mode & ~(R_OK | W_OK)) { 184 if (requested_mode != F_OK && requested_mode & ~(R_OK | W_OK)) {
113 return false; 185 return false;
114 } 186 }
115 switch (requested_mode) { 187
116 case F_OK: 188 for (size_t i = 0; i < num_of_permissions_; i++) {
117 // We allow to check for file existence if we can either read or write. 189 const char* path = permissions_array_[i].path.c_str();
118 return GetFileNameInWhitelist( 190 if ((permissions_array_[i].recursive &&
119 allowed_r_files_, requested_filename, file_to_access) || 191 strncmp(requested_filename, path, strlen(path)) == 0) ||
120 GetFileNameInWhitelist( 192 strcmp(requested_filename, path) == 0) {
121 allowed_w_files_, requested_filename, file_to_access); 193 switch (requested_mode) {
122 case R_OK: 194 case F_OK:
123 return GetFileNameInWhitelist( 195 if (permissions_array_[i].allow_read ||
124 allowed_r_files_, requested_filename, file_to_access); 196 permissions_array_[i].allow_write) {
125 case W_OK: 197 SetFileName(permissions_array_[i], requested_filename,
126 return GetFileNameInWhitelist( 198 file_to_access);
127 allowed_w_files_, requested_filename, file_to_access); 199 return true;
128 case R_OK | W_OK: { 200 }
129 bool allowed_for_read_and_write = 201 break;
130 GetFileNameInWhitelist(allowed_r_files_, requested_filename, NULL) && 202 case R_OK:
131 GetFileNameInWhitelist( 203 if (permissions_array_[i].allow_read) {
132 allowed_w_files_, requested_filename, file_to_access); 204 SetFileName(permissions_array_[i], requested_filename,
133 return allowed_for_read_and_write; 205 file_to_access);
206 return true;
207 }
208 break;
209 case W_OK:
210 if (permissions_array_[i].allow_write) {
211 SetFileName(permissions_array_[i], requested_filename,
212 file_to_access);
213 return true;
214 }
215 break;
216 case R_OK | W_OK:
217 if (permissions_array_[i].allow_read &&
218 permissions_array_[i].allow_write) {
219 SetFileName(permissions_array_[i], requested_filename,
220 file_to_access);
221 return true;
222 }
223 break;
224 default:
225 return false;
226 }
227 return false;
134 } 228 }
135 default:
136 return false;
137 } 229 }
230 return false;
138 } 231 }
139 232
140 // Check if |requested_filename| can be opened with flags |requested_flags|. 233 // Check if |requested_filename| can be opened with flags |requested_flags|.
141 // If |file_to_open| is not NULL, we will return the matching pointer from the 234 // If |file_to_open| is not NULL, we will return the matching pointer from the
142 // whitelist. For paranoia, a caller should then use |file_to_open| rather 235 // whitelist. For paranoia, a caller should then use |file_to_open| rather
143 // than |requested_filename|, so that it never attempts to open an 236 // than |requested_filename|, so that it never attempts to open an
144 // attacker-controlled file name, even if an attacker managed to fool the 237 // attacker-controlled file name, even if an attacker managed to fool the
145 // string comparison mechanism. 238 // string comparison mechanism.
146 // Return true if opening should be allowed, false otherwise. 239 // Return true if opening should be allowed, false otherwise.
147 // Async signal safe if and only if |file_to_open| is NULL. 240 // Async signal safe if and only if |file_to_open| is NULL.
148 bool BrokerPolicy::GetFileNameIfAllowedToOpen(const char* requested_filename, 241 bool BrokerPolicy::GetFileNameIfAllowedToOpen(const char* requested_filename,
149 int requested_flags, 242 int requested_flags,
150 const char** file_to_open) const { 243 const char** file_to_open,
151 if (!IsAllowedOpenFlags(requested_flags)) { 244 bool* unlink_after_open) const {
245 if (file_to_open && *file_to_open) {
246 // Make sure that callers never pass a non-empty string. In case callers
247 // wrongly forget to check the return value and look at the string
248 // instead, this could catch bugs.
249 RAW_LOG(FATAL, "*file_to_open should be NULL");
152 return false; 250 return false;
153 } 251 }
154 switch (requested_flags & O_ACCMODE) { 252 if (!ValidatePath(requested_filename))
155 case O_RDONLY: 253 return false;
156 return GetFileNameInWhitelist( 254 for (size_t i = 0; i < num_of_permissions_; i++) {
157 allowed_r_files_, requested_filename, file_to_open); 255 const char* path = permissions_array_[i].path.c_str();
158 case O_WRONLY: 256 if ((permissions_array_[i].recursive &&
159 return GetFileNameInWhitelist( 257 strncmp(requested_filename, path, strlen(path)) == 0) ||
160 allowed_w_files_, requested_filename, file_to_open); 258 strcmp(requested_filename, path) == 0) {
161 case O_RDWR: { 259 if (CheckFlags(permissions_array_[i], requested_flags)) {
162 bool allowed_for_read_and_write = 260 SetFileName(permissions_array_[i], requested_filename, file_to_open);
163 GetFileNameInWhitelist(allowed_r_files_, requested_filename, NULL) && 261 if (unlink_after_open)
164 GetFileNameInWhitelist( 262 *unlink_after_open = permissions_array_[i].unlink;
165 allowed_w_files_, requested_filename, file_to_open); 263 return true;
166 return allowed_for_read_and_write; 264 } else {
265 return false;
266 }
167 } 267 }
168 default:
169 return false;
170 } 268 }
269 return false;
171 } 270 }
172 271
173 } // namespace syscall_broker 272 } // namespace syscall_broker
174 273
175 } // namespace sandbox 274 } // namespace sandbox
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698