#179 fkinit: Prompt for password and OTP separately
Opened 2 months ago by sgallagh. Modified a month ago
sgallagh/fedora-packager main  into  main

file modified
+1
@@ -20,6 +20,7 @@ 

  Requires:       mock curl openssh-clients

  Requires:       redhat-rpm-config

  Requires:       fedpkg >= 1.0

+ Requires:       systemd

  Obsoletes:      fedora-cert < 0.6.0.3-4

  Recommends:     fedora-packager-yubikey

  Recommends:     fedora-packager-kerberos

file modified
+10 -2
@@ -103,8 +103,16 @@ 

  

  kinit -n @$domain -c FILE:$armorcache

  

- echo "Enter your password and OTP concatenated. (Ignore that the prompt is for only the token)"

- kinit -T FILE:$armorcache $_arg_user@$domain

+ F_PASSWORD=$(systemd-ask-password "FAS password:")

+ F_OTP=$(systemd-ask-password "FAS OTP (leave blank if not configured):")

+ 

+ echo -n ${F_PASSWORD}${F_OTP} | kinit -T FILE:$armorcache $_arg_user@$domain >/dev/null

+ unset F_PASSWORD

+ unset F_OTP

+ 

+ # Display the active credential cache overview

+ echo

+ klist

  

  

  # ^^^  TERMINATE YOUR CODE BEFORE THE BOTTOM ARGBASH MARKER  ^^^

Also runs klist afterwards to confirm success.

This adds an explicit dependency on systemd for the use of the
systemd-ask-password command. As that package was already an indirect
dependency, this doesn't increase the footprint.

Signed-off-by: Stephen Gallagher sgallagh@redhat.com

Currently, fkinit doesn't echo anything. Could showing the * leak how long the password is?

Any reason to hide this? The user is (hopefully) getting the OTP from another device, so it should already be visible to anyone looking at the other device.

Currently, fkinit doesn't echo anything. Could showing the * leak how long the password is?

That one is always a judgment call; on the one hand, it can leak the length of the password. Not having any feedback at all can lead to uncertainty as to whether input is working.

I suppose there's another option to generate a random number (1-3) of asterisks per character. That seems like it might be overkill, but I could do it.

Any reason to hide this? The user is (hopefully) getting the OTP from another device, so it should already be visible to anyone looking at the other device.

It's probably overkill, but as it's still privileged information, I assumed that hiding it would be preferable. It's short-lived information, so I suppose I could be talked into treating it differently.

rebased onto 3426836

2 months ago

I pushed an updated version that will print a random number of asterisks for the password. I decided to leave the OTP code obfuscated unless someone gives me a compelling reason to make it visible.

rebased onto 9bea487

2 months ago

Hmm, systems where fkinit is ran will almost certainly have systemd installed. So maybe just use the systemd-ask-password helper? It has more bells-and-whistles like press-TAB-to-suppress-output and a fancy prompt. We had long discussions about echo vs. no-echo vs. random-number-of-chars, and we settled on the approach with tab-to-suppress because random-number-of-chars can be quite confusing when people look at the screen and think they have fat-fingered something.

password=$(systemd-ask-password "Your password:")
otp=$(systemd-ask-password "OTP (press <enter> if not configured):")

OK, I just confirmed that there's no way to install fedora-packager without also getting the systemd package, so I'll switch to that (and we can make the dependency explicit).

rebased onto 0cf1c1e

a month ago

Patched (and I updated the initial comment with more details).

+ F_OTP=$(systemd-ask-password "Enter your FAS OTP (press <enter> if not configured):")

On second thought, systemd-ask-password says "press TAB for no echo" so I think this needs to say "press ENTER if not configured". Sorry for not noticing this earlier.

+ echo -n ${F_PASSWORD}${F_OTP} | kinit -T FILE:$armorcache $_arg_user@$domain > /dev/null

I'd remove the space: >/dev/null.

+ F_OTP=$(systemd-ask-password "Enter your FAS OTP (press <enter> if not configured):")

On second thought, systemd-ask-password says "press TAB for no echo" so I think this needs to say "press ENTER if not configured". Sorry for not noticing this earlier.

I decided to shorten these up a little to not consume so much of the line. They now read:
🔐 FAS password: (press TAB for no echo)
🔐 FAS OTP (leave blank if not configured): (press TAB for no echo)

The OTP prompt is still longer than is ideal, but the previous version was 79 characters long.

+ echo -n ${F_PASSWORD}${F_OTP} | kinit -T FILE:$armorcache $_arg_user@$domain > /dev/null

I'd remove the space: >/dev/null.

I wouldn't. It's a matter of personal preference, but I like to keep operators and values separate when the language supports it.

rebased onto d2b9c33

a month ago

rebased onto 6cf7fbe

a month ago

I also explicitly unset the internal $F_PASSWORD and $F_OTP values once they have been used. Since they weren't exported, they shouldn't be able to be leaked, but I figured it couldn't hurt to be overcautious.

LGTM.


I like to keep operators and values separate when the language supports it.

Yeah, but > is a unary operator, and when it's together with the op, it's easier to distinguish. I.e. [[ x > 12]] but foo >&2 and cat <<EOF. But OK, enough bikeshedding ;)

LGTM.


I like to keep operators and values separate when the language supports it.

Yeah, but > is a unary operator, and when it's together with the op, it's easier to distinguish. I.e. [[ x > 12]] but foo >&2 and cat <<EOF. But OK, enough bikeshedding ;)

OK, you talked me into it. I dropped the space.

Can someone with privilege merge this and do a build?

rebased onto 7d07872

a month ago
Metadata