Add non-interactive passphrase query about nitrocli HOT 9 CLOSED

Thanks for bringing that up for discussion. I agree that it would make sense to have that mechanism available for the program as a whole and not just tests or in the context of testing.

With respect to the actual mechanism used, what options do we have? Off hand I can think of three:

• argument to the program
• environment variable
• reading from stdin
• (am I missing something?)

All three should be automatable, though perhaps with different levels of convenience. Perhaps looking at the trade-offs each has would give us a good idea what may be the most suitable mechanism.

For the program argument approach I see two down sides: it may introduce unnecessary implementation complexity (not sure how straight forward it is to implement using argparse but it appears that not all commands should accept this argument [I am thinking of nitrocli status]). Also, given that we have two different passwords to deal with we likely need two different arguments. Third, being a command line argument the password would likely appear in clear text on output such as that of ps. For a short lived program such as nitrocli that may not be a big deal, but it should be a consideration.

Using an environment variable likely also means some form of additional argument to the program need to exist, in order to determine whether to use pinentry or the environment variable. I am not sure if we should just check for the existence of the variable and decide based on that (what's your opinion on that?). The difference is that this argument could probably be valid for all subcommands, as it is a program-wide setting. So that may make it easier to integrate overall. Being an environment variable it would also be visible in /proc, but the more common tools should not accidentally capture it, I guess.

Reading from stdin may require a bit more setup by the user to supply the password but it would make the process very explicit (and almost a bit painful so that users are not tempted to default to it). It's also the least easily readable by other processes. Similar to having an environment variable, we would need a program-wide parameter to decide whether to use pinentry or stdin. However, I am not sure how to differentiate between the admin and user password in such a scheme.

I was tempted to say stdin looking at the list, but I am not sure how to distinguish between the two passwords. Do you have an idea? Perhaps environment variables would be the better choice?

from nitrocli.

That’s mostly what I thought too, expect for the stdin part.

I am not sure if we should just check for the existence of the variable and decide based on that (what's your opinion on that?).

I’d add a top-level option --batch (or --no-interactive). With this approach, we would always have the more secure option as the default. The user would have to explicitly select the less secure option.

Reading from stdin […]
almost a bit painful so that users are not tempted to default to it

First of all, it would also be painful for us because we would have to deal with the terminal attributes to suppress user input. Secondly, I don’t think we should focus too much on making it painful to use the fallback. We provide a sane and safe default behavior. If a user decides to disable the default, they should be trusted to know what they are doing.

However, I am not sure how to differentiate between the admin and user password in such a scheme.

I think that’s a major problem with this approach. It is less flexible and more error-prone than environment variables. The user has to keep track of the PIN type required by each command – which might even change, e. g. for otp get. Also it’s easier to mix up the PINs as they are not specified together with an identifier.

from nitrocli.

Environment variables it is then :)

from nitrocli.

I thought briefly about this issue in the context of testing. Given the way I had the testing in mind (#51) environment variables are not a nice vehicle for conveying this information. They are not suitable because they are a program-wide thing. If we do not fork-exec a new process we will have trouble using them.

So in the testing context, a program argument would be more suitable. But, given how I think we should test, we could just evaluate the environment variable before the entry point for the test. The test could then just use a function argument while normal users of the program could use an environment variable.

from nitrocli.

Well, we could also modify the environment before calling the tests (std::env::set_var). I think it is important to keep the testing code as close to the actual command invocation as possible, so I’d like to avoid having two different mechanisms.

from nitrocli.

Well, we could also modify the environment before calling the tests (std::env::set_var).

Yeah, but that is racy, isn't it? Now we luck out because we serialize all the tests anyway, but if we ever decided to move synchronization into a different layer (say, the nitrokey crate) we may run into problems.

I think it is important to keep the testing code as close to the actual command invocation as possible, so I’d like to avoid having two different mechanisms.

I agree. And we don't introduce two mechanisms, we just hook into the one mechanism we provide at a slightly later point.

from nitrocli.

but if we ever decided to move synchronization into a different layer (say, the nitrokey crate) we may run into problems.

I think we have to synchronize the tests anyway. For example, we will most likely have one test that first configures an OTP slot and then verifies the generated code, and one test that erases an OTP slot and then verifies that the slot is not programmed. Of course we could synchronize on a per-test basis. But that would probably affect a majority of the tests, so I’d prefer to synchronize everything.

from nitrocli.

What I am getting at is that this is global state. Global state is bad, and comes with a lot of problems (random source that I only skimmed and that seems to get the core problems across: https://softwareengineering.stackexchange.com/questions/148108/why-is-global-state-so-evil). It is sometimes necessary but I can't recall a single case where it is indeed desirable. I don't believe it is necessary here, with the solution I outlined.

We can probably find workarounds. We may even find reasons why things may turn out fine. But my main point is that I don't want to have to think about those reasons and whether they apply here. I don't want to waste mental energy re-evaluating them in the future. I choose to use Rust for this project (among other reasons) because it allows me to rule out entire classes of errors, just by virtue of using (the safe parts of) the language.

Let's make this more concrete, to ensure we are really talking about the same thing. What I had in mind:

(pseudo code)

fn main() {
  pin = env["NITROCLI_PIN"]
  run(pin, [...args...])
}

#[test]
fn otp_test() {
  run("123456", [...args...])
}

The core logic resides in run, and that is what we test against, but the decision whether to use an environment variable or not happens in a brief piece of code in main that just prepares the arguments for run.
Now I understand there is a slight divergence between the "product" and what we test, but in short I believe this difference is marginal and manageable (either by additional tests or just by virtue of reviewing the code; and I will go into more detail why I think that is the case when responding to the comments you provided for the pull request).

from nitrocli.

It's ready for prime time!! :-)

(I hope)

from nitrocli.