Skip to content
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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ESS-ENN
Copy link
Collaborator

@ESS-ENN ESS-ENN commented Dec 12, 2024

Summary by CodeRabbit

  • New Features

    • Introduced new security rules for detecting insufficient RSA key sizes and hard-coded secrets in Ruby and Rust applications.
    • Added test cases for validating RSA key sizes and secret management practices in Ruby and Rust.
  • Tests

    • Implemented asynchronous test functions for database connections in Rust.
    • Added snapshots for testing RSA key size and hard-coded secrets in Ruby.
  • Documentation

    • Included references to security standards and best practices in the new rules.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


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.

Copy link

coderabbitai bot commented Dec 12, 2024

Walkthrough

This 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

File Path Change Summary
rules/ruby/security/insufficient-rsa-key-size-ruby.yml - New rule added: id: insufficient-rsa-key-size-ruby
- Utility functions for RSA generation and initialization
- Constraint defined for key size with regex
rules/ruby/security/ruby-cassandra-hardcoded-secret-ruby.yml - New rule added: id: ruby-cassandra-hardcoded-secret-ruby
- Rule signature added: kind: call
rules/rust/security/hardcoded-password-rust.yml - New rule added: id: hardcoded-password-rust
- Utility matches for MySQL and PostgreSQL connection options
tests/snapshots/hardcoded-password-rust-snapshot.yml - Added test functions for MySQL and PostgreSQL connections with hard-coded secrets
tests/snapshots/insufficient-rsa-key-size-ruby-snapshot.yml - Added snapshot for RSA key generation with insufficient size
tests/snapshots/ruby-cassandra-hardcoded-secret-ruby-snapshot.yml - Added snapshots demonstrating hard-coded credentials in Cassandra connections
tests/ruby/insufficient-rsa-key-size-ruby-test.yml - New test case added: id: insufficient-rsa-key-size-ruby for RSA key size validation
tests/ruby/ruby-cassandra-hardcoded-secret-ruby-test.yml - New test case added: id: ruby-cassandra-hardcoded-secret-ruby for hard-coded secrets validation
tests/rust/hardcoded-password-rust-test.yml - New test cases added for validating hard-coded passwords in Rust applications

Possibly related PRs

  • Rules - node-rsa-weak-key in Js/Ts #12: The rule for detecting weak RSA keys in JavaScript is related to the main PR's focus on RSA key size validation, as both address RSA key security.
  • Two Rust rules 17Oct2024 #36: The rules for detecting empty and hard-coded passwords in Rust applications are relevant as they share a common theme of enhancing security practices, similar to the main PR's focus on RSA key size validation.
  • Rules - One C rule and one Ruby rule #34: The Ruby rule for detecting hardcoded HTTP authentication in controllers is related to the main PR's focus on security practices in Ruby applications, specifically regarding sensitive information management.

Suggested reviewers

  • rohit121
  • petrisorcoderabbit

Poem

In the code where secrets hide,
A rabbit hops with eyes open wide.
With rules to guard and tests in place,
We ensure security in every space.
From RSA keys to passwords tight,
Our code will shine, secure and bright! 🐇✨


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai bot changed the title @coderabbitai Add security rules for RSA key sizes and hard-coded secrets in Ruby and Rust Dec 12, 2024
Copy link

@coderabbitai coderabbitai bot left a 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:

  1. Key generation through configuration files
  2. Dynamic key size calculation
  3. 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.html
tests/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:

  1. Detect other sensitive parameters (e.g., certificates, tokens)
  2. Check for common password patterns (e.g., test/dev credentials)
  3. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 52b2182 and 77415f8.

📒 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:

  1. Captures the direct connection pattern:
MySqlConnectOptions::new()
.password("password")
  1. Captures the indirect connection pattern:
let pg = PgConnectOptions::new();
pg.password("secret-password")
  1. 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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants