Page MenuHomeFreeBSD
Differential D26546 Diff 77484 usr.bin/bsdiff/bspatch/bspatch.c

Changeset View

Standalone View

View Options

usr.bin/bsdiff/bspatch/bspatch.c

Show First 20 Lines • Show All 150 Lines • ▼ Show 20 Lines if ((newfile = basename(argv[2])) == NULL)
err(1, "basename"); err(1, "basename");
/* open newfile */ /* open newfile */
if ((newfd = openat(dirfd, newfile, if ((newfd = openat(dirfd, newfile,
O_CREAT | O_TRUNC | O_WRONLY | O_BINARY, 0666)) < 0) O_CREAT | O_TRUNC | O_WRONLY | O_BINARY, 0666)) < 0)
err(1, "open(%s)", argv[2]); err(1, "open(%s)", argv[2]);
atexit(exit_cleanup); atexit(exit_cleanup);
#ifndef WITHOUT_CAPSICUM #ifndef WITHOUT_CAPSICUM
if (cap_enter() < 0)
err(1, "failed to enter security sandbox");
cap_rights_init(&rights_ro, CAP_READ, CAP_FSTAT, CAP_SEEK); cap_rights_init(&rights_ro, CAP_READ, CAP_FSTAT, CAP_SEEK);
cap_rights_init(&rights_wr, CAP_WRITE); cap_rights_init(&rights_wr, CAP_WRITE);
cap_rights_init(&rights_dir, CAP_UNLINKAT); cap_rights_init(&rights_dir, CAP_UNLINKAT);
if (cap_rights_limit(fileno(f), &rights_ro) < 0 || if (cap_rights_limit(fileno(f), &rights_ro) < 0 ||
cap_rights_limit(fileno(cpf), &rights_ro) < 0 || cap_rights_limit(fileno(cpf), &rights_ro) < 0 ||
cap_rights_limit(fileno(dpf), &rights_ro) < 0 || cap_rights_limit(fileno(dpf), &rights_ro) < 0 ||
cap_rights_limit(fileno(epf), &rights_ro) < 0 || cap_rights_limit(fileno(epf), &rights_ro) < 0 ||
cap_rights_limit(oldfd, &rights_ro) < 0 || cap_rights_limit(oldfd, &rights_ro) < 0 ||
cap_rights_limit(newfd, &rights_wr) < 0 || cap_rights_limit(newfd, &rights_wr) < 0 ||
cap_rights_limit(dirfd, &rights_dir) < 0) cap_rights_limit(dirfd, &rights_dir) < 0)
err(1, "cap_rights_limit() failed, could not restrict" err(1, "cap_rights_limit() failed, could not restrict"
" capabilities"); " capabilities");
if (cap_enter() < 0)
oshogboUnsubmitted
Not Done
Inline Actions

Is there any issue with that?
You can limit descriptors after entering the sandbox.

oshogbo: Is there any issue with that? You can limit descriptors after entering the sandbox.
emasteAuthorUnsubmitted
Done
Inline Actions

See the review description above - there's no issue with it as is, but IMO it's more clear to enter capability mode after limiting fd rights, because the cap_enter() servers as a marker that we're now in the sandbox and may use potentially-unsafe code.

emaste: See the review description above - there's no issue with it as is, but IMO it's more clear to…
pjdUnsubmitted
Not Done
Inline Actions

To be honest, even after reading the reasoning behind this change I still would prefer to enter capability mode as soon as possible. Entering a sandbox doesn't have to be a one-step process. For example:

  1. Enter capability mode.
  2. Limit descriptors 1&2.
  3. Do some work.
  4. Limit descriptors 3&4; close descriptor 2.
  5. Do more work.
  6. Limit descriptor 4 even further.

etc.
I'd encourage this kind of examples wherever possible.
Sandboxing is about limiting TCB, so in my opinion entering the sandbox as soon as possible and then limit further is a better example.

pjd: To be honest, even after reading the reasoning behind this change I still would prefer to enter…
err(1, "failed to enter security sandbox");
#endif #endif
/* /*
File format: File format:
0 8 "BSDIFF40" 0 8 "BSDIFF40"
8 8 X 8 8 X
16 8 Y 16 8 Y
24 8 sizeof(newfile) 24 8 sizeof(newfile)
▲ Show 20 Lines • Show All 126 Lines • Show Last 20 Lines