eCryptfs

Merge lp:~sergio-pena/ecryptfs/917509 into lp:~ecryptfs/ecryptfs/oldtrunk2

  1. 917509
  2. Merge into oldtrunk2
Proposed by Sergio Peña
Status: Merged
Merged at revision: 685
Proposed branch: lp:~sergio-pena/ecryptfs/917509
Merge into: lp:~ecryptfs/ecryptfs/oldtrunk2
Diff against target: 311 lines (+85/-166)
3 files modified
src/include/ecryptfs.h (+0/-1)
src/libecryptfs/main.c (+0/-100)
src/utils/mount.ecryptfs.c (+85/-65)
To merge this branch: bzr merge lp:~sergio-pena/ecryptfs/917509
Reviewer Review Type Date Requested Status
Dustin Kirkland  Needs Fixing
Tyler Hicks Needs Fixing
Review via email: mp+103779@code.launchpad.net

Description of the change

Here's the code with the use of execl() to mount eCryptfs.

I edited the last proposal after comments from Tyler.

I tested this on Ubuntu Precise and Centos 6.2.
The test included the use of /etc/fstab with noatime,noauto flags. Also I did the mount manually with noauto,noatime options and others ecryptfs options.

To post a comment you must log in.
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Thanks, Sergio! One last change needs to made. Check the man page of waitpid() and note that WEXITSTATUS() should only be used if WIFEXITED() is true.

review: Needs Fixing
Revision history for this message
Dustin Kirkland  (kirkland) wrote :

Sergio,

 - Also, check the indentation of your new ecryptfs_mount(), which is inconsistent with the rest of that file.
 - "if (strlen(opts) > 200) {" ... can we use a suitable constant here instead of an arbitrary "200"?
 - num_opts can go away, as that variable is no longer used, as can flags, in ecryptfs_do_mount()

:-Dustin

review: Needs Fixing
lp:~sergio-pena/ecryptfs/917509 updated
677. By Sergio Peña

* Check WIFEXITED() macro before use WEXITSTATUS().
* Fix code indentation.
* Remove unused variables.

Revision history for this message
Sergio Peña (sergio-pena) wrote :

Hi,

Here are the fixes based on your comments from your last merge proposal review.

lp:~sergio-pena/ecryptfs/917509 updated
678. By Sergio Peña

* Move --no-canonicalize before -t parameter. Current position
  was causing an error when mounting eCryptfs.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/include/ecryptfs.h'
2--- src/include/ecryptfs.h 2009-07-21 23:11:16 +0000
3+++ src/include/ecryptfs.h 2012-05-03 22:39:18 +0000
4@@ -514,7 +514,6 @@
5 int get_string_stdin(char **val, char *prompt, int echo);
6 int stack_pop(struct val_node **head);
7 int stack_pop_val(struct val_node **head, void **val);
8-int ecryptfs_mount(char *source, char *target, unsigned long flags, char *opts);
9 int ecryptfs_get_current_kernel_ciphers(
10 struct ecryptfs_cipher_elem *cipher_list_head);
11 int ecryptfs_default_cipher(struct ecryptfs_cipher_elem **default_cipher,
12
13=== modified file 'src/libecryptfs/main.c'
14--- src/libecryptfs/main.c 2011-02-20 05:19:54 +0000
15+++ src/libecryptfs/main.c 2012-05-03 22:39:18 +0000
16@@ -380,106 +380,6 @@
17 return rc;
18 }
19
20-int ecryptfs_mount(char *source, char *target, unsigned long flags, char *opts)
21-{
22- FILE *mtab_fd = NULL;
23- struct mntent mountent;
24- char *fullpath_source = NULL;
25- char *fullpath_target = NULL;
26- int rc;
27-
28- mountent.mnt_opts = NULL;
29- if (!source) {
30- rc = -EINVAL;
31- syslog(LOG_ERR, "Invalid source directory\n");
32- goto out;
33- }
34- if (!target) {
35- rc = -EINVAL;
36- syslog(LOG_ERR, "Invalid target directory\n");
37- goto out;
38- }
39- if (strlen(opts) > 200) {
40- rc = -EINVAL;
41- syslog(LOG_ERR, "Invalid mount options length\n");
42- goto out;
43- }
44-
45- fullpath_source = realpath(source, NULL);
46- if (!fullpath_source) {
47- rc = -errno;
48- syslog(LOG_ERR, "could not resolve full path for source %s [%d]",
49- source, -errno);
50- goto out;
51- }
52- fullpath_target = realpath(target, NULL);
53- if (!fullpath_target) {
54- rc = -errno;
55- syslog(LOG_ERR, "could not resolve full path for target %s [%d]",
56- target, -errno);
57- goto out;
58- }
59-
60- if (mount(fullpath_source, fullpath_target, "ecryptfs", flags, opts)) {
61- rc = -errno;
62- syslog(LOG_ERR, "Failed to perform eCryptfs mount: [%m]\n");
63- goto out;
64- }
65- mtab_fd = setmntent("/etc/mtab", "a");
66- if (!mtab_fd) {
67- rc = -EACCES;
68- syslog(LOG_ERR, "Failed to update the mount table\n");
69- goto out;
70- }
71- mountent.mnt_fsname = fullpath_source;
72- mountent.mnt_dir = fullpath_target;
73- mountent.mnt_type = "ecryptfs";
74- /* we need the following byte count:
75- * 200 max for opts
76- * 23 max for strings below
77- * 1 the final \0
78- */
79- mountent.mnt_opts = malloc(224);
80- if (!mountent.mnt_opts) {
81- rc = -ENOMEM;
82- syslog(LOG_ERR, "Failed to allocate memory for mount "
83- "options\n");
84- goto out;
85- }
86- memset(mountent.mnt_opts, 0, 224);
87- /* reporting the right mount opts */
88- if (flags & MS_RDONLY)
89- strcat(mountent.mnt_opts,"ro");
90- else
91- strcat(mountent.mnt_opts,"rw");
92- if (flags & MS_NOEXEC)
93- strcat(mountent.mnt_opts,",noexec");
94- if (flags & MS_NOSUID)
95- strcat(mountent.mnt_opts,",nosuid");
96- if (flags & MS_NODEV)
97- strcat(mountent.mnt_opts,",nodev");
98- if (opts) {
99- strcat(mountent.mnt_opts, ",");
100- strcat(mountent.mnt_opts, opts);
101- }
102- mountent.mnt_freq = 0;
103- mountent.mnt_passno = 0;
104- if (addmntent(mtab_fd, &mountent)) {
105- rc = -EIO;
106- syslog(LOG_ERR, "Failed to write to the mount "
107- "table\n");
108- goto out;
109- }
110- rc = 0;
111-out:
112- free(fullpath_source);
113- free(fullpath_target);
114- free(mountent.mnt_opts);
115- if (mtab_fd)
116- endmntent(mtab_fd);
117- return rc;
118-}
119-
120 static int zombie_semaphore_get(void)
121 {
122 int sem_id;
123
124=== modified file 'src/utils/mount.ecryptfs.c'
125--- src/utils/mount.ecryptfs.c 2011-12-13 20:47:12 +0000
126+++ src/utils/mount.ecryptfs.c 2012-05-03 22:39:18 +0000
127@@ -135,67 +135,6 @@
128 return rc;
129 }
130
131-static int ecryptfs_generate_mount_flags(char *options, int *flags)
132-{
133- char *opt;
134- char *next_opt;
135- char *end;
136- int num_options = 0;
137-
138- if (!options)
139- return 0;
140-
141- end = strchr(options, '\0');
142- opt = options;
143- while (opt) {
144- if ((next_opt = strchr(opt, ',')))
145- next_opt++;
146- /* the following mount options should be
147- * stripped out from what is passed into the
148- * kernel since these eight options are best
149- * passed as the mount flags rather than
150- * redundantly to the kernel and could
151- * generate spurious warnings depending on the
152- * level of the corresponding cifs vfs kernel
153- * code */
154- if (!strncmp(opt, "nosuid", 6))
155- *flags |= MS_NOSUID;
156- else if (!strncmp(opt, "suid", 4))
157- *flags &= ~MS_NOSUID;
158- else if (!strncmp(opt, "nodev", 5))
159- *flags |= MS_NODEV;
160- else if (!strncmp(opt, "dev", 3))
161- *flags &= ~MS_NODEV;
162- else if (!strncmp(opt, "noexec", 6))
163- *flags |= MS_NOEXEC;
164- else if (!strncmp(opt, "exec", 4))
165- *flags &= ~MS_NOEXEC;
166- else if (!strncmp(opt, "ro", 2))
167- *flags |= MS_RDONLY;
168- else if (!strncmp(opt, "rw", 2))
169- *flags &= ~MS_RDONLY;
170- else if (!strncmp(opt, "remount", 7))
171- *flags |= MS_REMOUNT;
172- else {
173- opt = next_opt;
174- num_options++;
175- continue;
176- }
177- if (!next_opt) {
178- if (opt != options)
179- end = --opt;
180- else
181- end = options;
182- *end = '\0';
183- break;
184- }
185- memcpy(opt, next_opt, end - next_opt);
186- end = end - (next_opt - opt);
187- *end = '\0';
188- }
189- return num_options;
190-}
191-
192 char *parameters_to_scrub[] = {
193 "key=",
194 "cipher=",
195@@ -446,6 +385,90 @@
196 return rc;
197 }
198
199+int ecryptfs_mount(char *source, char *target, char *opts)
200+{
201+ pid_t pid, pid_child;
202+ char *fullpath_source = NULL;
203+ char *fullpath_target = NULL;
204+ int rc, status;
205+
206+ if (!source) {
207+ rc = -EINVAL;
208+ syslog(LOG_ERR, "Invalid source directory\n");
209+ goto out;
210+ }
211+
212+ if (!target) {
213+ rc = -EINVAL;
214+ syslog(LOG_ERR, "Invalid target directory\n");
215+ goto out;
216+ }
217+
218+ if (strlen(opts) > 200) {
219+ rc = -EINVAL;
220+ syslog(LOG_ERR, "Invalid mount options length\n");
221+ goto out;
222+ }
223+
224+ /* source & target are canonicalized here, so the correct error
225+ * is sent to syslog.
226+ * /bin/mount tells you the error on normal output only, not to syslog.
227+ */
228+ fullpath_source = realpath(source, NULL);
229+ if (!fullpath_source) {
230+ rc = -errno;
231+ syslog(LOG_ERR, "could not resolve full path for source %s [%d]",
232+ source, -errno);
233+ goto out;
234+ }
235+
236+ fullpath_target = realpath(target, NULL);
237+ if (!fullpath_target) {
238+ rc = -errno;
239+ syslog(LOG_ERR, "could not resolve full path for target %s [%d]",
240+ target, -errno);
241+ goto out;
242+ }
243+
244+ pid = fork();
245+ if (pid == -1) {
246+ syslog(LOG_ERR, "Could not fork process to mount eCryptfs: [%d]\n", -errno);
247+ rc = -errno;
248+ } else if (pid == 0) {
249+ execl("/bin/mount", "mount", "-i", "--no-canonicalize", "-t", "ecryptfs", fullpath_source, fullpath_target, "-o", opts, NULL);
250+
251+ /* error message shown in console to let users know what was wrong */
252+ /* i.e. /bin/mount does not exist */
253+ perror("Failed to execute /bin/mount command");
254+ exit(errno);
255+ } else {
256+ pid_child = waitpid(pid, &status, 0);
257+ if (pid_child == -1) {
258+ syslog(LOG_ERR, "Failed waiting for /bin/mount process: [%d]\n", -errno);
259+ rc = -errno;
260+ goto out;
261+ }
262+
263+ rc = -EPERM;
264+ if (WIFEXITED(status)) {
265+ rc = (WEXITSTATUS(status))?-WEXITSTATUS(status):0;
266+ }
267+
268+ if (rc) {
269+ syslog(LOG_ERR, "Failed to perform eCryptfs mount: [%d]\n", rc);
270+ if (-EPIPE == rc) {
271+ rc = -EPERM;
272+ }
273+ }
274+ }
275+
276+out:
277+ free(fullpath_source);
278+ free(fullpath_target);
279+
280+ return rc;
281+}
282+
283 /**
284 * ecryptfs_do_mount
285 * @params:
286@@ -460,8 +483,6 @@
287 int sig_cache, struct passwd *pw)
288 {
289 int rc;
290- int flags = 0;
291- int num_opts = 0;
292 char *src = NULL, *targ = NULL, *opts = NULL, *new_opts = NULL, *temp;
293 char *val;
294
295@@ -472,7 +493,6 @@
296 rc = strip_userland_opts(opts);
297 if (rc)
298 goto out;
299- num_opts = ecryptfs_generate_mount_flags(opts, &flags);
300 if (!(temp = strdup("ecryptfs_unlink_sigs"))) {
301 rc = -ENOMEM;
302 goto out;
303@@ -512,7 +532,7 @@
304 rc);
305 goto out;
306 }
307- rc = ecryptfs_mount(src, targ, flags, new_opts);
308+ rc = ecryptfs_mount(src, targ, new_opts);
309 out:
310 free(src);
311 free(targ);

Subscribers

People subscribed via source and target branches