iScsc / iscsc.fr

The iScsc website, build with passion by wannabe devs 🔥
GNU General Public License v3.0
4 stars 12 forks source link

FEATURE: add an env-sharing script #29

Closed amtoine closed 1 year ago

amtoine commented 1 year ago

Related to #13.

This PR adds a script, in the ./scripts/ directory, to easily share secret files to the members of the iscsc.fr team.

the script itself

usage

./scripts/gpg-share.sh $file $[list of gpg users]

source

legend

:large_blue_circle: there is an info :green_circle: there is a success message :orange_circle: there is a warning :red_circle: there is an error and the script halts

in order

amtoine commented 1 year ago
  • as @atxr said it please provide a dependency check for gpg and the archive software

the dependencies might change, e.g. 7zz, but have a look at 6788388

  • the script currently doesn't work well for user in two word, I tried with mine "Clement Mabileau" --> it interprets it as two different users and then creates two encrypted file. But that's normal looking at the code and I don't think that's important once you know it, you just have to specify only ./gpg-share.sh file Mabileau!

yep, that's how arrays work in bash... and i hate it :sob: i tried to fix this in b4eea59 when USER_FILE is set, the users are extracted from this file and not from stdin

# users.txt
Alexandre Tullot
amtoine

and

USER_FILE=users.txt ./scripts/gpg-share .env.production

should only try to use "Alexandre Tullot" and not "Alexandre" and "Tullot" and the script should still work with stdin when USER_FILE is empty

  • currently the script isn't really clean because it leaves the user with multiple encrypted files at root without removing them after the archive is created (or 7zz does it? I could find out because I don't have this one confused), consider either cleaning them or put them in /tmp I think!

i moved the files to /tmp/ in 72582aa

  • you should add the script folder to the README repository structure I think!

added in 7c68495 :yum:

amtoine commented 1 year ago

@atxr

Is it possible to add an option such as the script reads the users from a file? That would be handy to avoid putting all the names of the team each time.

i've tried that in the previous comment and commits :wink:

ctmbl commented 1 year ago

the dependencies might change, e.g. 7zz, but have a look at https://github.com/iScsc/iscsc.fr/commit/6788388b49dcf2f8fbe744bcbb1337f2cd3c6461

i moved the files to /tmp/ in https://github.com/iScsc/iscsc.fr/commit/72582aa7224407baa553eea7fe031cd47a03441d

added in https://github.com/iScsc/iscsc.fr/commit/7c68495fc5e695e5a2ff73e3e27731588da5dde6 yum

love these three tks!

However the script still doesn't work as expected, but that seems normal as you couldn't test it :confused: I found two issues actually.
First running ./gpg-share.sh ../.env.example a2n-s gives me this;

./gpg-share.sh: info: encrypting '../.env.example' for 'a2n-s'...
gpg: using pgp trust model
gpg: using subkey 99A3107C6A2A29D8 instead of primary key BFDE075AC06746A5
gpg: 99A3107C6A2A29D8: There is no assurance this key belongs to the named user

sub  cv25519/99A3107C6A2A29D8 2022-07-24 a2n-s <stevan.antoine@gmail.com>
 Primary key fingerprint: FBC3 7C3E 1887 74E9 FBD5  2FB9 BFDE 075A C067 46A5
      Subkey fingerprint: 565B 938E 3C91 902C 51FD  71BF 99A3 107C 6A2A 29D8

It is NOT certain that the key belongs to the person named
in the user ID.  If you *really* know what you are doing,
you may answer the next question with yes.

Use this key anyway? (y/N) y
gpg: reading from '../.env.example'
gpg: can't create '/tmp/../.env.example.a2n-s.asc': Permission denied
gpg: ../.env.example: encryption failed: Permission denied

So why does it says weird things about the the key not belonging to the named user?
Second, at the end you'll understand that a simple sed isn't enough to properly create the key path :confused: or the user should be aware that the file he is trying to encrypt must be at $PWD! This problem of path of the file-to-encrypt can also lead to archiving gpg-share.sh instead of the encrypted files because of wrong path resolving...

./gpg-share.sh: info: storing files in archive
sed: -e expression #1, char 20: unknown option to `s'

7-Zip [64] 17.04 : Copyright (c) 1999-2021 Igor Pavlov : 2017-08-28
p7zip Version 17.04 (locale=en_US.UTF-8,Utf16=on,HugeFiles=on,64 bits,4 CPUs x64)

Scanning the drive:
1 file, 2566 bytes (3 KiB)

Creating archive: keys-0.1.0.7z

Items to compress: 1

Files read from disk: 1
Archive size: 1171 bytes (2 KiB)
Everything is Ok
./gpg-share.sh: info: content of the archive

7-Zip [64] 17.04 : Copyright (c) 1999-2021 Igor Pavlov : 2017-08-28
p7zip Version 17.04 (locale=en_US.UTF-8,Utf16=on,HugeFiles=on,64 bits,4 CPUs x64)

Scanning the drive for archives:
1 file, 1171 bytes (2 KiB)

Listing archive: keys-0.1.0.7z

--
Path = keys-0.1.0.7z
Type = 7z
Physical Size = 1171
Headers Size = 130
Method = LZMA2:12
Solid = -
Blocks = 1

   Date      Time    Attr         Size   Compressed  Name
------------------- ----- ------------ ------------  ------------------------
2022-11-14 00:06:15 ....A         2566         1041  gpg-share.sh
------------------- ----- ------------ ------------  ------------------------
2022-11-14 00:06:15               2566         1041  1 files

Or is it because of the weird sed: -e expression #1, char 20: unknown option to 's' ???

Also when using the USER_FILE variable the array creation seems to let a \n at the end of the user ids see:

./scripts/gpg-share.sh: success: key FBC37C3E188774E9FBD52FB9BFDE075AC06746A5 found for 'a2n-s'
./scripts/gpg-share.sh: success: key C4257605A0B2688C1CF2A04BB36E196507F03854 found for 'Clement Mabileau'
./scripts/gpg-share.sh: info: encrypting '.env.example' for 'a2n-s
'...
gpg: a2n-s
: skipped: No public key
gpg: .env.example: encryption failed: No public key
./scripts/gpg-share.sh: info: encrypting '.env.example' for 'Clement Mabileau
'...
gpg: Clement Mabileau
: skipped: No public key
gpg: .env.example: encryption failed: No public key

and

Scanning the drive:

WARNING: No such file or directory
/tmp/.env.example.a2n-s.asc

WARNING: No such file or directory
/tmp/.env.example..asc

WARNING: No such file or directory
/tmp/.env.example.Clement.asc

WARNING: No such file or directory
/tmp/.env.example.Mabileau.asc

WARNING: No such file or directory
/tmp/.env.example..asc

Every of these bugs would have been found by you if you could run it on your computer :confused: , maybe you can create a VM or a container? or test it through ssh on the iscsc server?

amtoine commented 1 year ago

@ctmbl

i can test the script by applying

diff --git a/scripts/gpg-share.sh b/scripts/gpg-share.sh
index 3619dfd..300a4e4 100755
--- a/scripts/gpg-share.sh
+++ b/scripts/gpg-share.sh
@@ -26,7 +26,7 @@ log_info () {

 DEPENDENCIES=(
-  npm
+  # npm
   gpg
   7zz
 )
@@ -52,7 +52,8 @@ check_dependencies () {
 check_dependencies

 # get the version
-VERSION=$(npm pkg get version | sed 's/"//g')
+# VERSION=$(npm pkg get version | sed 's/"//g')
+VERSION="0.1.0"

 # build the archive name
 ARCHIVE="keys-${VERSION}.7z"

do not worry :wink:

gpg output

this is because one of the following i think:

the path

the script should be ran from the root of the repo i did not test this from ./scripts/ and that's too much work for me for this PR... the script does not exhibit this behaviour from the root, what about keeping that for a separate issue? :relieved:

the USER_FILE variable

yup, you're right, i'll have a look at that newline (super annoying :rage:)

amtoine commented 1 year ago

if forgot a fix regarding the USER_FILE @ctmbl that's my fault, it should have worked directly in b4eea59 :yum:

fix should be complete in 30f860c arrays are terrible in bash, it's not the prettiest, with all these sed and tr manipulation and scope tricks, but i can not find a better solution for now... i just do not get why there is a newline in the script when using readarray but not from a shell directly :shrug:

amtoine commented 1 year ago

so, for now, please test the script only from the root :wink: it worked locally on my machine, with either stdin or a USER_FILE :+1:

ctmbl commented 1 year ago

@amtoine

gpg output

Thanks actually that was it, I'm not quite used to gpg right now that's true :sweat_smile:

the path

the script should be ran from the root of the repo i did not test this from ./scripts/ and that's too much work for me for this PR...

I totally understand but we then should document this behavior :wink:, either as a warning at the beginning of the script (after checking $PWD or not) or in a wiki.

the USER_FILE variable

Actually it now works at the encryption step:

./scripts/gpg-share.sh: info: encrypting '.env.example' for 'amtoine'...
gpg: using pgp trust model
gpg: using subkey 5CE93D5885AAB235 instead of primary key 37AAE9B486CFF1AB
gpg: This key probably belongs to the named user
gpg: reading from '.env.example'
gpg: writing to '/tmp/.env.example.amtoine.asc'
gpg: ECDH/AES256 encrypted for: "5CE93D5885AAB235 amtoine <stevan.antoine@gmail.com>"
./scripts/gpg-share.sh: info: encrypting '.env.example' for 'Clement Mabileau'...
gpg: using pgp trust model
gpg: using subkey F0443EE8B1A18913 instead of primary key B36E196507F03854
gpg: This key belongs to us
gpg: reading from '.env.example'
gpg: writing to '/tmp/.env.example.Clement Mabileau.asc'
gpg: ECDH/AES256 encrypted for: "F0443EE8B1A18913 Clement Mabileau <mabileau.clement@gmail.com>"

But still doesn't at the archiving step:

./scripts/gpg-share.sh: info: storing files in archive

7-Zip [64] 17.04 : Copyright (c) 1999-2021 Igor Pavlov : 2017-08-28
p7zip Version 17.04 (locale=en_US.UTF-8,Utf16=on,HugeFiles=on,64 bits,4 CPUs x64)

Scanning the drive:

WARNING: No such file or directory
/tmp/.env.example.Clement.asc

WARNING: No such file or directory
/tmp/.env.example.Mabileau.asc

1 file, 687 bytes (1 KiB)

Creating archive: keys-0.1.0.7z

Items to compress: 1

I know... these two words user ids are really annoying... sorry for that...

Also, could we pass the file as a command line argument instead of an environment variable? something like -u amtoine and -U users.txt to have a symetrical behavior while passing raw users or a file containing them or is it too much work?

amtoine commented 1 year ago

gpg output

Thanks actually that was it, I'm not quite used to gpg right now that's true sweat_smile

yep, this script assumes the keyring of the user is in good shape :thinking:

the path

I totally understand but we then should document this behavior wink, either as a warning at the beginning of the script (after checking $PWD or not) or in a wiki.

i've added a check in b069dd5 should terminate when not in the root of the repo :relieved:

the USER_FILE variable

Actually it now works at the encryption step:

cool :muscle:

But still doesn't at the archiving step: ... I know... these two words user ids are really annoying... sorry for that...

i've changed the way i store the files in the archive:grey_exclamation: in f35fbd3, 8a6e589 and e8233a2, you can find the following:

  • create a unique directory in /tmp/ with mktemp -u and mkdir
  • be explicite about the path to the encrypted files in the output of encrypt
  • archive as follows
  • in a subshell, cd to the directory with encrypted files and archive the files by listing them with find
  • return automatically from the subshell
  • cp the archive back inside the root of the repo

that way

  1. only the archive ends up in the repo
  2. the files are still available in /tmp/
  3. there should not be issues with the names with find

Also, could we pass the file as a command line argument instead of an environment variable? something like -u amtoine and -U users.txt to have a symetrical behavior while passing raw users or a file containing them or is it too much work?

i can think of that after the above changes are good to go :wink:

amtoine commented 1 year ago

mm, for the multi-word users.... i do not know it's a mess :sweat_smile:

what about not allowing them? it will be a lot easier and safer, to avoid files with spaces and so on...

EDIT

actually it's an issue with the way find and tar interact :thinking: let me try to fix this, i have the same behaviour as you have :wink:

proposition

the following diff

diff --git a/scripts/gpg-share.sh b/scripts/gpg-share.sh
index dbb4041..0482eed 100755
--- a/scripts/gpg-share.sh
+++ b/scripts/gpg-share.sh
@@ -137,7 +138,7 @@ encrypt () {
   do
       user=$(echo -n "$_user")
       log_info "encrypting '$file' for '$user' inside '$DUMP_DIR'..."
-      gpg --verbose --recipient "$user" --encrypt --armor --output "$DUMP_DIR/$file.$user.asc" "$file"
+      gpg --verbose --recipient "$user" --encrypt --armor --output $(echo "$DUMP_DIR/$file.$user.asc" | sed 's/\s\+/-/g') "$file"
   done
   log_success "all encrypted file have been saved inside '$DUMP_DIR'"
 }
@@ -149,7 +150,7 @@ archive () {
   # subshell here to avoid archiving the paths to the .asc files
   (
     cd "$DUMP_DIR"
-    tar cvf "$ARCHIVE" $(find . -type f)
+    tar cvf "$ARCHIVE" $(find . -type f | sed 's/\s\+/-/g')
   )
   cp "$DUMP_DIR/$ARCHIVE" .
 }

works with USER_FILE equal to

amtoine
Alexandre tullot

i simply replace the whitespaces in the filenames with "-" :smirk: is that ok for you :yum:

ctmbl commented 1 year ago

@amtoine

the path when running

i've added a check in https://github.com/iScsc/iscsc.fr/commit/b069dd5f3ba571de6f33c122e71b73583735127b should terminate when not in the root of the repo relieved

Exactly what I thought about! Tks a lot!

multiple works user ids/ whitespaces problem

create a unique directory in /tmp/ with mktemp -u and mkdir

I really love that! It's really prettier I think and also more reliable :wink:

actually it's an issue with the way find and tar interact thinking

I was just going to ping you about that... But I'm was also going to propose the same fix :wink:, replacing whitespaces by - or _ seems like the best option :+1:

However you don't need this

-    tar cvf "$ARCHIVE" $(find . -type f)
+    tar cvf "$ARCHIVE" $(find . -type f | sed 's/\s\+/-/g')

because you already saved the encrypted files without whitspaces :wink:, but it sure doesn't harm :+1:

Overall I agree with your proposition

flags -u and -U

let me know what you want to do!

amtoine commented 1 year ago

@atxr @ctmbl

glad it works :relieved:

i've added the fix in the latest a04938a :+1: without the extra sed on find as you suggested :ok_hand:

flags -u and -U

let me know what you want to do!

if that's ok with you both, i'd like to close this PR, as @atxr said it solves the original issue, and think of improvements in an issue i can open right after the merge :yum:

amtoine commented 1 year ago

also, please solve the threads if they have been addressed :wink: otherwise, we won't be able to merge :eyes:

atxr commented 1 year ago

i've added the fix in the latest a04938a +1 without the extra sed on find as you suggested ok_hand

Perfect :ok_hand:

flags -u and -U

if that's ok with you both, i'd like to close this PR, as @atxr said it solves the original issue, and think of improvements in an issue i can open right after the merge :yum:

Totally fine for me :wink: I resolved the remaining threads, we just need @ctmbl approval now.

amtoine commented 1 year ago

@ctmbl, there it is :ok_hand: