-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add security rules for RSA key sizes and hard-coded secrets in Ruby and Rust #110
base: main
Are you sure you want to change the base?
Conversation
Sakshis seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughThis pull request introduces several new security rules across Ruby and Rust languages to enhance the detection of insufficient RSA key sizes and hard-coded secrets in applications. Specifically, it adds rules for identifying RSA key generation with inadequate sizes in Ruby, detecting hard-coded secrets in Ruby applications using Cassandra, and recognizing hard-coded passwords in Rust applications. Additionally, it includes corresponding test cases and snapshots to validate these rules and ensure proper functioning. Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (11)
tests/rust/hardcoded-password-rust-test.yml (1)
1-54
: Consider adding more test scenarios.While the current test cases cover basic MySQL and PostgreSQL connections, consider adding tests for:
- Connection strings with embedded credentials
- Connection pools with hardcoded credentials
- Different credential formats (base64, hex, etc.)
rules/rust/security/hardcoded-password-rust.yml (1)
308-312
: Consider expanding database driver coverage.The rule currently covers MySQL and PostgreSQL. Consider adding patterns for:
- SQLite
- MongoDB
- Redis
- Other popular Rust database drivers
tests/ruby/insufficient-rsa-key-size-ruby-test.yml (1)
1-7
: Consider adding more test cases for comprehensive coverage.While the current test cases cover the basic valid (2048 bits) and invalid (204 bits) scenarios, consider adding:
- Edge case: 2047 bits (just below minimum)
- Edge case: 0 bits
- Edge case: negative bits
- Common mistake: 1024 bits (previously common but now insufficient)
id: insufficient-rsa-key-size-ruby valid: - | key = OpenSSL::PKey::RSA.new(2048) invalid: - | key = OpenSSL::PKey::RSA.new(204) + - | + key = OpenSSL::PKey::RSA.new(2047) + - | + key = OpenSSL::PKey::RSA.new(0) + - | + key = OpenSSL::PKey::RSA.new(-1) + - | + key = OpenSSL::PKey::RSA.new(1024)rules/ruby/security/insufficient-rsa-key-size-ruby.yml (3)
4-6
: Fix typo in error message.The error message contains a typo: "insufficent" should be "insufficient".
message: >- - The RSA key size $SIZE is insufficent by NIST standards. It is + The RSA key size $SIZE is insufficient by NIST standards. It is recommended to use a key length of 2048 or higher.
11-83
: Consider adding checks for common RSA key generation patterns.The current rule covers direct RSA key generation and assignment, but consider adding checks for:
- Key generation through configuration files
- Dynamic key size calculation
- Key size specified in environment variables
Would you like me to provide examples of these additional patterns?
7-10
: Enhance security documentation references.While the CWE reference is good, consider adding:
- Link to OWASP Cryptographic Storage Cheat Sheet
- Reference to FIPS 140-2 requirements
- Link to current BSI recommendations
note: >- [CWE-326] Inadequate Encryption Strength. [REFERENCES] - https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-57Pt3r1.pdf + - https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html + - https://csrc.nist.gov/publications/detail/fips/140/2/final + - https://www.bsi.bund.de/EN/Service-Navi/Publications/TechnicalGuidelines/tr02102/tr02102_node.htmltests/ruby/ruby-cassandra-hardcoded-secret-ruby-test.yml (2)
2-4
: Consider expanding valid test cases for better coverage.While the empty password case is valid, consider adding more valid cases such as:
- Using environment variables (e.g.,
ENV['CASSANDRA_PASSWORD']
)- Using a secure credentials manager
- Using a configuration file that's not in version control
5-12
: Consider additional invalid test cases for comprehensive detection.The current invalid cases cover direct hardcoding and variable assignment. Consider adding cases for:
- String interpolation (e.g.,
password: "pwd_#{suffix}"
)- Base64 encoded passwords
- Concatenated strings
rules/ruby/security/ruby-cassandra-hardcoded-secret-ruby.yml (3)
4-13
: Enhance security message with specific remediation steps.The warning message is good but could be more actionable. Consider adding:
- Specific examples of using environment variables
- Reference to Ruby-specific secure credential management solutions
- Link to Ruby-specific security best practices
14-53
: Consider additional security checks in Cassandra.cluster() pattern.The current pattern checks for hardcoded passwords but could be enhanced to:
- Detect other sensitive parameters (e.g., certificates, tokens)
- Check for common password patterns (e.g., test/dev credentials)
- Validate the presence of SSL/TLS configuration
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 17-17: wrong indentation: expected 4 but found 3
(indentation)
[warning] 37-37: wrong indentation: expected 13 but found 12
(indentation)
[warning] 48-48: wrong indentation: expected 9 but found 11
(indentation)
[warning] 51-51: wrong indentation: expected 13 but found 15
(indentation)
91-122
: Simplify nested pattern matching logic.The current nested pattern matching for variable assignments is complex and could be simplified for better maintainability.
Consider restructuring to:
any: - pattern: | $SECRET = "..." Cassandra.cluster(..., password: $SECRET, ...) - pattern: | $SECRET = "..." $OTHER = $SECRET Cassandra.cluster(..., password: $OTHER, ...)🧰 Tools
🪛 yamllint (1.35.1)
[warning] 92-92: wrong indentation: expected 9 but found 8
(indentation)
[warning] 93-93: wrong indentation: expected 12 but found 15
(indentation)
[warning] 96-96: wrong indentation: expected 17 but found 16
(indentation)
[warning] 97-97: wrong indentation: expected 20 but found 22
(indentation)
[warning] 98-98: too many spaces after colon
(colons)
[warning] 101-101: wrong indentation: expected 20 but found 22
(indentation)
[warning] 104-104: wrong indentation: expected 24 but found 30
(indentation)
[warning] 107-107: wrong indentation: expected 12 but found 14
(indentation)
[warning] 110-110: wrong indentation: expected 16 but found 15
(indentation)
[warning] 113-113: wrong indentation: expected 17 but found 16
(indentation)
[warning] 114-114: wrong indentation: expected 20 but found 22
(indentation)
[warning] 115-115: too many spaces after colon
(colons)
[warning] 118-118: wrong indentation: expected 20 but found 22
(indentation)
[warning] 121-121: wrong indentation: expected 24 but found 30
(indentation)
[error] 122-122: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
rules/ruby/security/insufficient-rsa-key-size-ruby.yml
(1 hunks)rules/ruby/security/ruby-cassandra-hardcoded-secret-ruby.yml
(1 hunks)rules/rust/security/hardcoded-password-rust.yml
(1 hunks)tests/__snapshots__/hardcoded-password-rust-snapshot.yml
(1 hunks)tests/__snapshots__/insufficient-rsa-key-size-ruby-snapshot.yml
(1 hunks)tests/__snapshots__/ruby-cassandra-hardcoded-secret-ruby-snapshot.yml
(1 hunks)tests/ruby/insufficient-rsa-key-size-ruby-test.yml
(1 hunks)tests/ruby/ruby-cassandra-hardcoded-secret-ruby-test.yml
(1 hunks)tests/rust/hardcoded-password-rust-test.yml
(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
rules/ruby/security/ruby-cassandra-hardcoded-secret-ruby.yml
[warning] 17-17: wrong indentation: expected 4 but found 3
(indentation)
[warning] 37-37: wrong indentation: expected 13 but found 12
(indentation)
[warning] 48-48: wrong indentation: expected 9 but found 11
(indentation)
[warning] 51-51: wrong indentation: expected 13 but found 15
(indentation)
[warning] 56-56: wrong indentation: expected 4 but found 3
(indentation)
[warning] 76-76: wrong indentation: expected 13 but found 12
(indentation)
[warning] 85-85: wrong indentation: expected 9 but found 11
(indentation)
[warning] 88-88: wrong indentation: expected 13 but found 15
(indentation)
[warning] 92-92: wrong indentation: expected 9 but found 8
(indentation)
[warning] 93-93: wrong indentation: expected 12 but found 15
(indentation)
[warning] 96-96: wrong indentation: expected 17 but found 16
(indentation)
[warning] 97-97: wrong indentation: expected 20 but found 22
(indentation)
[warning] 98-98: too many spaces after colon
(colons)
[warning] 101-101: wrong indentation: expected 20 but found 22
(indentation)
[warning] 104-104: wrong indentation: expected 24 but found 30
(indentation)
[warning] 107-107: wrong indentation: expected 12 but found 14
(indentation)
[warning] 110-110: wrong indentation: expected 16 but found 15
(indentation)
[warning] 113-113: wrong indentation: expected 17 but found 16
(indentation)
[warning] 114-114: wrong indentation: expected 20 but found 22
(indentation)
[warning] 115-115: too many spaces after colon
(colons)
[warning] 118-118: wrong indentation: expected 20 but found 22
(indentation)
[warning] 121-121: wrong indentation: expected 24 but found 30
(indentation)
[error] 122-122: trailing spaces
(trailing-spaces)
🔇 Additional comments (8)
tests/rust/hardcoded-password-rust-test.yml (1)
4-13
: LGTM! Good use of environment variables for sensitive data.
The valid test case correctly demonstrates the use of environment variables for password management.
rules/rust/security/hardcoded-password-rust.yml (1)
1-16
: LGTM! Well-documented rule with proper security references.
The rule includes relevant CWE and OWASP references, and clearly explains the security implications.
tests/__snapshots__/hardcoded-password-rust-snapshot.yml (2)
1-654
: LGTM! Comprehensive test snapshots.
The snapshots provide detailed AST matching labels that align with the test cases and rule patterns.
1-654
: Verify rule effectiveness with real-world code patterns.
Let's verify the rule against common database connection patterns in the wild:
✅ Verification successful
Rule implementation effectively captures database connection patterns
The verification results show that the rule correctly identifies hardcoded password patterns in both MySQL and PostgreSQL database connections:
- Captures the direct connection pattern:
MySqlConnectOptions::new()
.password("password")
- Captures the indirect connection pattern:
let pg = PgConnectOptions::new();
pg.password("secret-password")
- Test cases demonstrate both valid (using environment variables) and invalid (hardcoded) patterns:
.password(env!("pwd")) // Valid
.password("password") // Invalid
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for database connection patterns in the codebase
echo "Searching for database connection patterns..."
# Look for MySQL connection patterns
echo "MySQL patterns:"
rg -U 'MySqlConnectOptions::new\(\)[\s\S]*?\.password\('
# Look for PostgreSQL connection patterns
echo "PostgreSQL patterns:"
rg -U 'PgConnectOptions::new\(\)[\s\S]*?\.password\('
# Look for potential connection strings
echo "Connection string patterns:"
rg -U '(?i)(?:mysql|postgres(?:ql)?):\/\/[^:]+:[^@]+@'
Length of output: 25911
tests/__snapshots__/insufficient-rsa-key-size-ruby-snapshot.yml (1)
1-29
: LGTM! Snapshot correctly captures the test state.
The snapshot properly captures all necessary components with appropriate labels for error highlighting.
rules/ruby/security/insufficient-rsa-key-size-ruby.yml (1)
90-91
: Verify the key size constraint regex.
The regex for key size validation is complex and critical for security. Let's verify it catches all cases correctly:
✅ Verification successful
The RSA key size constraint regex is working correctly
The regex validation works as intended for RSA key sizes:
- Correctly accepts 2048 (minimum secure key size)
- Correctly flags values ≤ 2047 as insufficient (2047, 1024, 204, 0, -1)
- Properly handles edge cases like zero and negative numbers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Test the key size constraint regex against various inputs
# Expected: Should reject values ≤ 2047 and accept values ≥ 2048
echo "Testing key size constraint regex..."
regex='^(-?(0|[1-9][0-9]?|[1-9][0-9]{2}|1[0-9]{3}|20[0-3][0-9]|204[0-7])(\.[0-9]+)?|0|-[1-9][0-9]*|-[1-9][0-9]{2,}|-1[0-9]{3}|-20[0-3][0-9]|-204[0-7])$'
test_cases=(
"2048" # should not match (valid key size)
"2047" # should match (just below minimum)
"204" # should match (test case value)
"0" # should match (zero)
"-1" # should match (negative)
"1024" # should match (insufficient)
)
for test in "${test_cases[@]}"; do
if [[ $test =~ $regex ]]; then
echo "MATCHED (will trigger warning): $test"
else
echo "NOT MATCHED (will pass): $test"
fi
done
Length of output: 1918
tests/__snapshots__/ruby-cassandra-hardcoded-secret-ruby-snapshot.yml (1)
1-120
: LGTM! Comprehensive snapshot coverage.
The snapshot file correctly captures the AST patterns for both direct hardcoding and variable assignment cases, with proper source ranges and labels.
rules/ruby/security/ruby-cassandra-hardcoded-secret-ruby.yml (1)
1-127
: Verify rule effectiveness across different Ruby versions.
The rule implementation looks solid, but we should ensure it works across different Ruby versions and AST patterns.
✅ Verification successful
Rule implementation effectively handles Ruby Cassandra patterns
The rule implementation is verified to be effective across different Ruby Cassandra connection patterns. The test cases and snapshots demonstrate that it successfully:
- Catches hardcoded passwords in direct cluster configuration
- Detects passwords stored in variables
- Properly handles the valid case of empty password strings
- Correctly processes the required AST patterns including the
require 'cassandra'
statement
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for Ruby version-specific AST differences that might affect the rule
# Look for different Cassandra connection patterns
rg -g '*.rb' -A 3 'Cassandra\.cluster'
# Check for alternative credential passing methods
ast-grep --pattern 'Cassandra.cluster($ARGS)'
Length of output: 237
Script:
#!/bin/bash
# Let's check for test files and examples that might demonstrate Cassandra usage patterns
fd -e rb -e ruby | xargs rg -l "cassandra"
# Look for require statements to find files using Cassandra
rg "require ['|\"]cassandra" -l
# Search for any Cassandra-related test files
fd -e rb -e ruby test spec | xargs rg -l "cassandra"
# Look for configuration files that might contain Cassandra connection patterns
fd -e yml -e yaml | xargs rg -l "cassandra"
Length of output: 1613
Script:
#!/bin/bash
# Let's examine the test cases to verify rule effectiveness
cat tests/ruby/ruby-cassandra-hardcoded-secret-ruby-test.yml
# Also check the snapshot to see what patterns are being caught
cat tests/__snapshots__/ruby-cassandra-hardcoded-secret-ruby-snapshot.yml
Length of output: 3298
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 17-17: wrong indentation: expected 4 but found 3
(indentation)
[warning] 37-37: wrong indentation: expected 13 but found 12
(indentation)
[warning] 48-48: wrong indentation: expected 9 but found 11
(indentation)
[warning] 51-51: wrong indentation: expected 13 but found 15
(indentation)
[warning] 56-56: wrong indentation: expected 4 but found 3
(indentation)
[warning] 76-76: wrong indentation: expected 13 but found 12
(indentation)
[warning] 85-85: wrong indentation: expected 9 but found 11
(indentation)
[warning] 88-88: wrong indentation: expected 13 but found 15
(indentation)
[warning] 92-92: wrong indentation: expected 9 but found 8
(indentation)
[warning] 93-93: wrong indentation: expected 12 but found 15
(indentation)
[warning] 96-96: wrong indentation: expected 17 but found 16
(indentation)
[warning] 97-97: wrong indentation: expected 20 but found 22
(indentation)
[warning] 98-98: too many spaces after colon
(colons)
[warning] 101-101: wrong indentation: expected 20 but found 22
(indentation)
[warning] 104-104: wrong indentation: expected 24 but found 30
(indentation)
[warning] 107-107: wrong indentation: expected 12 but found 14
(indentation)
[warning] 110-110: wrong indentation: expected 16 but found 15
(indentation)
[warning] 113-113: wrong indentation: expected 17 but found 16
(indentation)
[warning] 114-114: wrong indentation: expected 20 but found 22
(indentation)
[warning] 115-115: too many spaces after colon
(colons)
[warning] 118-118: wrong indentation: expected 20 but found 22
(indentation)
[warning] 121-121: wrong indentation: expected 24 but found 30
(indentation)
[error] 122-122: trailing spaces
(trailing-spaces)
Summary by CodeRabbit
New Features
Tests
Documentation