-
Notifications
You must be signed in to change notification settings - Fork 390
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
Security Guide for Celestia Nodes #1759
Conversation
Tested with ports and peers for validator node
Add empty files for blocked IPs
WalkthroughThis pull request introduces a new "Security" item in the sidebar of the VitePress configuration, linking to a guide on securing Celestia applications. Additionally, it updates the Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
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: 9
🧹 Outside diff range and nitpick comments (6)
how-to-guides/celestia-app-security.md (5)
6-6
: Consider using a hyphen for the compound adjective "rate limiting".For clarity, you might want to hyphenate "rate limiting" when it's used as a compound adjective:
-By using [FireHOL](https://github.com/firehol/firehol), we introduce a flexible and lightweight firewall that supports advanced features like rate limiting and dynamic IP blacklisting. +By using [FireHOL](https://github.com/firehol/firehol), we introduce a flexible and lightweight firewall that supports advanced features like rate-limiting and dynamic IP blacklisting.🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
21-35
: Consider adding troubleshooting information for FireHOL installation.The installation instructions are clear, but it might be helpful to include some common troubleshooting steps or potential issues users might encounter during installation. For example:
- What to do if the installation fails?
- How to check if FireHOL is running correctly?
- Any system requirements or dependencies?
This additional information could help users navigate potential obstacles more easily.
125-131
: Consider making the interface binding tip more prominent.The tip about binding to the correct interface is crucial for proper configuration. Consider moving this information to the beginning of the configuration section and formatting it for better visibility. For example:
## Configure FireHOL for Celestia Node ::: warning Ensure you bind FireHOL to the correct network interface. The example uses `enp5s0`, but you should replace this with your actual interface name. To bind to all interfaces, use: ```sh interface any world:::
Create or modify your FireHOL configuration file...
--- `223-224`: **Add instructions for verifying the cron job's operation.** After setting up the cron job, it's important to verify that it's working correctly. Consider adding a section on how to check the cron job's operation. For example: ```markdown ### Verifying the Cron Job To ensure the cron job is running correctly: 1. Check the system logs for cron activity: ```sh sudo grep CRON /var/log/syslog
Manually run the script and check for any errors:
sudo /usr/local/bin/update-blocked-ips.shCheck the log file for update activities:
tail -f /var/log/update-blocked-ips.logIf you see regular updates in the log file and no errors when manually running the script, your cron job is likely working correctly.
--- `227-269`: **Add instructions for rolling back FireHOL configuration changes.** It's important to provide users with a way to revert changes if the new configuration causes issues. Consider adding a section on how to roll back changes: ```markdown ### Rolling Back Changes If the new FireHOL configuration causes issues, you can roll back to the previous state: 1. Stop the FireHOL service: ```sh sudo systemctl stop firehol
Restore the previous configuration (assuming you've made a backup):
sudo cp /etc/firehol/firehol.conf.bak /etc/firehol/firehol.confRestart FireHOL with the previous configuration:
sudo systemctl start fireholVerify that the previous rules are applied:
sudo iptables -LAlways make a backup of your working configuration before making changes.
</blockquote></details> <details> <summary>.vitepress/config.ts (1)</summary><blockquote> Line range hint `1-24`: **Approve with suggestion: Open Graph meta tags added** The addition of Open Graph meta tags for title and description is a good improvement for SEO and social media sharing. The conditional title setting is a nice touch to distinguish the home page from other pages. However, there's a small issue with the description meta tag: ```typescript { name: "og:description", content: pageData.frontmatter.layout === `${pageData.description}`, },
This doesn't actually set the description to the page's description. It's comparing the layout to the description as a string, which is likely not the intended behavior. Consider changing it to:
{ name: "og:description", content: pageData.description, },Or if you want to keep the conditional logic:
{ name: "og:description", content: pageData.frontmatter.layout === "home" ? "Home page description" : pageData.description, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .vitepress/config.ts (1 hunks)
- how-to-guides/celestia-app-security.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
how-to-guides/celestia-app-security.md
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
how-to-guides/celestia-app-security.md
9-9: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
133-133: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
9-9: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
🔇 Additional comments (1)
.vitepress/config.ts (1)
510-513
: LGTM: New security guide added to sidebarThe addition of the "Security" item in the sidebar under the "celestia-app" section is well-placed and follows the existing structure. This change aligns with the PR objectives to enhance security documentation for Celestia nodes.
|
||
::: | ||
|
||
#### Key Configuration Details |
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.
Adjust heading level for consistency.
The "Key Configuration Details" section uses an h4 heading, which is inconsistent with the document structure. Consider changing it to an h3 for better hierarchy:
### Key Configuration Details
🧰 Tools
🪛 Markdownlint
133-133: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
- Blocked IPs: Dynamic IP blocking is applied using blacklists like Firehole Level4 and | ||
|
||
- Emerging Threats. |
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.
Fix line break in the sentence about blocked IPs.
There's an unintended line break in the sentence about blocked IPs. Combine these lines for better readability:
- Blocked IPs: Dynamic IP blocking is applied using blacklists like Firehole Level4 and Emerging Threats.
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: 0
🧹 Outside diff range and nitpick comments (4)
how-to-guides/celestia-app-security.md (4)
1-20
: Approve changes with a minor suggestion.The introduction and key features section provide a clear and informative overview of the security guide. The heading structure has been improved as per previous suggestions.
Consider hyphenating the compound adjective in line 6:
-Validators in Celestia often rely on cloud or baremetal servers for their infrastructure. While some providers offer basic protection, most setups lack advanced firewall solutions such as DDoS mitigation. This makes it critical to implement your own measures. By using [FireHOL](https://github.com/firehol/firehol), we introduce a flexible and lightweight firewall that supports advanced features like rate limiting and dynamic IP blacklisting. This solution ensures that your node can handle increasing traffic while protecting it from malicious actors. +Validators in Celestia often rely on cloud or baremetal servers for their infrastructure. While some providers offer basic protection, most setups lack advanced firewall solutions such as DDoS mitigation. This makes it critical to implement your own measures. By using [FireHOL](https://github.com/firehol/firehol), we introduce a flexible and lightweight firewall that supports advanced features like rate-limiting and dynamic IP blacklisting. This solution ensures that your node can handle increasing traffic while protecting it from malicious actors.🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
9-9: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
9-9: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
133-150
: Approve key configuration details with minor formatting suggestion.The key configuration details provide a clear summary of the FireHOL setup. However, there's a minor formatting issue that can be improved.
Fix the formatting of the blocked IPs bullet point:
-- Blocked IPs: Dynamic IP blocking is applied using blacklists like Firehole Level4 and - -- Emerging Threats. + - Blocked IPs: Dynamic IP blocking is applied using blacklists like Firehole Level4 and Emerging Threats.
151-224
: Approve cron job setup with script improvement suggestions.The cron job setup instructions are clear and comprehensive. However, the bash script for updating blocked IPs can be improved for better error handling and logging.
Consider enhancing the bash script with improved error handling and logging:
#!/bin/bash # Log file LOG_FILE="/var/log/update-blocked-ips.log" # Function for logging log() { echo "$(date): $1" >> "$LOG_FILE" } # Temporary files for downloading the blocklists tmp_emerging=$(mktemp) || { log "Failed to create temporary file"; exit 1; } tmp_firehol=$(mktemp) || { log "Failed to create temporary file"; exit 1; } tmp_nets=$(mktemp) || { log "Failed to create temporary file"; exit 1; } # Download the block lists from Emerging Threats and FireHOL Level 4 log "Downloading Emerging Threats blocklist" if ! wget -O "$tmp_emerging" "http://rules.emergingthreats.net/fwrules/emerging-Block-IPs.txt"; then log "Failed to download Emerging Threats blocklist" rm "$tmp_emerging" exit 1 fi log "Downloading FireHOL Level 4 blocklist" if ! wget -O "$tmp_firehol" "https://iplists.firehol.org/files/firehol_level4.netset"; then log "Failed to download FireHOL Level 4 blocklist" rm "$tmp_emerging" "$tmp_firehol" exit 1 fi # Preprocess the blocklists: Extract only network entries (lines with '/') log "Preprocessing blocklists" grep '/' "$tmp_emerging" > "$tmp_nets" cat "$tmp_firehol" >> "$tmp_nets" # Update the IPSet collections using FireHOL log "Updating IPSet collections" if ! firehol ipset_update_from_file blocked_nets nets "$tmp_nets"; then log "Failed to update IPSet collections" rm "$tmp_emerging" "$tmp_firehol" "$tmp_nets" exit 1 fi # Clean up temporary files rm "$tmp_emerging" "$tmp_firehol" "$tmp_nets" log "Update completed successfully"This enhanced version includes logging, better error handling, and cleanup of temporary files even if the script fails.
227-282
: Approve configuration application and testing with suggestions for improvements.The instructions for applying and testing the FireHOL configuration are clear and comprehensive. However, there are a few areas that can be improved for better clarity and effectiveness.
- Improve the port testing commands to verify external access:
-Test specific port access using netcat: - -```sh -nc -zv <your_server_ip> 26657 # Test RPC port -nc -zv <your_server_ip> 26656 # Test P2P port -``` +Test specific port access using netcat from another machine or using your server's public IP: + +```sh +nc -zv <your_server_public_ip> 26657 # Test RPC port +nc -zv <your_server_public_ip> 26656 # Test P2P port +``` + +Replace `<your_server_public_ip>` with your server's actual public IP address. + +If you don't have access to another machine, you can use online port checking tools or ask a trusted peer to attempt the connection.
- Restructure the final tips section for better readability:
## Best Practices for Ongoing Maintenance To ensure your Celestia node remains secure: 1. **Monitor Firewall Logs** - Regularly check `/var/log/kern.log` for unusual traffic patterns. - Set up log rotation to manage log file sizes. 2. **Adjust Rate Limits** - Periodically review and adjust rate limits based on your node's traffic load. - Monitor for legitimate traffic being blocked and adjust accordingly. 3. **Keep Blacklists Updated** - Ensure the cron job for updating blacklists is running correctly. - Periodically review and update your blacklist sources for optimal protection. 4. **Perform Regular Security Audits** - Review your FireHOL configuration monthly for any necessary updates. - Stay informed about new security threats and adjust your configuration as needed.These changes will provide a clearer, more actionable set of best practices for users to follow and ensure more accurate testing of external access to the node.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- how-to-guides/celestia-app-security.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
how-to-guides/celestia-app-security.md
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
how-to-guides/celestia-app-security.md
9-9: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
9-9: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
🔇 Additional comments (3)
how-to-guides/celestia-app-security.md (3)
21-38
: LGTM: Clear installation instructions.The FireHOL installation instructions are clear, concise, and include all necessary steps, including verification of the installation.
1-282
: Overall, excellent security guide with room for minor improvements.This comprehensive guide for securing Celestia nodes using FireHOL is well-structured and informative. It covers all essential aspects of firewall configuration, automated IP blocking, and ongoing maintenance. The suggested improvements throughout the review will further enhance the security, clarity, and effectiveness of the guide.
Key improvements suggested:
- Limiting localhost and internal network rules for better security.
- Enhancing the bash script for updating blocked IPs with improved error handling and logging.
- Improving port testing instructions to verify external access accurately.
- Restructuring the final tips section for better readability and actionability.
Implementing these suggestions will result in a more robust and user-friendly security guide for Celestia node operators.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
9-9: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
9-9: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
39-132
: Approve configuration with security improvement suggestions.The FireHOL configuration is comprehensive and covers essential aspects of securing a Celestia node. However, there are potential security improvements that can be made.
Consider limiting the localhost and internal network rules to only necessary ports and services. This can improve security if the internal network is compromised. For example:
- # Allow all traffic from localhost without specifying ports - server all accept src 127.0.0.1 + # Allow specific services from localhost + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 127.0.0.1 - # Allow all traffic from internal network without specifying ports - server all accept src 192.168.1.0/24 + # Allow specific services from internal network + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 192.168.1.0/24To verify the impact of these changes, you can run the following script:
This script will help identify any services that might be unintentionally exposed by the current broad localhost and internal network rules.
- no blanks needed between items in this list - remove all instances of more than 1 blank line in a row - update wording to use celestia consensus node or celestia app
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: 5
🧹 Outside diff range and nitpick comments (1)
how-to-guides/celestia-app-security.md (1)
6-6
: Improve readability with proper hyphenation.Add hyphens to compound adjectives for better readability:
-Validators in Celestia often rely on cloud or baremetal servers for their infrastructure. While some providers offer basic protection, most setups lack advanced firewall solutions such as DDoS mitigation. This makes it critical to implement your own measures. By using [FireHOL](https://github.com/firehol/firehol), we introduce a flexible and lightweight firewall that supports advanced features like rate limiting and dynamic IP blacklisting. This solution ensures that your node can handle increasing traffic while protecting it from malicious actors. +Validators in Celestia often rely on cloud or bare-metal servers for their infrastructure. While some providers offer basic protection, most setups lack advanced firewall solutions such as DDoS mitigation. This makes it critical to implement your own measures. By using [FireHOL](https://github.com/firehol/firehol), we introduce a flexible and lightweight firewall that supports advanced features like rate-limiting and dynamic IP-blacklisting. This solution ensures that your node can handle increasing traffic while protecting it from malicious actors.🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- how-to-guides/celestia-app-security.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
how-to-guides/celestia-app-security.md
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
how-to-guides/celestia-app-security.md
8-8: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
8-8: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
🔇 Additional comments (2)
how-to-guides/celestia-app-security.md (2)
98-102
:⚠️ Potential issueRestrict access from localhost and internal network.
Allowing unrestricted access (
server all accept
) from localhost and internal network is a security risk. Consider:
- Limiting access to specific required ports
- Implementing proper network segmentation
- Using specific IP addresses instead of entire network ranges
- # Allow all traffic from localhost without specifying ports - server all accept src 127.0.0.1 - - # Allow all traffic from internal network without specifying ports - server all accept src 192.168.1.0/24 + # Allow specific services from localhost + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 127.0.0.1 + + # Allow specific services from internal network (adjust IPs as needed) + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 192.168.1.0/24Likely invalid or redundant comment.
80-91
:⚠️ Potential issueDocument the purpose of additional ports.
The additional ports (11656, 11065, 36656, 6065, 9099) are opened without explanation of their purpose. Please document:
- What services use these ports?
- Why both TCP and UDP are needed?
- Are these ports required for all deployments?
This documentation is crucial for security auditing and maintenance.
Let's verify if these ports are documented elsewhere:
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 (3)
how-to-guides/celestia-app-security.md (3)
6-6
: Improve readability with proper hyphenation.Add hyphens to compound adjectives:
- "rate-limiting"
- "IP-blacklisting"
🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
112-115
: Enhance interface binding security guidance.The tip about interface binding needs more context about security implications:
- Binding to specific interfaces provides better security control
- Using
interface any world
might expose services unnecessarilyConsider expanding the tip to explain these security trade-offs.
168-170
: Enhance guidance on IP list selection.The tip about adding more IP lists needs security considerations:
- Evaluate the reliability and update frequency of additional sources
- Consider the false-positive rate of each list
- Document the performance impact of adding more lists
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- how-to-guides/celestia-app-security.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
how-to-guides/celestia-app-security.md
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
how-to-guides/celestia-app-security.md
8-8: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
8-8: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
🔇 Additional comments (1)
how-to-guides/celestia-app-security.md (1)
8-8
:⚠️ Potential issueAdjust heading hierarchy for better document structure.
Change the heading level to maintain proper hierarchy:
-### Key features we'll leverage include: +## Key FeaturesLikely invalid or redundant comment.
🧰 Tools
🪛 Markdownlint
8-8: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
8-8: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
thanks for updates. please also implement and resolve (or ignore and resolve) coderabbits suggestions! |
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- how-to-guides/celestia-app-security.md (1 hunks)
🧰 Additional context used
📓 Learnings (1)
how-to-guides/celestia-app-security.md (2)
Learnt from: GeoddHQ PR: celestiaorg/docs#1759 File: how-to-guides/celestia-app-security.md:225-230 Timestamp: 2024-10-24T07:19:46.855Z Learning: In guides for Celestia node security, when testing FireHOL configurations, basic port testing using `nc` is sufficient for troubleshooting; extended port testing procedures are not necessary.
Learnt from: GeoddHQ PR: celestiaorg/docs#1759 File: how-to-guides/celestia-app-security.md:24-28 Timestamp: 2024-10-24T07:13:07.603Z Learning: In the `how-to-guides/celestia-app-security.md`, the FireHOL service status is tested later in the guide, so adding additional verification steps immediately after installation is not necessary.
🪛 LanguageTool
how-to-guides/celestia-app-security.md
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Markdownlint
how-to-guides/celestia-app-security.md
8-8: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
8-8: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
🔇 Additional comments (3)
how-to-guides/celestia-app-security.md (3)
234-239
: LGTM!The basic port testing using
nc
is sufficient for troubleshooting FireHOL configurations.
79-91
:⚠️ Potential issueReview and document additional ports.
Several ports are opened without documentation or rate limiting:
- 11656, 11065, 6065: No documented purpose found
- 36656: Standard CometBFT P2P port
- 9099: gRPC endpoint
Recommendations:
- Remove undocumented ports unless they serve a specific purpose
- Add rate limiting for port 9099 similar to other gRPC ports
- Document the purpose of each remaining port
- # Peers, Seeds, Addrbook ports - server custom 11656 tcp/11656 default accept - server custom 11656 udp/11656 default accept - server custom 11065 tcp/11065 default accept - server custom 11065 udp/11065 default accept # Standard CometBFT P2P ports server custom 36656 tcp/36656 default accept server custom 36656 udp/36656 default accept - server custom 6065 tcp/6065 default accept - server custom 6065 udp/6065 default accept # gRPC endpoint ports server custom 9099 tcp/9099 default accept - server custom 9099 udp/9099 default accept + protection syn-floods 15/sec 30 # Add DDoS protection
99-103
:⚠️ Potential issueRestrict internal network access for better security.
The current configuration allows all traffic from localhost and internal network without port restrictions. This could pose security risks if the internal network is compromised.
Consider limiting these rules to only necessary ports:
- # Allow all traffic from localhost without specifying ports - server all accept src 127.0.0.1 + # Allow specific services from localhost + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 127.0.0.1 - # Allow all traffic from internal network without specifying ports - server all accept src 192.168.1.0/24 + # Allow specific services from internal network + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 192.168.1.0/24Likely invalid or redundant comment.
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 (1)
how-to-guides/celestia-app-security.md (1)
178-178
: Fix grammatical error in tip.The sentence has incorrect noun number:
-You can add more IP list from the db +You can add more IP lists from the db🧰 Tools
🪛 LanguageTool
[uncategorized] ~178-~178: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...works" ``` ::: tip You can add more IP list from the db, not adding more list will ...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- how-to-guides/celestia-app-security.md (1 hunks)
🧰 Additional context used
📓 Learnings (1)
how-to-guides/celestia-app-security.md (2)
Learnt from: GeoddHQ PR: celestiaorg/docs#1759 File: how-to-guides/celestia-app-security.md:225-230 Timestamp: 2024-10-24T07:19:46.855Z Learning: In guides for Celestia node security, when testing FireHOL configurations, basic port testing using `nc` is sufficient for troubleshooting; extended port testing procedures are not necessary.
Learnt from: GeoddHQ PR: celestiaorg/docs#1759 File: how-to-guides/celestia-app-security.md:24-28 Timestamp: 2024-10-24T07:13:07.603Z Learning: In the `how-to-guides/celestia-app-security.md`, the FireHOL service status is tested later in the guide, so adding additional verification steps immediately after installation is not necessary.
🪛 LanguageTool
how-to-guides/celestia-app-security.md
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~178-~178: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...works" ``` ::: tip You can add more IP list from the db, not adding more list will ...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
🪛 Markdownlint
how-to-guides/celestia-app-security.md
8-8: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
🔇 Additional comments (2)
how-to-guides/celestia-app-security.md (2)
99-103
:⚠️ Potential issueReview unrestricted internal network access.
Allowing all traffic from localhost and internal network without port restrictions could pose security risks if the internal network is compromised. Consider:
- Limiting access to specific required ports
- Implementing rate limiting for internal traffic
- # Allow all traffic from localhost without specifying ports - server all accept src 127.0.0.1 - - # Allow all traffic from internal network without specifying ports - server all accept src 192.168.1.0/24 + # Allow specific services from localhost + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 127.0.0.1 + + # Allow specific services from internal network + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 192.168.1.0/24
79-91
:⚠️ Potential issueDocument and secure additional ports.
Several ports are opened without proper documentation or rate limiting:
- 11656, 11065: Purpose not documented
- 6065: Purpose not documented
- 9099: gRPC endpoint without rate limiting
Consider:
- Documenting the purpose of each port
- Adding rate limiting similar to other ports
- Removing unused ports
✅ Verification successful
Let me search for more context about these ports in configuration or documentation files.
Let me try a different search strategy focusing on markdown files since this appears to be documentation.
Based on the search results, I can now provide a comprehensive response about the ports mentioned in the review comment:
Document ports with proper rate limiting and security measures
The ports mentioned in the FireHOL configuration serve specific purposes:
- 11656: Standard CometBFT P2P port for peers and seeds communication
- 11065: Standard CometBFT P2P port for peers and seeds communication
- 36656, 6065: Additional CometBFT P2P ports for network communication
- 9099: gRPC endpoint for state queries and transaction broadcasting
Recommendations:
- Add rate limiting similar to other ports in the config:
# Add protection for P2P ports protection syn-floods 20/sec 50 # For 11656, 11065 protection syn-floods 15/sec 30 # For 36656, 6065 protection syn-floods 15/sec 30 # For gRPC 9099
- Document port purposes in comments:
# Peers and Seeds ports for network communication server custom 11656 tcp/11656 default accept # CometBFT P2P server custom 11065 tcp/11065 default accept # CometBFT P2P # Additional CometBFT P2P ports server custom 36656 tcp/36656 default accept # CometBFT P2P server custom 6065 tcp/6065 default accept # CometBFT P2P # gRPC endpoint for state queries and transactions server custom 9099 tcp/9099 default accept # gRPC endpointAll these ports are standard CometBFT and gRPC ports used for P2P communication and state access in the Celestia network. They should be properly rate-limited to prevent DDoS attacks while maintaining network functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for port usage in the codebase for port in 11656 11065 6065 9099; do echo "=== Port $port ===" rg -n "$port" doneLength of output: 1183
Script:
#!/bin/bash # Search for port documentation in config files and docs echo "=== Searching for port documentation ===" rg -i -A 3 -B 3 "(port|listen|addr|endpoint)" \ --type md \ --type yaml \ --type toml \ --type conf # Search specifically for P2P and gRPC related config echo -e "\n=== Searching for P2P and gRPC config ===" rg -i "(p2p|grpc|rpc)" \ --type md \ --type yaml \ --type toml \ --type confLength of output: 412
Script:
#!/bin/bash # Search in markdown files for network configuration documentation echo "=== Searching documentation for network ports ===" rg -i -A 5 "port" --type md echo -e "\n=== Searching documentation for P2P and gRPC ===" rg -i -A 5 "(p2p|grpc)" --type md echo -e "\n=== Looking at specific files that might contain config info ===" fd -e md -e txt -e yaml -e toml "config|network|setup" -x cat {}Length of output: 184906
## Key Features We'll Leverage | ||
|
||
1. Traffic filtering and rate limiting to mitigate potential DDoS attacks. | ||
2. Dynamic IP blocking using blacklists like [Firehol Level 4](https://iplists.firehol.org/?ipset=firehol_level4) and [Emerging Threats](https://iplists.firehol.org/?ipset=et_block) to automatically block known malicious IPs. |
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.
2. Dynamic IP blocking using blacklists like [Firehol Level 4](https://iplists.firehol.org/?ipset=firehol_level4) and [Emerging Threats](https://iplists.firehol.org/?ipset=et_block) to automatically block known malicious IPs. | |
2. Dynamic IP blocking using blacklists like [Firehol Level 4](https://iplists.firehol.org/?ipset=firehol_level4) and [Emerging Threats](https://iplists.firehol.org/?ipset=et_block) to automatically block known malicious IPs. |
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.
Anything specific thing made here ?
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.
@jcstein this is a terrible idea IMHO, we should not recommend products that exercise a centralised control over connectivity.
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.
nits! thanks again for updates
Co-authored-by: Josh Stein <[email protected]>
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 (2)
how-to-guides/celestia-app-security.md (2)
1-3
: Enhance the description to be more specific.Consider updating the description to be more specific about the security measures:
-description: Security guide for Celestia consensus nodes +description: Guide for securing Celestia consensus nodes using FireHOL firewall and DDoS protection
125-125
: Fix line break in blocked IPs description.Combine the line for better readability:
-- Blocked IPs: Dynamic IP blocking is applied using blacklists like Firehole Level4 and -- Emerging Threats. +- Blocked IPs: Dynamic IP blocking is applied using blacklists like Firehole Level4 and Emerging Threats.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- how-to-guides/celestia-app-security.md (1 hunks)
🧰 Additional context used
📓 Learnings (1)
how-to-guides/celestia-app-security.md (2)
Learnt from: GeoddHQ PR: celestiaorg/docs#1759 File: how-to-guides/celestia-app-security.md:225-230 Timestamp: 2024-10-24T07:19:46.855Z Learning: In guides for Celestia node security, when testing FireHOL configurations, basic port testing using `nc` is sufficient for troubleshooting; extended port testing procedures are not necessary.
Learnt from: GeoddHQ PR: celestiaorg/docs#1759 File: how-to-guides/celestia-app-security.md:24-28 Timestamp: 2024-10-24T07:13:07.603Z Learning: In the `how-to-guides/celestia-app-security.md`, the FireHOL service status is tested later in the guide, so adding additional verification steps immediately after installation is not necessary.
🔇 Additional comments (4)
how-to-guides/celestia-app-security.md (4)
14-28
: LGTM! Installation steps are clear and sufficient.The installation instructions are concise and include the necessary verification step.
203-251
: LGTM! Testing steps are appropriate.The testing and verification steps are clear and sufficient for basic troubleshooting.
135-175
: 🛠️ Refactor suggestionEnhance script security.
Consider these security improvements:
- Use HTTPS for downloading blocklists
- Verify file checksums if available
- Add a timeout for wget commands to prevent hanging
# Download the block lists from Emerging Threats and FireHOL Level 4 -wget -O $tmp_emerging "http://rules.emergingthreats.net/fwrules/emerging-Block-IPs.txt" +wget --timeout=300 -O $tmp_emerging "https://rules.emergingthreats.net/fwrules/emerging-Block-IPs.txt" -wget -O $tmp_firehol "https://iplists.firehol.org/files/firehol_level4.netset" +wget --timeout=300 -O $tmp_firehol "https://iplists.firehol.org/files/firehol_level4.netset"Likely invalid or redundant comment.
99-103
:⚠️ Potential issueRestrict internal network access.
Allowing all traffic from localhost and internal network without port restrictions could pose security risks if the internal network is compromised. Consider:
- Limiting access to specific required ports
- Implementing rate limiting for internal traffic
- # Allow all traffic from localhost without specifying ports - server all accept src 127.0.0.1 - - # Allow all traffic from internal network without specifying ports - server all accept src 192.168.1.0/24 + # Allow specific services from localhost + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 127.0.0.1 + + # Allow specific services from internal network + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 192.168.1.0/24Likely invalid or redundant comment.
as per change request
…elestia-official-docs into security-celestia-app
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- how-to-guides/celestia-app-security.md (1 hunks)
🧰 Additional context used
📓 Learnings (1)
how-to-guides/celestia-app-security.md (2)
Learnt from: GeoddHQ PR: celestiaorg/docs#1759 File: how-to-guides/celestia-app-security.md:225-230 Timestamp: 2024-10-24T07:19:46.855Z Learning: In guides for Celestia node security, when testing FireHOL configurations, basic port testing using `nc` is sufficient for troubleshooting; extended port testing procedures are not necessary.
Learnt from: GeoddHQ PR: celestiaorg/docs#1759 File: how-to-guides/celestia-app-security.md:24-28 Timestamp: 2024-10-24T07:13:07.603Z Learning: In the `how-to-guides/celestia-app-security.md`, the FireHOL service status is tested later in the guide, so adding additional verification steps immediately after installation is not necessary.
🪛 LanguageTool
how-to-guides/celestia-app-security.md
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🔇 Additional comments (6)
how-to-guides/celestia-app-security.md (6)
1-13
: LGTM! Clear and comprehensive introduction.The metadata, title, and introduction effectively communicate the purpose and importance of the security guide for Celestia consensus nodes.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ll that supports advanced features like rate limiting and dynamic IP blacklisting. This solut...(EN_COMPOUND_ADJECTIVE_INTERNAL)
14-28
: LGTM! Installation instructions are clear and sufficient.The installation steps are concise and include necessary version verification.
118-126
: LGTM! Clear documentation of configuration details.The key configuration details are well-documented and accurately describe each port's purpose.
127-202
: LGTM! Cron job setup is appropriate for initial implementation.The script provides sufficient logging and error handling for the initial implementation.
203-251
: LGTM! Testing procedures are sufficient.The configuration application and testing steps are clear and appropriate for verifying the setup.
99-103
: 🛠️ Refactor suggestionConsider limiting localhost and internal network access.
The current configuration allows unrestricted access from localhost and internal networks. Consider limiting access to specific services for better security.
- # Allow all traffic from localhost without specifying ports - server all accept src 127.0.0.1 + # Allow specific services from localhost + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 127.0.0.1 - # Allow all traffic from internal network without specifying ports - server all accept src 192.168.1.0/24 + # Allow specific services from internal network + server custom celestia-services tcp/26656,26657,26658,26659,26660,9090,9091,1317 accept src 192.168.1.0/24Likely invalid or redundant comment.
Added all your requests |
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.
Strongly against this.
## Key Features We'll Leverage | ||
|
||
1. Traffic filtering and rate limiting to mitigate potential DDoS attacks. | ||
2. Dynamic IP blocking using blacklists like [Firehol Level 4](https://iplists.firehol.org/?ipset=firehol_level4) and [Emerging Threats](https://iplists.firehol.org/?ipset=et_block) to automatically block known malicious IPs. |
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.
@jcstein this is a terrible idea IMHO, we should not recommend products that exercise a centralised control over connectivity.
Agreed! The |
Overview
This PR adds documentation for securing Celestia nodes using FireHOL. The guide includes:
Motivation
As the Celestia network scales and validators join from various locations, often with varying levels of infrastructure security, it's essential to implement protection directly on the node. FireHOL provides an effective solution to mitigate DDoS attacks, manage traffic spikes, and block malicious IPs without relying on third-party services. This proactive approach helps ensure nodes remain secure, stable, and performant as the network grows.
Summary by CodeRabbit
New Features
Documentation