From 4c507f30afdc7eb0d60f45486fb42d039c83cfc1 Mon Sep 17 00:00:00 2001 From: Daniel Mueller Date: Sun, 2 May 2021 10:17:59 -0700 Subject: [PATCH] Fix handling of empty user input in pinentry module 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 --- CHANGELOG.md | 1 + src/pinentry.rs | 38 +++++++++++++++++++++++++------------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a47dd15..e296ab3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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` diff --git a/src/pinentry.rs b/src/pinentry.rs index 48e43c4a..769444da 100644 --- a/src/pinentry.rs +++ b/src/pinentry.rs @@ -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; @@ -187,6 +187,9 @@ fn parse_pinentry_pin(response: R) -> anyhow::Result where R: AsRef, { + const DATA_PREFIX: &str = "D "; + const ERR_PREFIX: &str = "ERR "; + let string = response.as_ref(); let lines: Vec<&str> = string.lines().collect(); @@ -195,19 +198,20 @@ where // > OK // or potentially: // > ERR 83886179 Operation cancelled - 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. @@ -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";