Skip to content

Commit

Permalink
Fix handling of empty user input in pinentry module
Browse files Browse the repository at this point in the history
As it turns out, the pinentry program responds slightly differently to a
request to enter a password/PIN when the user enters one vs. when he/she
just continues, leaving the input empty, as is illustrated here:

When the user enters 'hello':
  > $ gpg-connect-agent 'GET_PASSPHRASE --data cacheid errormsg prompt description' '/bye'
  > D hello
  > OK

When the user enters nothing:
  > $ gpg-connect-agent 'GET_PASSPHRASE --data cacheid errormsg prompt description' '/bye'
  > OK

So far we always expected a two line response, with the first line
containing the 'D ' prefix, which prevents us from handling empty input
properly.
This change fixes this shortcoming by adding a case for it.

Fixes #176
  • Loading branch information
d-e-s-o committed May 2, 2021
1 parent bed131a commit 4c507f3
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Unreleased
extensions
- Allowed entering of `base32` encoded strings containing spaces
- Fixed pinentry dialog highlighting some messages incorrectly as errors
- Fixed handling of empty user input through pinentry
- Switched to using GitHub Actions as the project's CI pipeline
- Updated minimum supported Rust version to `1.43.0`
- Bumped `nitrokey` dependency to `0.9.0`
Expand Down
38 changes: 25 additions & 13 deletions src/pinentry.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// pinentry.rs

// Copyright (C) 2017-2020 The Nitrocli Developers
// Copyright (C) 2017-2021 The Nitrocli Developers
// SPDX-License-Identifier: GPL-3.0-or-later

use std::borrow;
Expand Down Expand Up @@ -187,6 +187,9 @@ fn parse_pinentry_pin<R>(response: R) -> anyhow::Result<String>
where
R: AsRef<str>,
{
const DATA_PREFIX: &str = "D ";
const ERR_PREFIX: &str = "ERR ";

let string = response.as_ref();
let lines: Vec<&str> = string.lines().collect();

Expand All @@ -195,19 +198,20 @@ where
// > OK
// or potentially:
// > ERR 83886179 Operation cancelled <Pinentry>
if lines.len() == 2 && lines[1] == "OK" && lines[0].starts_with("D ") {
// We got the only valid answer we accept.
let (_, pass) = lines[0].split_at(2);
return Ok(pass.to_string());
}

// Check if we are dealing with a special "ERR " line and report that
// specially.
if !lines.is_empty() && lines[0].starts_with("ERR ") {
let (_, error) = lines[0].split_at(4);
anyhow::bail!("{}", error);
//
// Furthermore, in case of an empty password we'd get just an OK.
match lines.as_slice() {
["OK"] => Ok(String::new()),
[line, "OK"] if line.starts_with(DATA_PREFIX) => {
let (_, pass) = line.split_at(DATA_PREFIX.len());
Ok(pass.to_string())
}
[line] if line.starts_with(ERR_PREFIX) => {
let (_, error) = line.split_at(ERR_PREFIX.len());
anyhow::bail!("{}", error);
}
_ => anyhow::bail!("Unexpected response: {}", string),
}
anyhow::bail!("Unexpected response: {}", string)
}

/// Inquire a secret from the user.
Expand Down Expand Up @@ -333,6 +337,14 @@ where
mod tests {
use super::*;

#[test]
fn parse_pinentry_pin_empty() {
let response = "OK\n";
let expected = "";

assert_eq!(parse_pinentry_pin(response).unwrap(), expected)
}

#[test]
fn parse_pinentry_pin_good() {
let response = "D passphrase\nOK\n";
Expand Down

0 comments on commit 4c507f3

Please sign in to comment.