From a0a176fa5dd534daaad29bf24a93b6a5c4c22ecc Mon Sep 17 00:00:00 2001
From: Josh Rabinowitz <joshr@joshr.com>
Date: Sat, 7 Mar 2020 13:07:03 -0600
Subject: [PATCH] Issue 552 508 revoked keys (#553)

* warn about 'tell' on expired/revoked/invalid keys
* error if 'tell' used on email with multiple keys
* improve test of 'tell' with subdirs
---
 CHANGELOG.md                     |   8 ++--
 man/man1/git-secret-add.1        | Bin 1629 -> 1627 bytes
 man/man1/git-secret-cat.1        | Bin 1469 -> 1467 bytes
 man/man1/git-secret-changes.1    | Bin 1669 -> 1667 bytes
 man/man1/git-secret-clean.1      | Bin 1099 -> 1097 bytes
 man/man1/git-secret-hide.1       | Bin 3106 -> 3104 bytes
 man/man1/git-secret-init.1       | Bin 1420 -> 1418 bytes
 man/man1/git-secret-killperson.1 | Bin 1007 -> 1005 bytes
 man/man1/git-secret-list.1       | Bin 1133 -> 1131 bytes
 man/man1/git-secret-remove.1     | Bin 1110 -> 1108 bytes
 man/man1/git-secret-reveal.1     | Bin 1760 -> 1758 bytes
 man/man1/git-secret-tell.1       | Bin 1748 -> 1930 bytes
 man/man1/git-secret-tell.1.ronn  |   8 ++--
 man/man1/git-secret-usage.1      | Bin 852 -> 850 bytes
 man/man1/git-secret-whoknows.1   | Bin 983 -> 981 bytes
 man/man7/git-secret.7            | Bin 9245 -> 9243 bytes
 src/_utils/_git_secret_tools.sh  |  63 +++++++++++++++++++++----------
 src/commands/git_secret_tell.sh  |   2 +-
 tests/test_add.bats              |   3 +-
 tests/test_expiration.bats       |   9 ++++-
 20 files changed, 63 insertions(+), 30 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 52d86f96..93385ed0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,10 +1,12 @@
 # Changelog
 
-## {Next Version}
+## {{Next Version}}
 
 ### Bugfixes
 
-- Don't let reveal clobber secret files (#579)
+- In 'tell', warn about disabled, revoked, expired, or invalid keys (#552, #508, #317, #290, #283, #238)
+- Error if 'tell' is used on an email address with multiple keys (#552)
+- Don't let 'reveal' clobber secret files (#579)
 
 ### Misc
 
@@ -34,7 +36,7 @@
 ### Features
 
 - Support SECRETS_PINENTRY env var for gnupg --pinentry-mode parameter (#221)
-- Show output from gnupg if 'hide' fails (#516)
+- Show output from gnupg if 'hide' fails (#516, #202, #317)
 - Add support for Busybox (#478)
 
 ### Bugfixes
diff --git a/man/man1/git-secret-add.1 b/man/man1/git-secret-add.1
index 96f93441bdb050c38e4225b50c362816865a5bf4..fe29932a7e760abab80a1127e0d8ecbf40c6e7b2 100644
GIT binary patch
delta 16
Xcmcc1bDL*E4y$isQF6w{;#aHyIqL?{

delta 18
Zcmcc3bC+jA4!c)kUTI=c<;KEStN=)s2fqLS

diff --git a/man/man1/git-secret-cat.1 b/man/man1/git-secret-cat.1
index 41fbe2c1a620a225d62f6c314d21c3d664be542d..fcec3c13e148c87f5799d67fd16a9450ddd5830d 100644
GIT binary patch
delta 16
XcmdnXy_<VN4y$isQF6w{VlP$zG|vU@

delta 18
ZcmdnZy_b7J4!c)kUTI=c<;FrURscfw2HgMv

diff --git a/man/man1/git-secret-changes.1 b/man/man1/git-secret-changes.1
index feb4c6f233edd604cb3232dc22cb799a078cb63b..0bfa3a8e63b81c557f95a4d256fcbf34d9fb8fce 100644
GIT binary patch
delta 16
XcmZqWZRVX&!0MY=l$^1#oR19vE;t2G

delta 18
ZcmZqXZRMR%!0wfpSDIK<xv`Xw4FEfj20j1)

diff --git a/man/man1/git-secret-clean.1 b/man/man1/git-secret-clean.1
index 59903dfd245d49447f69bc2a202d75702e6ba18f..51d433233499d7dd8323d30197ad91a465373b94 100644
GIT binary patch
delta 16
XcmX@jagt*~9;<I+QF6w{(re5BH&F(X

delta 18
ZcmX@fahhX79=lg!UTI=c<;LP`%m78G2Y~<p

diff --git a/man/man1/git-secret-hide.1 b/man/man1/git-secret-hide.1
index 8c2b9beaf1ac1dfbb83d1c0cee124ac6e6ad846e..c3bc761792c836ab888c5e97f45f3355953efb70 100644
GIT binary patch
delta 16
XcmZ1^u|Q%%E~{^1QF6w{l6BkwGwlXG

delta 18
ZcmZ1=u}ES<F1uG^UTI=c<;J3Q+yFt|2Q2^q

diff --git a/man/man1/git-secret-init.1 b/man/man1/git-secret-init.1
index 50e0341dd407e1f9d1db44d8a9d7a14cc48ae0bf..75bc5643d18a1db3e3beee86f46ffa4b6c949830 100644
GIT binary patch
delta 16
XcmeC-?&6-1%j%n0l$^1#M1mCnF1rO>

delta 18
ZcmeC;?%|%0%kGt!SDIK<xv@xs6#zUS21@_{

diff --git a/man/man1/git-secret-killperson.1 b/man/man1/git-secret-killperson.1
index 4d76da4d567299a09c716c5134c3a30a2fec7d18..0292f93a87cb4ef5e7c155176611cc723f5f3067 100644
GIT binary patch
delta 16
XcmaFQ{+4}0F{^K4QF6w{s#0bEI`Ia9

delta 18
ZcmaFM{+@k8F}qh{UTI=c<;IFqW&lav2XFuY

diff --git a/man/man1/git-secret-list.1 b/man/man1/git-secret-list.1
index 26bda7d065a1533b71f3dbc857bba68e9e3bbed3..7e9c5f5c85c586f6fd56fa6edf10388d904c6f19 100644
GIT binary patch
delta 16
XcmaFM@tR{oE~{^1QF6w{lAp`~JB0@K

delta 18
ZcmaFO@s?vkF1uG^UTI=c<;J3)%m7Ma2j&0(

diff --git a/man/man1/git-secret-remove.1 b/man/man1/git-secret-remove.1
index 067cfa284ea18a24d80d3ff1f703a8fc8a400242..03b9458c9270ac3ab2dcb9bcb15a321dd9258aa0 100644
GIT binary patch
delta 16
Xcmcb{afM?-KC5qHQF6w{vIoonIK2k6

delta 18
Zcmcb@agAd_KD$?9UTI=c<;Ic+%m7D12crN0

diff --git a/man/man1/git-secret-reveal.1 b/man/man1/git-secret-reveal.1
index c65f5a25706b0375c3bb60c20a8dc182446c19d4..1e5948c5d784cffb0446cd2edfd618598658f872 100644
GIT binary patch
delta 16
XcmaFBdyjWQKC5qHQF6w{vMe?LIgAEh

delta 18
Zcmcb|`+#>sKD$?9UTI=c<;IdMHULOX2TlM0

diff --git a/man/man1/git-secret-tell.1 b/man/man1/git-secret-tell.1
index 05f56a6c430d7f3c094caa33d136d02ade5101c5..d18ce63b256c658c0b335b8e579b36c54c2f0da7 100644
GIT binary patch
delta 319
zcmXYsJx;_x422aUDung~NT#A;7X;!0hz2AmD5%nS6VJxVWE^>%4XNm<Ksm}DffFEc
z9112tWy?SBeedh=b@A5j-=miGyL}%>AP|LClqSs{rjKlSY&AhCsuVj36^9LU{n~R0
z4UNFTWVhJ6$zGmFBTI-N4hna@VJnuAN+km$?+f<`=!_K3i`8YZx`2VbhXFMJuBEqG
zE`rr)fJ#8m9V=C*ptO}{ZW5Oy79BWJ^4+56G{sN>`Y2bx4%*%ucb@*VW*H}aI&kS1
zAtv^;qZZi%R#i$g-Q~+HPD6flc55(5C{E_0O`@|Jxa~xZD)<lbi-=(yaNK?7dHw@G
CRdSmE

delta 149
zcmeC;zrs5qm)$EduQai!a$?aRro_C-m5hGOG3v#Wk2C7ZCn^-A78U2`0r?6ArAaxN
z$qL!2l?s`8CHV>^8L68^nOqnri?OIo7GRQaN(+k7Q*g|yEKjT~)=|hTQ78v#1W`rV
t3gww4848Jcl?wT3Ae{=usmVpDB{8}{<BAnBa|`l|N>Wocx3e%a0{}JiGiCq)

diff --git a/man/man1/git-secret-tell.1.ronn b/man/man1/git-secret-tell.1.ronn
index 352c7f3d..a86134d3 100644
--- a/man/man1/git-secret-tell.1.ronn
+++ b/man/man1/git-secret-tell.1.ronn
@@ -7,14 +7,16 @@ git-secret-tell - adds a person, who can access private data.
 
 
 ## DESCRIPTION
-`git-secret-tell` receives an email addresses as an input, searches for the `gpg`-key in the `gpg`'s 
-`homedir` by these emails, then imports a person's public key into the `git-secret`'s inner keychain. 
+`git-secret-tell` receives one or more email addresses as an input, searches for the `gpg`-key in the `gpg`
+`homedir` by these emails, then imports the corresponding public key into `git-secret`'s inner keychain. 
 From this moment this person can encrypt new files with the keyring which contains their key,
 but they cannot decrypt the old files, which were already encrypted without their key. 
 The files should be re-encrypted with the new keyring by someone who has the unencrypted files.
 
-**Do not manually import secret key into `git-secret`**. Anyways, it won't work with any of the secret-keys imported.
+Versions of `git-secret tell` after 0.3.2 will warn about keys that are expired, revoked, or otherwise invalid,
+and also if multiple keys are found for a single email address.
 
+**Do not manually import secret keys into `git-secret`**. It won't work with imported secret keys anyway.
 
 ## OPTIONS
 
diff --git a/man/man1/git-secret-usage.1 b/man/man1/git-secret-usage.1
index 7f98210d48df1d970a4df8430bd85a0ffb631b6d..e7110a8723a549b57df44b6c9976c51b9caa26ca 100644
GIT binary patch
delta 16
Xcmcb@c8P659;<I+QF6w{()&yRI5-BV

delta 18
Zcmcb_c7<(19=lg!UTI=c<;LRsOaMl!2bTZ<

diff --git a/man/man1/git-secret-whoknows.1 b/man/man1/git-secret-whoknows.1
index dcf3ecb2ff35d4fa7c5c5464b4e489257724b72a..28c890db0f8fb16752aa8a301a20dac1e9d25de7 100644
GIT binary patch
delta 16
Xcmcc4ewBSfA**j<QF6w{ig;!KH`xX^

delta 18
Zcmcc0ew}?nA-h*%UTI=c<;JpjW&lQF2POaj

diff --git a/man/man7/git-secret.7 b/man/man7/git-secret.7
index 2fa49713b2d3bab737d43cf20b879036dee0fc3a..34bd969437f06aecacdfa61e6fab991439b33b2b 100644
GIT binary patch
delta 16
XcmbR1G23H8I;(GDQF6w{y!FZeI9&#z

delta 18
ZcmbR3G1p^4I=fe5UTI=c<;I-#$^b_02ao^&

diff --git a/src/_utils/_git_secret_tools.sh b/src/_utils/_git_secret_tools.sh
index 5c72c625..302bbf7d 100644
--- a/src/_utils/_git_secret_tools.sh
+++ b/src/_utils/_git_secret_tools.sh
@@ -539,7 +539,7 @@ function _exe_is_busybox {
   echo "$is_busybox"
 }
 
-
+# this is used by just about every command
 function _user_required {
   # This function does a bunch of validations:
   # 1. It calls `_secrets_dir_exists` to verify that "$_SECRETS_DIR" exists.
@@ -606,18 +606,19 @@ function _assert_keychain_contains_emails {
   gpg_uids=$(_get_users_in_gpg_keyring "$homedir")
   for email in "${emails[@]}"; do
     if [[ $email != *"@"* ]]; then
-        _abort "does not appear to be an email: $email"
+      _abort "does not appear to be an email: $email"
     fi
-    local email_ok=0
+    local emails_found=0
     for uid in $gpg_uids; do
-        if [[ "$uid" == "$email" ]]; then
-            email_ok=1
-            break
-        fi
+      if [[ "$uid" == "$email" ]]; then
+        emails_found=$((emails_found+1))
+      fi
     done
-    if [[ $email_ok -eq 0 ]]; then
-      _abort "email not found in gpg keyring: $email"
-    fi
+    if [[ $emails_found -eq 0 ]]; then
+      _abort "no key found in gpg keyring for: $email"
+    elif [[ $emails_found -gt 1 ]]; then
+      _abort "$emails_found keys found in gpg keyring for: $email"
+    fi 
   done
 }
 
@@ -630,7 +631,7 @@ function _get_encrypted_filename {
   echo "${filename}${SECRETS_EXTENSION}" | sed -e 's#^\./##'
 }
 
-
+# this is used throughout this file, and in 'whoknows'
 function _get_users_in_gpg_keyring {
   # show the users in the gpg keyring.
   # `whoknows` command uses it internally.
@@ -642,19 +643,41 @@ function _get_users_in_gpg_keyring {
     args+=( "--homedir" "$homedir" )
   fi
 
-  # we use --fixed-list-mode so older versions of gpg emit 'uid:' lines.
-  # here gawk splits on colon as --with-colon, exact matches field 1 as 'uid', and selects field 10 "User-ID" 
-  # the gensub regex extracts email from <> within field 10. (If there's no <>, then field is just an email address 
-  #  (and maybe a comment) and the regex just passes it through.)
-  # sed at the end removes any 'comment' that appears in parentheses, for #530
-  # 3>&- closes fd 3 for bats, see https://github.com/bats-core/bats-core#file-descriptor-3-read-this-if-bats-hangs
+  ## We use --fixed-list-mode so older versions of gpg emit 'uid:' lines.
+  ## Gawk splits on colon as --with-colon, matches field 1 as 'uid', 
   result=$($SECRETS_GPG_COMMAND "${args[@]}" --no-permission-warning --list-public-keys --with-colon --fixed-list-mode | \
-      gawk -F: '$1~/uid/{print gensub(/.*<(.*)>.*/, "\\1", "g", $10); }' | \
-      sed 's/([^)]*)//g' 3>&-)
+      gawk -F: '$1=="uid"' )
 
-  echo "$result"
+  # For #508 / #552: warn user if gpg indicates keys are one of:
+  # i=invalid, d=disabled, r=revoked, e=expired, n=not valid
+  # See https://github.com/gpg/gnupg/blob/master/doc/DETAILS#field-2---validity # for more on gpg 'validity codes'.
+  local invalid_lines
+  invalid_lines=$(echo "$result" | gawk -F: '$2=="i" || $2=="d" || $2=="r" || $2=="e" || $2=="n"')
+
+  local emails
+  emails=$(_extract_emails_from_gpg_output "$result")
+
+  local emails_with_invalid_keys
+  emails_with_invalid_keys=$(_extract_emails_from_gpg_output "$invalid_lines")
+
+  if [[ -n "$emails_with_invalid_keys" ]]; then
+     _warn "at least one key for email(s) is revoked, expired, or otherwise invalid: $emails_with_invalid_keys"
+  fi
+
+  echo "$emails"
 }
 
+function _extract_emails_from_gpg_output {
+  local result=$1
+
+  # gensub() outputs email from <> within field 10, "User-ID".  If there's no <>, then field is just an email address 
+  #  (and maybe a comment) and we pass it through.
+  # Sed at the end removes any 'comment' that appears in parentheses, for #530
+  # 3>&- closes fd 3 for bats, see https://github.com/bats-core/bats-core#file-descriptor-3-read-this-if-bats-hangs
+  local emails
+  emails=$(echo "$result" | gawk -F: '{print gensub(/.*<(.*)>.*/, "\\1", "g", $10); }' | sed 's/([^)]*)//g' 3>&-)
+  echo "$emails"
+}
 
 function _get_users_in_gitsecret_keyring {
   # show the users in the gitsecret keyring.
diff --git a/src/commands/git_secret_tell.sh b/src/commands/git_secret_tell.sh
index af22db24..768b818d 100644
--- a/src/commands/git_secret_tell.sh
+++ b/src/commands/git_secret_tell.sh
@@ -64,7 +64,7 @@ function tell {
   if [[ "${#emails[@]}" -eq 0 ]]; then
     # If after possible addition of git_email, emails are still empty,
     # we should raise an exception.
-    _abort "you must provide at least one email address."
+    _abort "you must use -m or provide at least one email address."
   fi
 
   _assert_keychain_contains_emails "$homedir" "${emails[@]}"
diff --git a/tests/test_add.bats b/tests/test_add.bats
index a063d2d0..e2995bee 100644
--- a/tests/test_add.bats
+++ b/tests/test_add.bats
@@ -105,7 +105,8 @@ function teardown {
   run git secret add -i "$test_file"
   [ "$status" -eq 0 ]
 
-  run _file_has_line "$test_file" "../.gitignore"
+  [[ -f "$current_dir/.gitignore" ]]
+  run _file_has_line "$test_file" "$current_dir/.gitignore"
   [ "$status" -eq 0 ]
 
   # .gitignore was not created:
diff --git a/tests/test_expiration.bats b/tests/test_expiration.bats
index fcee482a..8a387cbe 100644
--- a/tests/test_expiration.bats
+++ b/tests/test_expiration.bats
@@ -18,7 +18,7 @@ function teardown {
   unset_current_state
 }
 
-@test "test 'hide' using expired key" {
+@test "run 'hide' using expired key" {
   FILE_TO_HIDE="$TEST_DEFAULT_FILENAME"
   FILE_CONTENTS="hidden content юникод"
   set_state_secret_add "$FILE_TO_HIDE" "$FILE_CONTENTS"
@@ -34,6 +34,11 @@ function teardown {
 }
 
 
+@test "run 'whoknows' using expired key" {
+  run git secret whoknows
+  [ $status -eq 0 ] 
+}
+
 @test "run 'whoknows -l' on only expired user" {
   run git secret whoknows -l
   [ "$status" -eq 0 ]
@@ -50,7 +55,7 @@ function teardown {
 
 
 
-@test "run 'whoknows -l' on expired and normal user" {
+@test "run 'whoknows -l' on normal key and expired key" {
   install_fixture_key "$TEST_DEFAULT_USER"
   set_state_secret_tell "$TEST_DEFAULT_USER"