From b77d7882e9b7de252feef3e57aa275e3e5529334 Mon Sep 17 00:00:00 2001 From: Eric Brown Date: Tue, 2 Jan 2024 14:26:18 -0800 Subject: [PATCH] Add golang_org_x_weak_cipher and testing (#194) Signed-off-by: Eric Brown --- .../go/golang_org_x_crypto/weak_cipher.py | 155 ++++++++++++++++++ .../rules/go/golang_org_x_crypto/weak_hash.py | 4 +- setup.cfg | 5 +- .../examples/weak_cipher_blowfish.go | 21 +++ .../weak_cipher_blowfish_new_salted_cipher.go | 22 +++ .../examples/weak_cipher_cast5.go | 21 +++ .../examples/weak_cipher_tea.go | 21 +++ .../weak_cipher_tea_new_cipher_with_rounds.go | 21 +++ .../examples/weak_cipher_twofish.go | 63 +++++++ .../examples/weak_cipher_xtea.go | 21 +++ .../golang_org_x_crypto/test_weak_cipher.py | 52 ++++++ .../go/golang_org_x_crypto/test_weak_hash.py | 2 +- 12 files changed, 405 insertions(+), 3 deletions(-) create mode 100644 precli/rules/go/golang_org_x_crypto/weak_cipher.py create mode 100644 tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_blowfish.go create mode 100644 tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_blowfish_new_salted_cipher.go create mode 100644 tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_cast5.go create mode 100644 tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_tea.go create mode 100644 tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_tea_new_cipher_with_rounds.go create mode 100644 tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_twofish.go create mode 100644 tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_xtea.go create mode 100644 tests/unit/rules/go/golang_org_x_crypto/test_weak_cipher.py diff --git a/precli/rules/go/golang_org_x_crypto/weak_cipher.py b/precli/rules/go/golang_org_x_crypto/weak_cipher.py new file mode 100644 index 00000000..b5b3f389 --- /dev/null +++ b/precli/rules/go/golang_org_x_crypto/weak_cipher.py @@ -0,0 +1,155 @@ +# Copyright 2024 Secure Saurce LLC +r""" +==================================================================== +Use of a Broken or Risky Cryptographic Algorithm in X Crypto Package +==================================================================== + +Using weak ciphers for cryptographic algorithms can pose significant security +risks, and it's generally advised to avoid them in favor of stronger, more +secure algorithms. Here's some guidance that advises against using weak +ciphers like Blowfish, CAST5, TEA/XTEA, and Twofish: + +Blowfish: Developed in 1993, Blowfish is a block cipher known for its +simplicity. However, its small block size of 64 bits makes it susceptible to +birthday attacks in modern contexts. This vulnerability is significant when +encrypting large amounts of data, which is common in current applications. + +CAST5 (CAST-128): CAST5, a symmetric encryption algorithm, suffers from +similar issues as Blowfish due to its 64-bit block size. While it was +considered secure for its time, modern applications typically require +algorithms with larger block sizes for enhanced security. + +TEA/XTEA: The Tiny Encryption Algorithm (TEA) and its successor, eXtended +TEA (XTEA), are lightweight block ciphers. They are notable for their +simplicity and ease of implementation but have known vulnerabilities, +including susceptibility to differential cryptanalysis. These weaknesses +make them less suitable for applications where strong security is a priority. + +Twofish: As a finalist in the Advanced Encryption Standard (AES) competition, +Twofish is a respected algorithm. However, it was not selected as the +standard, and over time, AES has become the more tested and trusted choice +in most cryptographic applications. + +In summary, there is a consensus among reputable standards organizations, +industry experts, and security professionals that weak ciphers like Blowfish, +CAST5, TEA/XTEA, and Twofish should be avoided due to their known +vulnerabilities and weaknesses. Instead, it is advisable to use stronger, +more secure cryptographic algorithms and adhere to industry best practices +and regulatory requirements for encryption and security. + +------- +Example +------- + +.. code-block:: python + :linenos: + :emphasize-lines: 11 + + package main + + import ( + "log" + "golang.org/x/crypto/twofish" + ) + + func main() { + key := []byte("examplekey123456") + + _, err := twofish.NewCipher(key) + if err != nil { + log.Fatalf("Failed to create cipher: %v", err) + } + } + +----------- +Remediation +----------- + +It is advisable to use stronger, more secure cryptographic algorithms such as +AES. + +.. code-block:: python + :linenos: + :emphasize-lines: 5,11 + + package main + + import ( + "log" + "crypto/aes" + ) + + func main() { + key := []byte("examplekey123456") + + _, err := aes.NewCipher(key) + if err != nil { + log.Fatalf("Failed to create cipher: %v", err) + } + } + +.. seealso:: + + - `Use of a Broken or Risky Cryptographic Algorithm in Crypto Package `_ + - `blowfish package - golang.org_x_crypto_twofish - Go Packages `_ + - `cast5 package - golang.org_x_crypto_twofish - Go Packages `_ + - `tea package - golang.org_x_crypto_twofish - Go Packages `_ + - `twofish package - golang.org_x_crypto_twofish - Go Packages `_ + - `xtea package - golang.org_x_crypto_twofish - Go Packages `_ + - `CWE-327: Use of a Broken or Risky Cryptographic Algorithm `_ + +.. versionadded:: 1.0.0 + +""" # noqa: E501 +from precli.core.config import Config +from precli.core.level import Level +from precli.core.location import Location +from precli.core.result import Result +from precli.rules import Rule + + +class WeakCipher(Rule): + def __init__(self, id: str): + super().__init__( + id=id, + name="use_of_a_broken_or_risky_cryptographic_algorithm", + full_descr=__doc__, + cwe_id=327, + message="Weak ciphers like {} should be avoided due to their " + "known vulnerabilities and weaknesses.", + targets=("call"), + wildcards={}, + config=Config(enabled=False), + ) + + def analyze(self, context: dict, **kwargs: dict) -> Result: + call = kwargs.get("call") + + if call.name_qualified in [ + "golang.org/x/crypto/blowfish.NewCipher", + "golang.org/x/crypto/blowfish.NewSaltedCipher", + "golang.org/x/crypto/cast5.NewCipher", + "golang.org/x/crypto/tea.NewCipher", + "golang.org/x/crypto/tea.NewCipherWithRounds", + "golang.org/x/crypto/twofish.NewCipher", + "golang.org/x/crypto/xtea.NewCipher", + ]: + # TODO: Need to remove arguments for NewSaltedCipher and + # NewCipherWithRounds + fixes = Rule.get_fixes( + context=context, + deleted_location=Location(node=call.function_node), + description="It is advisable to use a stronger, more " + "secure cryptographic algorithm like AES.", + inserted_content="aes.NewCipher", + ) + return Result( + rule_id=self.id, + location=Location( + file_name=context["file_name"], + node=call.function_node, + ), + level=Level.ERROR, + message=self.message.format(call.name), + fixes=fixes, + ) diff --git a/precli/rules/go/golang_org_x_crypto/weak_hash.py b/precli/rules/go/golang_org_x_crypto/weak_hash.py index 412a605d..42b8ba2d 100644 --- a/precli/rules/go/golang_org_x_crypto/weak_hash.py +++ b/precli/rules/go/golang_org_x_crypto/weak_hash.py @@ -67,7 +67,7 @@ .. seealso:: - - `Reversible One Way Hash in X Crypto Package `_ + - `Reversible One Way Hash in X Crypto Package `_ - `md4 package - golang.org_x_crypto_md4 - Go Packages `_ - `ripemd160 package - golang.org_x_crypto_ripemd160 - Go Packages `_ - `CWE-328: Use of Weak Hash `_ @@ -76,6 +76,7 @@ .. versionadded:: 1.0.0 """ # noqa: E501 +from precli.core.config import Config from precli.core.level import Level from precli.core.location import Location from precli.core.result import Result @@ -93,6 +94,7 @@ def __init__(self, id: str): "expectations.", targets=("call"), wildcards={}, + config=Config(enabled=False), ) def analyze(self, context: dict, **kwargs: dict) -> Result: diff --git a/setup.cfg b/setup.cfg index 85ad1d94..184ab4e2 100644 --- a/setup.cfg +++ b/setup.cfg @@ -45,8 +45,11 @@ precli.rules.go = # precli/rules/go/golang_org_x_crypto/ssh_insecure_ignore_hostkey.py GO501 = precli.rules.go.golang_org_x_crypto.ssh_insecure_ignore_hostkey:SshInsecureIgnoreHostKey + # precli/rules/go/golang_org_x_crypto/weak_cipher.py + GO502 = precli.rules.go.golang_org_x_crypto.weak_cipher:WeakCipher + # precli/rules/go/golang_org_x_crypto/weak_hash.py - GO502 = precli.rules.go.golang_org_x_crypto.weak_hash:WeakHash + GO503 = precli.rules.go.golang_org_x_crypto.weak_hash:WeakHash precli.rules.python = # precli/rules/python/stdlib/assert/assert.py diff --git a/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_blowfish.go b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_blowfish.go new file mode 100644 index 00000000..a941087a --- /dev/null +++ b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_blowfish.go @@ -0,0 +1,21 @@ +// level: ERROR +// start_line: 17 +// end_line: 17 +// start_column: 14 +// end_column: 32 +package main + +import ( + "log" + + "golang.org/x/crypto/blowfish" +) + +func main() { + key := []byte("examplekey123456") + + _, err := blowfish.NewCipher(key) + if err != nil { + log.Fatalf("Failed to create cipher: %v", err) + } +} diff --git a/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_blowfish_new_salted_cipher.go b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_blowfish_new_salted_cipher.go new file mode 100644 index 00000000..7e6cfac8 --- /dev/null +++ b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_blowfish_new_salted_cipher.go @@ -0,0 +1,22 @@ +// level: ERROR +// start_line: 18 +// end_line: 18 +// start_column: 14 +// end_column: 38 +package main + +import ( + "log" + + "golang.org/x/crypto/blowfish" +) + +func main() { + key := []byte("examplekey123456") + salt := []byte("1234567890abcdef") + + _, err := blowfish.NewSaltedCipher(key, salt) + if err != nil { + log.Fatalf("Failed to create cipher: %v", err) + } +} diff --git a/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_cast5.go b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_cast5.go new file mode 100644 index 00000000..a0ced64e --- /dev/null +++ b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_cast5.go @@ -0,0 +1,21 @@ +// level: ERROR +// start_line: 17 +// end_line: 17 +// start_column: 14 +// end_column: 29 +package main + +import ( + "log" + + "golang.org/x/crypto/cast5" +) + +func main() { + key := []byte("examplekey123456") + + _, err := cast5.NewCipher(key) + if err != nil { + log.Fatalf("Failed to create cipher: %v", err) + } +} diff --git a/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_tea.go b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_tea.go new file mode 100644 index 00000000..571f69d2 --- /dev/null +++ b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_tea.go @@ -0,0 +1,21 @@ +// level: ERROR +// start_line: 17 +// end_line: 17 +// start_column: 14 +// end_column: 27 +package main + +import ( + "log" + + "golang.org/x/crypto/tea" +) + +func main() { + key := []byte("examplekey123456") + + _, err := tea.NewCipher(key) + if err != nil { + log.Fatalf("Failed to create cipher: %v", err) + } +} diff --git a/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_tea_new_cipher_with_rounds.go b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_tea_new_cipher_with_rounds.go new file mode 100644 index 00000000..11cceaa9 --- /dev/null +++ b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_tea_new_cipher_with_rounds.go @@ -0,0 +1,21 @@ +// level: ERROR +// start_line: 17 +// end_line: 17 +// start_column: 14 +// end_column: 37 +package main + +import ( + "log" + + "golang.org/x/crypto/tea" +) + +func main() { + key := []byte("examplekey123456") + + _, err := tea.NewCipherWithRounds(key, 64) + if err != nil { + log.Fatalf("Failed to create cipher: %v", err) + } +} diff --git a/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_twofish.go b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_twofish.go new file mode 100644 index 00000000..bd016ecd --- /dev/null +++ b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_twofish.go @@ -0,0 +1,63 @@ +// level: ERROR +// start_line: 35 +// end_line: 35 +// start_column: 18 +// end_column: 35 +package main + +import ( + "crypto/cipher" + "encoding/hex" + "fmt" + "golang.org/x/crypto/twofish" + "log" +) + +// pkcs7Pad pads the plaintext to be a multiple of the block size +func pkcs7Pad(plaintext []byte, blockSize int) []byte { + padding := blockSize - len(plaintext)%blockSize + padtext := bytes.Repeat([]byte{byte(padding)}, padding) + return append(plaintext, padtext...) +} + +// pkcs7Unpad removes the padding from the plaintext +func pkcs7Unpad(plaintext []byte) []byte { + length := len(plaintext) + padLen := int(plaintext[length-1]) + return plaintext[:(length - padLen)] +} + +func main() { + // Twofish key (can be 16, 24, or 32 bytes) + key := []byte("examplekey123456") // 16 bytes for a 128-bit key + + // Create a new Twofish cipher block with the key + block, err := twofish.NewCipher(key) + if err != nil { + log.Fatalf("Failed to create cipher: %v", err) + } + + // The plaintext that needs to be encrypted + plaintext := []byte("Hello, Twofish!") + + // Pad plaintext to be a multiple of the block size + paddedPlaintext := pkcs7Pad(plaintext, twofish.BlockSize) + + ciphertext := make([]byte, len(paddedPlaintext)) + for i := 0; i < len(paddedPlaintext); i += twofish.BlockSize { + block.Encrypt(ciphertext[i:i+twofish.BlockSize], paddedPlaintext[i:i+twofish.BlockSize]) + } + + fmt.Printf("Ciphertext: %x\n", ciphertext) + + // Decrypting the ciphertext + decrypted := make([]byte, len(ciphertext)) + for i := 0; i < len(ciphertext); i += twofish.BlockSize { + block.Decrypt(decrypted[i:i+twofish.BlockSize], ciphertext[i:i+twofish.BlockSize]) + } + + // Remove padding + decrypted = pkcs7Unpad(decrypted) + + fmt.Printf("Decrypted: %s\n", decrypted) +} diff --git a/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_xtea.go b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_xtea.go new file mode 100644 index 00000000..d3808302 --- /dev/null +++ b/tests/unit/rules/go/golang_org_x_crypto/examples/weak_cipher_xtea.go @@ -0,0 +1,21 @@ +// level: ERROR +// start_line: 17 +// end_line: 17 +// start_column: 14 +// end_column: 28 +package main + +import ( + "log" + + "golang.org/x/crypto/xtea" +) + +func main() { + key := []byte("examplekey123456") + + _, err := xtea.NewCipher(key) + if err != nil { + log.Fatalf("Failed to create cipher: %v", err) + } +} diff --git a/tests/unit/rules/go/golang_org_x_crypto/test_weak_cipher.py b/tests/unit/rules/go/golang_org_x_crypto/test_weak_cipher.py new file mode 100644 index 00000000..dcddae39 --- /dev/null +++ b/tests/unit/rules/go/golang_org_x_crypto/test_weak_cipher.py @@ -0,0 +1,52 @@ +# Copyright 2024 Secure Saurce LLC +import os + +from parameterized import parameterized + +from precli.core.level import Level +from precli.parsers import go +from precli.rules import Rule +from tests.unit.rules import test_case + + +class CryptoWeakHashTests(test_case.TestCase): + def setUp(self): + super().setUp() + self.rule_id = "GO502" + self.parser = go.Go(enabled=[self.rule_id]) + self.base_path = os.path.join( + "tests", + "unit", + "rules", + "go", + "golang_org_x_crypto", + "examples", + ) + + def test_rule_meta(self): + rule = Rule.get_by_id(self.rule_id) + self.assertEqual(self.rule_id, rule.id) + self.assertEqual( + "use_of_a_broken_or_risky_cryptographic_algorithm", rule.name + ) + self.assertEqual( + f"https://docs.securesauce.dev/rules/{self.rule_id}", rule.help_url + ) + self.assertEqual(True, rule.default_config.enabled) + self.assertEqual(Level.WARNING, rule.default_config.level) + self.assertEqual(-1.0, rule.default_config.rank) + self.assertEqual("327", rule.cwe.cwe_id) + + @parameterized.expand( + [ + "weak_cipher_blowfish.go", + "weak_cipher_blowfish_new_salted_cipher.go", + "weak_cipher_cast5.go", + "weak_cipher_tea.go", + "weak_cipher_tea_new_cipher_with_rounds.go", + "weak_cipher_twofish.go", + "weak_cipher_xtea.go", + ] + ) + def test(self, filename): + self.check(filename) diff --git a/tests/unit/rules/go/golang_org_x_crypto/test_weak_hash.py b/tests/unit/rules/go/golang_org_x_crypto/test_weak_hash.py index 5217f35c..ff41ece1 100644 --- a/tests/unit/rules/go/golang_org_x_crypto/test_weak_hash.py +++ b/tests/unit/rules/go/golang_org_x_crypto/test_weak_hash.py @@ -12,7 +12,7 @@ class CryptoWeakHashTests(test_case.TestCase): def setUp(self): super().setUp() - self.rule_id = "GO502" + self.rule_id = "GO503" self.parser = go.Go(enabled=[self.rule_id]) self.base_path = os.path.join( "tests",