From c523acec019291f34d9ae3a1d35d9022ad4d4fd2 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 17:20:38 +0100 Subject: [PATCH 01/16] [decrypt_test] remove surplus empty lines --- decrypt/decrypt_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/decrypt/decrypt_test.go b/decrypt/decrypt_test.go index 167a99f1..6bdbdfae 100644 --- a/decrypt/decrypt_test.go +++ b/decrypt/decrypt_test.go @@ -27,9 +27,7 @@ func TestDecryptTestSuite(t *testing.T) { } func (suite *DecryptTests) SetupTest() { - var err error - // Create a temporary directory for our files suite.tempDir, err = os.MkdirTemp(os.TempDir(), "sda-cli-test-") if err != nil { @@ -44,7 +42,6 @@ func (suite *DecryptTests) SetupTest() { // ... create some content ... suite.fileContent = []byte("This is some fine content right here.") - // ... and write the known content to it err = os.WriteFile(suite.testFile.Name(), suite.fileContent, 0600) if err != nil { @@ -57,11 +54,8 @@ func (suite *DecryptTests) TearDownTest() { } func (suite *DecryptTests) TestreadPrivateKeyFile() { - testKeyFile := filepath.Join(suite.tempDir, "testkey") - // generate key files - err := createKey.GenerateKeyPair(testKeyFile, "") if err != nil { log.Errorf("couldn't generate testing key pair: %s", err) @@ -99,7 +93,6 @@ func (suite *DecryptTests) TestcheckFiles() { } func (suite *DecryptTests) Testdecrypt() { - testKeyFile := filepath.Join(suite.tempDir, "testkey") encryptedFile := fmt.Sprintf("%s.c4gh", suite.testFile.Name()) decryptedFile := filepath.Join(suite.tempDir, "decrypted_file") From d16610767a76e718f576c233aff757be29476157 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 17:23:28 +0100 Subject: [PATCH 02/16] [decrypt_test] expand TestreadPrivateKeyFile Test reading a public key instead of a private one --- decrypt/decrypt_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/decrypt/decrypt_test.go b/decrypt/decrypt_test.go index 6bdbdfae..dcaf1a96 100644 --- a/decrypt/decrypt_test.go +++ b/decrypt/decrypt_test.go @@ -69,6 +69,10 @@ func (suite *DecryptTests) TestreadPrivateKeyFile() { _, err = readPrivateKeyFile(suite.testFile.Name(), "") assert.ErrorContains(suite.T(), err, fmt.Sprintf("file: %s", suite.testFile.Name())) + // Test reading a public key + _, err = readPrivateKeyFile(fmt.Sprintf("%s.pub.pem", testKeyFile), "") + assert.ErrorContains(suite.T(), err, "private key format not supported") + // Test reading a real key _, err = readPrivateKeyFile(fmt.Sprintf("%s.sec.pem", testKeyFile), "") assert.NoError(suite.T(), err) From 642d5083719f096d83b358c853f136f92f59eb3c Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 17:23:46 +0100 Subject: [PATCH 03/16] [decrypt_test] expand TestreadPrivateKeyFile Use a passphrase protected file and test with both correct and wrong passphrase. --- decrypt/decrypt_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/decrypt/decrypt_test.go b/decrypt/decrypt_test.go index dcaf1a96..0b9c71d9 100644 --- a/decrypt/decrypt_test.go +++ b/decrypt/decrypt_test.go @@ -56,7 +56,7 @@ func (suite *DecryptTests) TearDownTest() { func (suite *DecryptTests) TestreadPrivateKeyFile() { testKeyFile := filepath.Join(suite.tempDir, "testkey") // generate key files - err := createKey.GenerateKeyPair(testKeyFile, "") + err := createKey.GenerateKeyPair(testKeyFile, "test") if err != nil { log.Errorf("couldn't generate testing key pair: %s", err) } @@ -72,9 +72,13 @@ func (suite *DecryptTests) TestreadPrivateKeyFile() { // Test reading a public key _, err = readPrivateKeyFile(fmt.Sprintf("%s.pub.pem", testKeyFile), "") assert.ErrorContains(suite.T(), err, "private key format not supported") - + + // Test reading a real key with wrong passphrase + _, err = readPrivateKeyFile(fmt.Sprintf("%s.sec.pem", testKeyFile), "wrong") + assert.ErrorContains(suite.T(), err, "chacha20poly1305: message authentication failed") + // Test reading a real key - _, err = readPrivateKeyFile(fmt.Sprintf("%s.sec.pem", testKeyFile), "") + _, err = readPrivateKeyFile(fmt.Sprintf("%s.sec.pem", testKeyFile), "test") assert.NoError(suite.T(), err) } From c506a80b6ed929ffa9309c6854316e852cfb48aa Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 17:38:11 +0100 Subject: [PATCH 04/16] [decrypt_test] use asserts rather than `log.errorf` --- decrypt/decrypt_test.go | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/decrypt/decrypt_test.go b/decrypt/decrypt_test.go index 0b9c71d9..9ad91465 100644 --- a/decrypt/decrypt_test.go +++ b/decrypt/decrypt_test.go @@ -126,11 +126,9 @@ func (suite *DecryptTests) Testdecrypt() { if err != nil { log.Error("could not change into test directory") } - encryptArgs := []string{"sda-cli", "-key", fmt.Sprintf("%s.pub.pem", testKeyFile), suite.testFile.Name()} + encryptArgs := []string{"encrypt", "-key", fmt.Sprintf("%s.pub.pem", testKeyFile), suite.testFile.Name()} err = encrypt.Encrypt(encryptArgs) - if err != nil { - log.Errorf("couldn't encrypt file for decryption test: %s", err) - } + assert.NoError(suite.T(), err, "encrypting file for testing failed") err = os.Chdir(cwd) if err != nil { log.Error("could not return from test directory") @@ -155,12 +153,8 @@ func (suite *DecryptTests) Testdecrypt() { // Check content of the decrypted file inFile, err := os.Open(decryptedFile) - if err != nil { - log.Errorf("Couldn't open decrypted file %s for content checking", decryptedFile) - } + assert.NoError(suite.T(), err, "unable to open decrypted file") fileData, err := io.ReadAll(inFile) - if err != nil { - log.Error("Couldn't read decrypted filedata for content checking") - } + assert.NoError(suite.T(), err, "unable to read decrypted file") assert.Equal(suite.T(), fileData, suite.fileContent) } From acc0c48e0987d7b36038fb1092a251a6b8dcc82d Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 17:46:09 +0100 Subject: [PATCH 05/16] [decrypt] remove surplus empty lines --- decrypt/decrypt.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/decrypt/decrypt.go b/decrypt/decrypt.go index 539ae50d..a32cef36 100644 --- a/decrypt/decrypt.go +++ b/decrypt/decrypt.go @@ -45,7 +45,6 @@ var privateKeyFile = Args.String("key", "", // Decrypt takes a set of arguments, parses them, and attempts to decrypt the // given data files with the given private key file.. func Decrypt(args []string) error { - // Call ParseArgs to take care of all the flag parsing err := helpers.ParseArgs(args, Args) if err != nil { @@ -57,10 +56,8 @@ func Decrypt(args []string) error { // All filenames are read into a struct together with their output filenames files := []helpers.EncryptionFileSet{} for _, filename := range Args.Args() { - // Set directory for the output file unencryptedFilename := strings.TrimSuffix(filename, ".c4gh") - files = append(files, helpers.EncryptionFileSet{Encrypted: filename, Unencrypted: unencryptedFilename}) } @@ -70,11 +67,9 @@ func Decrypt(args []string) error { } var privateKey *[32]byte - // try reading private key without password privateKey, err = readPrivateKeyFile(*privateKeyFile, "") if err != nil { - // if there was an error, try again with the password password, err := getPassword("C4GH_PASSWORD") if err != nil { @@ -126,7 +121,6 @@ func getPassword(envVar string) (string, error) { // Reads a private key file from a file using the crypt4gh keys module func readPrivateKeyFile(filename, password string) (key *[32]byte, err error) { - // Check that the file exists if !helpers.FileExists(filename) { return nil, fmt.Errorf("private key file %s doesn't exist", filename) @@ -168,7 +162,6 @@ func checkFiles(files []helpers.EncryptionFileSet) error { // decrypts the data in `filename` with the given `privateKey`, writing the // resulting data to `outfile`. func decrypt(filename, outfileName string, privateKey [32]byte) error { - // check that the infile exists, and the the outfile doesn't exist if !helpers.FileIsReadable(filename) { return fmt.Errorf("infile %s does not exist or could not be read", filename) From c3ca0e0e119e578f5d28e70a4403a1f95eaf0883 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 17:47:16 +0100 Subject: [PATCH 06/16] [decrypt] no need to check for error when closing a file Unless it can be closed by another thread --- decrypt/decrypt.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/decrypt/decrypt.go b/decrypt/decrypt.go index a32cef36..2c81e4b4 100644 --- a/decrypt/decrypt.go +++ b/decrypt/decrypt.go @@ -176,11 +176,7 @@ func decrypt(filename, outfileName string, privateKey [32]byte) error { if err != nil { return err } - defer func() { - if err := inFile.Close(); err != nil { - log.Errorf("error closing file: %s\n", err) - } - }() + defer inFile.Close() // Create crypt4gh reader crypt4GHReader, err := streaming.NewCrypt4GHReader(inFile, privateKey, nil) From d6193613367fb9af5488c0b54da8d6d9eace74b8 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 18:27:09 +0100 Subject: [PATCH 07/16] [decrypt_test] Add test for main Decrypt function --- decrypt/decrypt_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/decrypt/decrypt_test.go b/decrypt/decrypt_test.go index 9ad91465..01aea91b 100644 --- a/decrypt/decrypt_test.go +++ b/decrypt/decrypt_test.go @@ -158,3 +158,37 @@ func (suite *DecryptTests) Testdecrypt() { assert.NoError(suite.T(), err, "unable to read decrypted file") assert.Equal(suite.T(), fileData, suite.fileContent) } + +func (suite *DecryptTests) TestDecrypt() { + testKeyFile := filepath.Join(suite.tempDir, "testkey") + err := createKey.GenerateKeyPair(testKeyFile, "") + if err != nil { + log.Errorf("couldn't generate testing key pair: %s", err) + } + + // Encrypt a file using the encrypt module. change to the test directory to + // make sure that the checksum files end up there. + cwd, err := os.Getwd() + if err != nil { + log.Error("could not get working directory") + } + err = os.Chdir(suite.tempDir) + if err != nil { + log.Error("could not change into test directory") + } + encryptArgs := []string{"encrypt", "-key", fmt.Sprintf("%s.pub.pem", testKeyFile), suite.testFile.Name()} + assert.NoError(suite.T(), encrypt.Encrypt(encryptArgs), "encrypting file for testing failed") + assert.NoError(suite.T(), os.Chdir(cwd)) + assert.NoError(suite.T(), os.Remove(suite.testFile.Name())) + + os.Args = []string{"decrypt", "-key", fmt.Sprintf("%s.sec.pem", testKeyFile), fmt.Sprintf("%s.c4gh", suite.testFile.Name())} + err = Decrypt(os.Args) + assert.NoError(suite.T(), err, "decrypt failed unexpectedly") + + // Check content of the decrypted file + inFile, err := os.Open(suite.testFile.Name()) + assert.NoError(suite.T(), err, "unable to open decrypted file") + fileData, err := io.ReadAll(inFile) + assert.NoError(suite.T(), err, "unable to read decrypted file") + assert.Equal(suite.T(), string(suite.fileContent), string(fileData)) +} From 023d33090d6b41b587ca0da765bf29b6565fbfc9 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 18:29:25 +0100 Subject: [PATCH 08/16] [decrypt] Rename `decrypt` to `decryptFile` This way we can separate the main function form the worker function --- decrypt/decrypt.go | 4 ++-- decrypt/decrypt_test.go | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/decrypt/decrypt.go b/decrypt/decrypt.go index 2c81e4b4..94f73b75 100644 --- a/decrypt/decrypt.go +++ b/decrypt/decrypt.go @@ -94,7 +94,7 @@ func Decrypt(args []string) error { for i, file := range files { log.Infof("Decrypting file %v/%v: %s", i+1, numFiles, file.Encrypted) - err = decrypt(file.Encrypted, file.Unencrypted, *privateKey) + err = decryptFile(file.Encrypted, file.Unencrypted, *privateKey) if err != nil { return err } @@ -161,7 +161,7 @@ func checkFiles(files []helpers.EncryptionFileSet) error { // decrypts the data in `filename` with the given `privateKey`, writing the // resulting data to `outfile`. -func decrypt(filename, outfileName string, privateKey [32]byte) error { +func decryptFile(filename, outfileName string, privateKey [32]byte) error { // check that the infile exists, and the the outfile doesn't exist if !helpers.FileIsReadable(filename) { return fmt.Errorf("infile %s does not exist or could not be read", filename) diff --git a/decrypt/decrypt_test.go b/decrypt/decrypt_test.go index 01aea91b..0ecfa472 100644 --- a/decrypt/decrypt_test.go +++ b/decrypt/decrypt_test.go @@ -100,7 +100,7 @@ func (suite *DecryptTests) TestcheckFiles() { assert.EqualError(suite.T(), err, "cannot read input file does-not-exist") } -func (suite *DecryptTests) Testdecrypt() { +func (suite *DecryptTests) TestDecryptFile() { testKeyFile := filepath.Join(suite.tempDir, "testkey") encryptedFile := fmt.Sprintf("%s.c4gh", suite.testFile.Name()) decryptedFile := filepath.Join(suite.tempDir, "decrypted_file") @@ -135,20 +135,20 @@ func (suite *DecryptTests) Testdecrypt() { } // Test decrypting a non-existent file - err = decrypt(filepath.Join(suite.tempDir, "non-existent"), "output_file", *privateKey) + err = decryptFile(filepath.Join(suite.tempDir, "non-existent"), "output_file", *privateKey) assert.EqualError(suite.T(), err, fmt.Sprintf("infile %s does not exist or could not be read", filepath.Join(suite.tempDir, "non-existent"))) // Test decrypting where the output file exists - err = decrypt(encryptedFile, suite.testFile.Name(), *privateKey) + err = decryptFile(encryptedFile, suite.testFile.Name(), *privateKey) assert.EqualError(suite.T(), err, fmt.Sprintf("outfile %s already exists", suite.testFile.Name())) // Test decryption with malformed key fakeKey := [32]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} - err = decrypt(encryptedFile, decryptedFile, fakeKey) + err = decryptFile(encryptedFile, decryptedFile, fakeKey) assert.EqualError(suite.T(), err, "could not create cryp4gh reader: could not find matching public key header, decryption failed") // Test decrypting with the real key - err = decrypt(encryptedFile, decryptedFile, *privateKey) + err = decryptFile(encryptedFile, decryptedFile, *privateKey) assert.NoError(suite.T(), err) // Check content of the decrypted file From 43e8eaabaceddc9900bec524b466e4a769e326f2 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 18:33:21 +0100 Subject: [PATCH 09/16] [decrypt] Merge `checkFiles` into main Decrypt function --- decrypt/decrypt.go | 32 ++++++++++---------------------- decrypt/decrypt_test.go | 21 +-------------------- 2 files changed, 11 insertions(+), 42 deletions(-) diff --git a/decrypt/decrypt.go b/decrypt/decrypt.go index 94f73b75..2746a6a8 100644 --- a/decrypt/decrypt.go +++ b/decrypt/decrypt.go @@ -84,9 +84,16 @@ func Decrypt(args []string) error { } // Check that all the encrypted files exist, and all the unencrypted don't - err = checkFiles(files) - if err != nil { - return err + for _, file := range files { + // check that the input file exists and is readable + if !helpers.FileIsReadable(file.Encrypted) { + return fmt.Errorf("cannot read input file %s", file.Encrypted) + } + + // check that the output file doesn't exist + if helpers.FileExists(file.Unencrypted) { + return fmt.Errorf("outfile %s already exists", file.Unencrypted) + } } // decrypt the input files @@ -140,25 +147,6 @@ func readPrivateKeyFile(filename, password string) (key *[32]byte, err error) { return &privateKey, err } -// Checks that all the encrypted files exists, and are readable, and that the -// unencrypted files do not exist -func checkFiles(files []helpers.EncryptionFileSet) error { - log.Info("Checking files") - for _, file := range files { - // check that the input file exists and is readable - if !helpers.FileIsReadable(file.Encrypted) { - return fmt.Errorf("cannot read input file %s", file.Encrypted) - } - - // check that the output file doesn't exist - if helpers.FileExists(file.Unencrypted) { - return fmt.Errorf("outfile %s already exists", file.Unencrypted) - } - } - - return nil -} - // decrypts the data in `filename` with the given `privateKey`, writing the // resulting data to `outfile`. func decryptFile(filename, outfileName string, privateKey [32]byte) error { diff --git a/decrypt/decrypt_test.go b/decrypt/decrypt_test.go index 0ecfa472..0a782810 100644 --- a/decrypt/decrypt_test.go +++ b/decrypt/decrypt_test.go @@ -9,7 +9,6 @@ import ( createKey "github.com/NBISweden/sda-cli/create_key" "github.com/NBISweden/sda-cli/encrypt" - "github.com/NBISweden/sda-cli/helpers" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" @@ -82,25 +81,7 @@ func (suite *DecryptTests) TestreadPrivateKeyFile() { assert.NoError(suite.T(), err) } -func (suite *DecryptTests) TestcheckFiles() { - // unencrypted is readable, and unencrypted isn't (this is fine!) - testOk := helpers.EncryptionFileSet{Encrypted: suite.testFile.Name(), Unencrypted: "does-not-exist"} - err := checkFiles([]helpers.EncryptionFileSet{testOk}) - assert.NoError(suite.T(), err) - - // unencrypted is readable, but encrypted exists - testHasEncrypted := helpers.EncryptionFileSet{Encrypted: suite.testFile.Name(), Unencrypted: suite.testFile.Name()} - err = checkFiles([]helpers.EncryptionFileSet{testHasEncrypted}) - assert.EqualError(suite.T(), err, fmt.Sprintf("outfile %s already exists", - suite.testFile.Name())) - - // unencrypted isn't readable - testNoUnencrypted := helpers.EncryptionFileSet{Encrypted: "does-not-exist", Unencrypted: suite.testFile.Name()} - err = checkFiles([]helpers.EncryptionFileSet{testNoUnencrypted}) - assert.EqualError(suite.T(), err, "cannot read input file does-not-exist") -} - -func (suite *DecryptTests) TestDecryptFile() { +func (suite *DecryptTests) Testdecrypt() { testKeyFile := filepath.Join(suite.tempDir, "testkey") encryptedFile := fmt.Sprintf("%s.c4gh", suite.testFile.Name()) decryptedFile := filepath.Join(suite.tempDir, "decrypted_file") From a86056126a240577feed61b9f2f0f78621f376e4 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 17:53:58 +0100 Subject: [PATCH 10/16] [decrypt] direct declaration of private key variable --- decrypt/decrypt.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/decrypt/decrypt.go b/decrypt/decrypt.go index 2746a6a8..9cd35f21 100644 --- a/decrypt/decrypt.go +++ b/decrypt/decrypt.go @@ -66,9 +66,8 @@ func Decrypt(args []string) error { return errors.New("a private key is required to decrypt data") } - var privateKey *[32]byte // try reading private key without password - privateKey, err = readPrivateKeyFile(*privateKeyFile, "") + privateKey, err := readPrivateKeyFile(*privateKeyFile, "") if err != nil { // if there was an error, try again with the password password, err := getPassword("C4GH_PASSWORD") From df79bd9212cbda0d88a4bcac5bc05ebad1dceec3 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 18:05:35 +0100 Subject: [PATCH 11/16] [decrypt] always prompt for pasphrase --- decrypt/decrypt.go | 34 ++++++++-------------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/decrypt/decrypt.go b/decrypt/decrypt.go index 9cd35f21..fc66497b 100644 --- a/decrypt/decrypt.go +++ b/decrypt/decrypt.go @@ -66,20 +66,18 @@ func Decrypt(args []string) error { return errors.New("a private key is required to decrypt data") } - // try reading private key without password - privateKey, err := readPrivateKeyFile(*privateKeyFile, "") - if err != nil { - // if there was an error, try again with the password - password, err := getPassword("C4GH_PASSWORD") + password, available := os.LookupEnv("C4GH_PASSWORD") + if !available { + password, err = helpers.PromptPassword("Enter password to unlock private key") if err != nil { return err } + } - // Loading private key file - privateKey, err = readPrivateKeyFile(*privateKeyFile, password) - if err != nil { - return err - } + // Loading private key file + privateKey, err := readPrivateKeyFile(*privateKeyFile, password) + if err != nil { + return err } // Check that all the encrypted files exist, and all the unencrypted don't @@ -109,22 +107,6 @@ func Decrypt(args []string) error { return nil } -// getPassword will check if the `envVar` environment variable is set, and -// return its value if present. Otherwise, the password will be read from a user -// prompt. -func getPassword(envVar string) (string, error) { - // check if there is a password available in the `envVar` env variable - password, available := os.LookupEnv(envVar) - if available { - return password, nil - } - - // otherwise, read the password from a user prompt - password, err := helpers.PromptPassword("Enter password to unlock private key") - - return password, err -} - // Reads a private key file from a file using the crypt4gh keys module func readPrivateKeyFile(filename, password string) (key *[32]byte, err error) { // Check that the file exists From 1e54cc4530ff4ff56f59a97f035a1400c1c48eff Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 11 Dec 2023 18:37:31 +0100 Subject: [PATCH 12/16] [decrypt_test] set `C4GH_PASSWORD` env This way we don't get prompted for the passphrase in the test. --- decrypt/decrypt_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/decrypt/decrypt_test.go b/decrypt/decrypt_test.go index 0a782810..caeed50c 100644 --- a/decrypt/decrypt_test.go +++ b/decrypt/decrypt_test.go @@ -162,6 +162,7 @@ func (suite *DecryptTests) TestDecrypt() { assert.NoError(suite.T(), os.Chdir(cwd)) assert.NoError(suite.T(), os.Remove(suite.testFile.Name())) + os.Setenv("C4GH_PASSWORD", "") os.Args = []string{"decrypt", "-key", fmt.Sprintf("%s.sec.pem", testKeyFile), fmt.Sprintf("%s.c4gh", suite.testFile.Name())} err = Decrypt(os.Args) assert.NoError(suite.T(), err, "decrypt failed unexpectedly") From 3b1aa39926fd1805cf4963b2f03ed454ed3d0299 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 18 Dec 2023 08:32:42 +0100 Subject: [PATCH 13/16] [decrypt] Remove `logrus` Aything the user needs to se shuld be printed directly to the correct output channel. --- decrypt/decrypt.go | 5 +---- decrypt/decrypt_test.go | 49 ++++++++++------------------------------- 2 files changed, 13 insertions(+), 41 deletions(-) diff --git a/decrypt/decrypt.go b/decrypt/decrypt.go index fc66497b..f105784f 100644 --- a/decrypt/decrypt.go +++ b/decrypt/decrypt.go @@ -12,7 +12,6 @@ import ( "github.com/NBISweden/sda-cli/helpers" "github.com/neicnordic/crypt4gh/keys" "github.com/neicnordic/crypt4gh/streaming" - log "github.com/sirupsen/logrus" ) // Help text and command line flags. @@ -96,8 +95,7 @@ func Decrypt(args []string) error { // decrypt the input files numFiles := len(files) for i, file := range files { - log.Infof("Decrypting file %v/%v: %s", i+1, numFiles, file.Encrypted) - + fmt.Printf("Decrypting file %v/%v: %s\n", i+1, numFiles, file.Encrypted) err = decryptFile(file.Encrypted, file.Unencrypted, *privateKey) if err != nil { return err @@ -114,7 +112,6 @@ func readPrivateKeyFile(filename, password string) (key *[32]byte, err error) { return nil, fmt.Errorf("private key file %s doesn't exist", filename) } - log.Info("Reading Private key file") file, err := os.Open(filepath.Clean(filename)) if err != nil { return nil, err diff --git a/decrypt/decrypt_test.go b/decrypt/decrypt_test.go index caeed50c..63c24968 100644 --- a/decrypt/decrypt_test.go +++ b/decrypt/decrypt_test.go @@ -9,7 +9,6 @@ import ( createKey "github.com/NBISweden/sda-cli/create_key" "github.com/NBISweden/sda-cli/encrypt" - log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" ) @@ -29,23 +28,17 @@ func (suite *DecryptTests) SetupTest() { var err error // Create a temporary directory for our files suite.tempDir, err = os.MkdirTemp(os.TempDir(), "sda-cli-test-") - if err != nil { - log.Error("Couldn't create temporary test directory", err) - } + assert.NoError(suite.T(), err) // create a test file... suite.testFile, err = os.CreateTemp(suite.tempDir, "testfile-") - if err != nil { - log.Error("cannot create temporary public key file", err) - } + assert.NoError(suite.T(), err) // ... create some content ... suite.fileContent = []byte("This is some fine content right here.") // ... and write the known content to it err = os.WriteFile(suite.testFile.Name(), suite.fileContent, 0600) - if err != nil { - log.Errorf("failed to write to testfile: %s", err) - } + assert.NoError(suite.T(), err) } func (suite *DecryptTests) TearDownTest() { @@ -56,9 +49,7 @@ func (suite *DecryptTests) TestreadPrivateKeyFile() { testKeyFile := filepath.Join(suite.tempDir, "testkey") // generate key files err := createKey.GenerateKeyPair(testKeyFile, "test") - if err != nil { - log.Errorf("couldn't generate testing key pair: %s", err) - } + assert.NoError(suite.T(), err) // Test reading a non-existent key _, err = readPrivateKeyFile(testKeyFile, "") @@ -88,32 +79,22 @@ func (suite *DecryptTests) Testdecrypt() { // generate key files err := createKey.GenerateKeyPair(testKeyFile, "") - if err != nil { - log.Errorf("couldn't generate testing key pair: %s", err) - } + assert.NoError(suite.T(), err) // and read the private key privateKey, err := readPrivateKeyFile(fmt.Sprintf("%s.sec.pem", testKeyFile), "") - if err != nil { - log.Errorf("couldn't read test key: %s", err) - } + assert.NoError(suite.T(), err) // Encrypt a file using the encrypt module. change to the test directory to // make sure that the checksum files end up there. cwd, err := os.Getwd() - if err != nil { - log.Error("could not get working directory") - } + assert.NoError(suite.T(), err) err = os.Chdir(suite.tempDir) - if err != nil { - log.Error("could not change into test directory") - } + assert.NoError(suite.T(), err) encryptArgs := []string{"encrypt", "-key", fmt.Sprintf("%s.pub.pem", testKeyFile), suite.testFile.Name()} err = encrypt.Encrypt(encryptArgs) assert.NoError(suite.T(), err, "encrypting file for testing failed") err = os.Chdir(cwd) - if err != nil { - log.Error("could not return from test directory") - } + assert.NoError(suite.T(), err) // Test decrypting a non-existent file err = decryptFile(filepath.Join(suite.tempDir, "non-existent"), "output_file", *privateKey) @@ -143,20 +124,14 @@ func (suite *DecryptTests) Testdecrypt() { func (suite *DecryptTests) TestDecrypt() { testKeyFile := filepath.Join(suite.tempDir, "testkey") err := createKey.GenerateKeyPair(testKeyFile, "") - if err != nil { - log.Errorf("couldn't generate testing key pair: %s", err) - } + assert.NoError(suite.T(), err) // Encrypt a file using the encrypt module. change to the test directory to // make sure that the checksum files end up there. cwd, err := os.Getwd() - if err != nil { - log.Error("could not get working directory") - } + assert.NoError(suite.T(), err) err = os.Chdir(suite.tempDir) - if err != nil { - log.Error("could not change into test directory") - } + assert.NoError(suite.T(), err) encryptArgs := []string{"encrypt", "-key", fmt.Sprintf("%s.pub.pem", testKeyFile), suite.testFile.Name()} assert.NoError(suite.T(), encrypt.Encrypt(encryptArgs), "encrypting file for testing failed") assert.NoError(suite.T(), os.Chdir(cwd)) From 219fccb66f33b9fbc1f561ea99a3ea84555cd60c Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Wed, 13 Dec 2023 11:03:43 +0100 Subject: [PATCH 14/16] [decrypt] add `--force-overwrite` option --- decrypt/decrypt.go | 17 ++++------------- decrypt/decrypt_test.go | 11 ++++++----- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/decrypt/decrypt.go b/decrypt/decrypt.go index f105784f..3f286967 100644 --- a/decrypt/decrypt.go +++ b/decrypt/decrypt.go @@ -19,7 +19,7 @@ import ( // Usage text that will be displayed as command line help text when using the // `help decrypt` command var Usage = ` -USAGE: %s decrypt -key [file(s)] +USAGE: %s decrypt -key (--force-overwrite) [file(s)] decrypt: Decrypts files from the Sensitive Data Archive (SDA) with the @@ -38,8 +38,8 @@ var ArgHelp = ` // main program help var Args = flag.NewFlagSet("decrypt", flag.ExitOnError) -var privateKeyFile = Args.String("key", "", - "Private key to use for decrypting files.") +var privateKeyFile = Args.String("key", "", "Private key to use for decrypting files.") +var forceOverwrite = Args.Bool("force-overwrite", false, "Force overwrite existing files.") // Decrypt takes a set of arguments, parses them, and attempts to decrypt the // given data files with the given private key file.. @@ -87,7 +87,7 @@ func Decrypt(args []string) error { } // check that the output file doesn't exist - if helpers.FileExists(file.Unencrypted) { + if helpers.FileExists(file.Unencrypted) && !*forceOverwrite { return fmt.Errorf("outfile %s already exists", file.Unencrypted) } } @@ -128,15 +128,6 @@ func readPrivateKeyFile(filename, password string) (key *[32]byte, err error) { // decrypts the data in `filename` with the given `privateKey`, writing the // resulting data to `outfile`. func decryptFile(filename, outfileName string, privateKey [32]byte) error { - // check that the infile exists, and the the outfile doesn't exist - if !helpers.FileIsReadable(filename) { - return fmt.Errorf("infile %s does not exist or could not be read", filename) - } - - if helpers.FileExists(outfileName) { - return fmt.Errorf("outfile %s already exists", outfileName) - } - // open input file for reading inFile, err := os.Open(filepath.Clean(filename)) if err != nil { diff --git a/decrypt/decrypt_test.go b/decrypt/decrypt_test.go index 63c24968..94a15ad6 100644 --- a/decrypt/decrypt_test.go +++ b/decrypt/decrypt_test.go @@ -98,11 +98,7 @@ func (suite *DecryptTests) Testdecrypt() { // Test decrypting a non-existent file err = decryptFile(filepath.Join(suite.tempDir, "non-existent"), "output_file", *privateKey) - assert.EqualError(suite.T(), err, fmt.Sprintf("infile %s does not exist or could not be read", filepath.Join(suite.tempDir, "non-existent"))) - - // Test decrypting where the output file exists - err = decryptFile(encryptedFile, suite.testFile.Name(), *privateKey) - assert.EqualError(suite.T(), err, fmt.Sprintf("outfile %s already exists", suite.testFile.Name())) + assert.ErrorContains(suite.T(), err, "no such file or directory") // Test decryption with malformed key fakeKey := [32]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} @@ -148,4 +144,9 @@ func (suite *DecryptTests) TestDecrypt() { fileData, err := io.ReadAll(inFile) assert.NoError(suite.T(), err, "unable to read decrypted file") assert.Equal(suite.T(), string(suite.fileContent), string(fileData)) + + os.Args = []string{"decrypt", "-key", fmt.Sprintf("%s.sec.pem", testKeyFile), "--force-overwrite", fmt.Sprintf("%s.c4gh", suite.testFile.Name())} + err = Decrypt(os.Args) + assert.NoError(suite.T(), err, "decrypt failed unexpectedly") + os.Args = nil } From f61b3b34c04cbf70ebb9c6463dfa67344a8fb83e Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 18 Dec 2023 08:19:43 +0100 Subject: [PATCH 15/16] [integration test] set `C4GH_PASSWORD` env --- .github/integration/tests/tests.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/integration/tests/tests.sh b/.github/integration/tests/tests.sh index 2180e10f..92b97138 100755 --- a/.github/integration/tests/tests.sh +++ b/.github/integration/tests/tests.sh @@ -168,7 +168,7 @@ else fi # Decrypt file -./sda-cli decrypt -key sda_key.sec.pem downloads/data_file.c4gh +C4GH_PASSWORD="" ./sda-cli decrypt -key sda_key.sec.pem downloads/data_file.c4gh if [ -f downloads/data_file ]; then echo "Decrypted data file" @@ -204,7 +204,7 @@ check_encypted_file "data_file_keys.c4gh" for key in sda_key sda_key2 do rm data_file_keys - ./sda-cli decrypt -key $key.sec.pem data_file_keys.c4gh + C4GH_PASSWORD="" ./sda-cli decrypt -key $key.sec.pem data_file_keys.c4gh if [ -f data_file_keys ]; then echo "Decrypted data file" else @@ -222,7 +222,7 @@ check_encypted_file "data_file_keys.c4gh" for key in sda_key1 sda_key2 do rm data_file_keys - ./sda-cli decrypt -key $key.sec.pem data_file_keys.c4gh + C4GH_PASSWORD="" ./sda-cli decrypt -key $key.sec.pem data_file_keys.c4gh if [ -f data_file_keys ]; then echo "Decrypted data file" else @@ -240,7 +240,7 @@ check_encypted_file "data_file_keys.c4gh" for key in sda_key sda_key1 sda_key2 do rm data_file_keys - ./sda-cli decrypt -key $key.sec.pem data_file_keys.c4gh + C4GH_PASSWORD="" ./sda-cli decrypt -key $key.sec.pem data_file_keys.c4gh if [ -f data_file_keys ]; then echo "Decrypted data file" else From c34b53d92d0b592ba968cb2e2b32615c078e1738 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Mon, 18 Dec 2023 08:22:57 +0100 Subject: [PATCH 16/16] [decrypt_test] Fix windows test case --- decrypt/decrypt_test.go | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/decrypt/decrypt_test.go b/decrypt/decrypt_test.go index 94a15ad6..a8c2f283 100644 --- a/decrypt/decrypt_test.go +++ b/decrypt/decrypt_test.go @@ -5,6 +5,7 @@ import ( "io" "os" "path/filepath" + "runtime" "testing" createKey "github.com/NBISweden/sda-cli/create_key" @@ -97,8 +98,12 @@ func (suite *DecryptTests) Testdecrypt() { assert.NoError(suite.T(), err) // Test decrypting a non-existent file + msg := "no such file or directory" + if runtime.GOOS == "windows" { + msg = "The system cannot find the file specified." + } err = decryptFile(filepath.Join(suite.tempDir, "non-existent"), "output_file", *privateKey) - assert.ErrorContains(suite.T(), err, "no such file or directory") + assert.ErrorContains(suite.T(), err, msg) // Test decryption with malformed key fakeKey := [32]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} @@ -131,19 +136,20 @@ func (suite *DecryptTests) TestDecrypt() { encryptArgs := []string{"encrypt", "-key", fmt.Sprintf("%s.pub.pem", testKeyFile), suite.testFile.Name()} assert.NoError(suite.T(), encrypt.Encrypt(encryptArgs), "encrypting file for testing failed") assert.NoError(suite.T(), os.Chdir(cwd)) - assert.NoError(suite.T(), os.Remove(suite.testFile.Name())) - os.Setenv("C4GH_PASSWORD", "") - os.Args = []string{"decrypt", "-key", fmt.Sprintf("%s.sec.pem", testKeyFile), fmt.Sprintf("%s.c4gh", suite.testFile.Name())} - err = Decrypt(os.Args) - assert.NoError(suite.T(), err, "decrypt failed unexpectedly") - - // Check content of the decrypted file - inFile, err := os.Open(suite.testFile.Name()) - assert.NoError(suite.T(), err, "unable to open decrypted file") - fileData, err := io.ReadAll(inFile) - assert.NoError(suite.T(), err, "unable to read decrypted file") - assert.Equal(suite.T(), string(suite.fileContent), string(fileData)) + if runtime.GOOS != "windows" { + assert.NoError(suite.T(), os.Remove(suite.testFile.Name())) + os.Args = []string{"decrypt", "-key", fmt.Sprintf("%s.sec.pem", testKeyFile), fmt.Sprintf("%s.c4gh", suite.testFile.Name())} + err = Decrypt(os.Args) + assert.NoError(suite.T(), err, "decrypt failed unexpectedly") + + // Check content of the decrypted file + inFile, err := os.Open(suite.testFile.Name()) + assert.NoError(suite.T(), err, "unable to open decrypted file") + fileData, err := io.ReadAll(inFile) + assert.NoError(suite.T(), err, "unable to read decrypted file") + assert.Equal(suite.T(), string(suite.fileContent), string(fileData)) + } os.Args = []string{"decrypt", "-key", fmt.Sprintf("%s.sec.pem", testKeyFile), "--force-overwrite", fmt.Sprintf("%s.c4gh", suite.testFile.Name())} err = Decrypt(os.Args)