Mailing List Archive

Hi GnuPG folks,

On 08/04/2018 10:24 PM, Damien Goutte-Gattat via Gnupg-devel wrote:
> Damien Goutte-Gattat (4):
> curses: Disable echoing if backspace is pressed first.
> gtk: Disable echoing if backspace is pressed first.
> qt: Disable echoing if backspace is pressed first.
> tqt: Disable echoing if backspace is pressed first.

Given that the feature was requested on the bug tracker, that the idea
received Werner's approval at the time, and that no one complained about
the implementation in the past 10 days, I intend to apply the patchset
to the master branch of pinentry by tomorrow.

Thanks to Andre and "deloptes" for the green light regarding the Qt and
TQt variants.

Cheers,

Damien

On Tue, 14 Aug 2018 16:15, gnupg-devel@gnupg.org said:

> the implementation in the past 10 days, I intend to apply the patchset
> to the master branch of pinentry by tomorrow.

Fine. My only concern is that a user accidentally hits Backspace does
not see anything and then hist Okay. When entering a PIN that could
increase the odds of accidentally blocking the card (false entry
counter). So what about either

- not using this feature for a PIN
- making sure "[no echo]" is always disabled.


Shalom-Salam,

Werner


p.s.
I did not looked at the code so it might already do that.

--
# Please read: Daniel Ellsberg - The Doomsday Machine #
Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz.

On 21/08/18 12:25, Werner Koch wrote:
> - not using this feature for a PIN

The GnuK is using a "PIN" but it can actually be a real passphrase, and it
encrypts the data on card. So I don't think GnuPG can really tell apart between
a PIN and a passphrase...

My 2 cents,

Peter.

--
I use the GNU Privacy Guard (GnuPG) in combination with Enigmail.
You can send me encrypted mail if you want some privacy.
My key is available at <http://digitalbrains.com/2012/openpgp-key-peter>

Hi Werner,

> So what about either
>
> - not using this feature for a PIN
> - making sure "[no echo]" is always disabled.

(I assume you meant "always displayed" or similar here.)

For the curses pinentry, "[no echo]" is always displayed when
echoing is disabled.

There's no easy way to achieve the same thing with the Gtk/Qt/TQt
pinentries though, so for them I am in favor of the first solution.
It's implemented with the patch below, which uses the presence of
"PIN" in the prompt text to distinguish between a PIN prompt and a
passphrase prompt (same logic already used in gpg-agent).

-- >8 --
Subject: [PATCH pinentry] Disallow echo disabling when prompting for a PIN.

* gtk+-2/pinentry-gtk-2.c (create_window): Do not setup
the backspace handler when prompting for a PIN.
callback only when not prompting for a PIN.
* qt/pinentrydialog.h (_got_input): Rename field to
_disable_echo_allowed.
* qt/pinentrydialog.cpp (PinEntryDialog::setPrompt): Disallow
echo disabling when prompting for a PIN.
* tqt/pinentrydialog.h (_got_input): Rename field to
_disable_echo_allowed.
* tqt/pinentrydialog.cpp (PinEntryDialog::setPrompt): Disallow
echo disabling when prompting for a PIN.

Signed-off-by: Damien Goutte-Gattat <dgouttegattat@incenp.org>
---
gtk+-2/pinentry-gtk-2.c | 10 ++++++++--
qt/pinentrydialog.cpp | 8 +++++---
qt/pinentrydialog.h | 2 +-
tqt/pinentrydialog.cpp | 8 +++++---
tqt/pinentrydialog.h | 2 +-
5 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/gtk+-2/pinentry-gtk-2.c b/gtk+-2/pinentry-gtk-2.c
index 1e07fdc..a4522e4 100644
--- a/gtk+-2/pinentry-gtk-2.c
+++ b/gtk+-2/pinentry-gtk-2.c
@@ -729,8 +729,14 @@ create_window (pinentry_t ctx)
gtk_widget_set_size_request (entry, 200, -1);
g_signal_connect (G_OBJECT (entry), "changed",
G_CALLBACK (changed_text_handler), entry);
- g_signal_connect (G_OBJECT (entry), "backspace",
- G_CALLBACK (backspace_handler), entry);
+
+ /* Enable disabling echo if we're not asking for a PIN. */
+ if (pinentry->prompt && !strstr (pinentry->prompt, "PIN"))
+ {
+ g_signal_connect (G_OBJECT (entry), "backspace",
+ G_CALLBACK (backspace_handler), entry);
+ }
+
hbox = gtk_hbox_new (FALSE, HIG_TINY);
gtk_box_pack_start (GTK_BOX (hbox), entry, TRUE, TRUE, 0);
/* There was a wish in issue #2139 that this button should not
diff --git a/qt/pinentrydialog.cpp b/qt/pinentrydialog.cpp
index b7f2e53..a58e636 100644
--- a/qt/pinentrydialog.cpp
+++ b/qt/pinentrydialog.cpp
@@ -138,7 +138,7 @@ PinEntryDialog::PinEntryDialog(QWidget *parent, const char *name,
: QDialog(parent),
mRepeat(NULL),
_grabbed(false),
- _got_input(false),
+ _disable_echo_allowed(true),
mVisibilityTT(visibilityTT),
mHideTT(hideTT),
mVisiActionEdit(NULL),
@@ -318,6 +318,8 @@ void PinEntryDialog::setPrompt(const QString &txt)
{
_prompt->setText(txt);
_prompt->setVisible(!txt.isEmpty());
+ if (txt.contains("PIN"))
+ _disable_echo_allowed = false;
}

QString PinEntryDialog::prompt() const
@@ -362,7 +364,7 @@ void PinEntryDialog::setQualityBarTT(const QString &txt)

void PinEntryDialog::onBackspace()
{
- if (!_got_input) {
+ if (_disable_echo_allowed) {
_edit->setEchoMode(QLineEdit::NoEcho);
if (mRepeat) {
mRepeat->setEchoMode(QLineEdit::NoEcho);
@@ -380,7 +382,7 @@ void PinEntryDialog::updateQuality(const QString &txt)
_timer->stop();
}

- _got_input = true;
+ _disable_echo_allowed = false;

if (!_have_quality_bar || !_pinentry_info) {
return;
diff --git a/qt/pinentrydialog.h b/qt/pinentrydialog.h
index 396f03b..d5e6963 100644
--- a/qt/pinentrydialog.h
+++ b/qt/pinentrydialog.h
@@ -109,7 +109,7 @@ private:
bool _grabbed;
bool _have_quality_bar;
bool _timed_out;
- bool _got_input;
+ bool _disable_echo_allowed;
pinentry_t _pinentry_info;
QTimer *_timer;
QString mRepeatError,
diff --git a/tqt/pinentrydialog.cpp b/tqt/pinentrydialog.cpp
index 6a2ae12..b7aa309 100644
--- a/tqt/pinentrydialog.cpp
+++ b/tqt/pinentrydialog.cpp
@@ -33,7 +33,7 @@
PinEntryDialog::PinEntryDialog( TQWidget* parent, const char* name,
bool modal, bool enable_quality_bar )
: TQDialog( parent, name, modal, TQt::WStyle_StaysOnTop ), _grabbed( false ),
- _got_input( false )
+ _disable_echo_allowed ( true )
{
TQBoxLayout* top = new TQVBoxLayout( this, 6 );
TQBoxLayout* upperLayout = new TQHBoxLayout( top );
@@ -134,7 +134,7 @@ void PinEntryDialog::updateQuality( const SecTQString & txt )
int percent;
TQPalette pal;

- _got_input = true;
+ _disable_echo_allowed = false;

if (!_have_quality_bar || !_pinentry_info)
return;
@@ -166,7 +166,7 @@ void PinEntryDialog::updateQuality( const SecTQString & txt )

void PinEntryDialog::onBackspace()
{
- if (!_got_input)
+ if (_disable_echo_allowed)
_edit->setEchoMode( SecTQLineEdit::NoEcho );
}

@@ -208,6 +208,8 @@ SecTQString PinEntryDialog::text() const
void PinEntryDialog::setPrompt( const TQString& txt )
{
_prompt->setText( txt );
+ if (txt.contains("PIN"))
+ _disable_echo_allowed = false;
}

TQString PinEntryDialog::prompt() const
diff --git a/tqt/pinentrydialog.h b/tqt/pinentrydialog.h
index eb4d332..8ec3fd5 100644
--- a/tqt/pinentrydialog.h
+++ b/tqt/pinentrydialog.h
@@ -87,7 +87,7 @@ private:
bool _grabbed;
bool _have_quality_bar;
pinentry_t _pinentry_info;
- bool _got_input;
+ bool _disable_echo_allowed;
};


--
2.14.4


_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

On Tue, 21 Aug 2018 21:31, gnupg-devel@gnupg.org said:

> (I assume you meant "always displayed" or similar here.)

Sure.

> For the curses pinentry, "[no echo]" is always displayed when
> echoing is disabled.

I noticed that.

> It's implemented with the patch below, which uses the presence of
> "PIN" in the prompt text to distinguish between a PIN prompt and a
> passphrase prompt (same logic already used in gpg-agent).

It ist not ideal but the best solution we have right now. It would be
better if gpg-agent could tell pinentry who is the requestor (gpg-agent
or scdaemon); this requires quite some re-structuring in gpg-agent. So
this would be something for the future.


Salam-Shalom,

Werner


--
# Please read: Daniel Ellsberg - The Doomsday Machine #
Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz.

On 08/22/2018 09:29 AM, Werner Koch wrote:
> It ist not ideal but the best solution we have right now.

OK then, I will merge that. Thanks.

Regards,

Damien