Andrew Gallagher:
> On 07/01/16 19:41, anonym wrote:
>>
>> I haven't tested it, but I did have a look at src/tcp-helper.c since the
>> necessity of memory unsafe code surprised me:
>
> For the record, I never intended to use C. It was originally written in
> perl (like the GUI wrapper), but setuidperl is no longer supported and I
> needed to drop any requirement for sudo. The perl documentation suggests
> writing a setuid C wrapper that does (more or less)
> "setreuid(0,0); system(MY_PROGRAM);"
> and then lists all the reasons why you shouldn't.
Ok. I'm not entirely sure why you'd need setuid.
> So I reluctantly rewrote the helper in C, and after getting angry with C
> I gave in and #included <string> (forgetting to rename the source
> accordingly). Much of my time since has been spent trying to extricate
> myself from the inevitable foot-shooting. ;-)
Ack!
> Rest of your comments fully understood,
:)
> hands in the air bang to rights guv.
What does this mean? I'm not a native English speaker, which possibly
explains it.
>> Best practice is to drop the input sanitation and pick alternatives to
>> `system()`/`popen()` that avoids invoking the shell and calls `exec()`
>> or similar. I'm too ignorant of C/C++ to recommend you anything.
>
> Yes, these were originally perl system(,,,) and open(,,,) calls, which
> are safe as they don't invoke a shell.
Ok. Still, I'm confused by the need for setuid and going C(++). For the
record, Python's subprocess module would do the same job:
https://docs.python.org/3/library/subprocess.html
> C has no simple equivalent that I
> am aware of,
I guess that'd be a combo of fork() + exec() + IPC to get the results
(return code, stdout/err) back to the parent. :) Or, preferably, using
some existing library which makes this easier.
> so I cheated by adding the overkill input sanitation
> (bearing in mind that in normal use the input parameters will in 99.9%
> of cases be "/live/persistence/TailsData_unlocked" and "/dev/sd[a-z]").
Understood.
> 1. is it worth spending any more time fixing the dodgy C++, or is it
> just throwing good effort after bad?
It's not worth the effort. Sorry, but let's drop the C/C++.
> 2. is the general structure of userspace GUI + setuid helper still the
> way to go?
>
> 3. Since tails-installer-invoker and tails-installer remain distinct
> programs (and tails-installer closely tracks liveusb-creator), is it
> necessary/desirable to merge persistence cloning into either of them, or
> should/could it remain a separate (linked) executable? In other words,
> do I really need to learn Python? ;-)
I mainly threw myself in the discussion to avert us from adding any
C/C++ code. I leave it to sajolida and u to decide on how the backup
tool should integrate into the installer (or if it should be separate),
and for u and intrigeri to clarify the privileges separation situation
in the installer (I think it's run as a normal users, and udisks is used
to partition, format, luksOpen etc without any risky setuid business).
All this will influence the answers to these questions.
Cheers!
