Merge pull request #85 from neicnordic/keys_no_panic

Do not panic on unexpected key format

internal/version/version.go

@@ -7,7 +7,7 @@ import (
 )
 
 // The version in the current branch
-var Version = "1.8.0"
+var Version = "1.8.1"
 
 // If this is "" (empty string) then it means that it is a final release.
 // Otherwise, this is a pre-release e.g. "dev", "beta", "rc1", etc.

keys/keys.go

@@ -95,6 +95,11 @@ func ReadPrivateKey(reader io.Reader, passPhrase []byte) (privateKey [chacha20po
 	// Not OpenSSH private key, assuming OpenSSL private key, trying to figure out type (Ed25519 or X25519)
 	block, _ := pem.Decode(allBytes)
 
+	if block == nil {
+		return [chacha20poly1305.KeySize]byte{}, fmt.Errorf("Read of unrecognized private key format failed; " +
+			"expected PEM encoded key")
+	}
+
 	var openSSLPrivateKey openSSLPrivateKey
 	if _, err = asn1.Unmarshal(block.Bytes, &openSSLPrivateKey); err == nil {
 		// Trying to read OpenSSL Ed25519 private key and convert to X25519 private key
@@ -115,7 +120,7 @@ func ReadPrivateKey(reader io.Reader, passPhrase []byte) (privateKey [chacha20po
 	}
 
 	// Interpreting bytes as Crypt4GH private key bytes (https://crypt4gh.readthedocs.io/en/latest/keys.html)
-	if string(block.Bytes[:7]) == magic {
+	if len(block.Bytes) > 8 && string(block.Bytes[:7]) == magic {
 		return readCrypt4GHPrivateKey(block.Bytes, passPhrase)
 	}
 
@@ -233,6 +238,12 @@ func ReadPublicKey(reader io.Reader) (publicKey [chacha20poly1305.KeySize]byte,
 
 	// Not OpenSSH public key, assuming OpenSSL public key
 	block, _ := pem.Decode(allBytes)
+
+	if block == nil {
+		return [chacha20poly1305.KeySize]byte{}, fmt.Errorf("Read of unrecognized public key format failed; " +
+			"expected PEM encoded key")
+	}
+
 	var openSSLPublicKey openSSLPublicKey
 	if _, err = asn1.Unmarshal(block.Bytes, &openSSLPublicKey); err == nil {
 		// Trying to read OpenSSL Ed25519 public key and convert to X25519 public key
@@ -250,6 +261,9 @@ func ReadPublicKey(reader io.Reader) (publicKey [chacha20poly1305.KeySize]byte,
 		}
 	}
 
+	if len(block.Bytes) < chacha20poly1305.KeySize {
+		return publicKey, fmt.Errorf("Unsupported key file format")
+	}
 	// Interpreting bytes as Crypt4GH public key bytes (X25519)
 	copy(publicKey[:], block.Bytes[len(block.Bytes)-chacha20poly1305.KeySize:])
 

keys/keys_test.go

@@ -18,7 +18,10 @@ const crypt4ghX25519Pub = `-----BEGIN CRYPT4GH PUBLIC KEY-----
 y67skGFKqYN+0n+1P0FyxYa/lHPUWiloN4kdrx7J3BA=
 -----END CRYPT4GH PUBLIC KEY-----
 `
-
+const badPEM = `-----BEGIN SOMETHING-----
+y67s
+-----END SOMETHING-----
+`
 const sshEd25519SecEnc = `-----BEGIN OPENSSH PRIVATE KEY-----
 b3BlbnNzaC1rZXktdjEAAAAACmFlczI1Ni1jdHIAAAAGYmNyeXB0AAAAGAAAABCKYb3joJ
 xaRg4JDkveDbaTAAAAEAAAAAEAAAAzAAAAC3NzaC1lZDI1NTE5AAAAIA65hmgJeJakva2c
@@ -173,13 +176,17 @@ func TestReadKey(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 
+			var err error
 			var key [chacha20poly1305.KeySize]byte
 			if test.readPrivateKeyFunction != nil {
-				key, _ = test.readPrivateKeyFunction(strings.NewReader(test.content), test.passPhrase)
+				key, err = test.readPrivateKeyFunction(strings.NewReader(test.content), test.passPhrase)
 			} else {
-				key, _ = test.readPublicKeyFunction(strings.NewReader(test.content))
+				key, err = test.readPublicKeyFunction(strings.NewReader(test.content))
 			}
 
+			if err != nil {
+				t.Errorf("Unexpected error: %v for test %s", err, test.name)
+			}
 			if hex.EncodeToString(key[:]) != test.hash {
 				t.Fail()
 			}
@@ -318,3 +325,27 @@ func TestGenerateSharedKey(t *testing.T) {
 		})
 	}
 }
+
+func TestReadBrokenKey(t *testing.T) {
+
+	if _, err := ReadPrivateKey(strings.NewReader("error"), nil); err == nil {
+		t.Errorf("Didn't get an error on a faulty private key")
+	}
+
+	if _, err := ReadPublicKey(strings.NewReader("error")); err == nil {
+		t.Errorf("Didn't get an error on a faulty public key")
+	}
+
+	if _, err := ReadPrivateKey(strings.NewReader(badPEM), nil); err == nil {
+		t.Errorf("Didn't get an error on a faulty private key")
+	}
+
+	if _, err := ReadPublicKey(strings.NewReader(badPEM)); err == nil {
+		t.Errorf("Didn't get an error on a faulty public key")
+	}
+
+	if _, err := ReadPrivateKey(strings.NewReader(crypt4ghX25519Sec), nil); err == nil {
+		t.Errorf("Didn't get an error on an encrypted private key without password")
+	}
+
+}