SHARMAN-4090 -- LAN SSH not working in 6.3 SDK - #369
Open
umasankar098 wants to merge 3 commits into
Open
Conversation
Reason for change: Creating a firewall rule to block SSH using WAN IP from LAN client Test Procedure: NA Risks: LOW Priority: P1 Signed-off-by: usi096 <umasankar.srinivasan@sky.uk>
|
📋 PR Format Reminder
Expected: |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a feature-flag-controlled firewall rule intended to prevent LAN clients from SSH’ing to the gateway via its WAN IP (port 10022), addressing “LAN SSH not working in 6.3 SDK” by gating the behavior on a sysevent-driven feature flag.
Changes:
- Map a partner config feature flag (
Device.X_RDK_Features.LanSshNewPortSupport.Enable) into a new sysevent (LanSshNewPortSupport). - Add a sysevent check in the firewall code and conditionally install an iptables rule blocking LAN→WAN-IP SSH on port 10022.
- Expose (currently) a helper prototype in
firewall.hfor the new sysevent check.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| source/scripts/init/src/apply_system_defaults/apply_system_defaults.c | Adds sysevent mapping for the new LAN SSH port support feature flag. |
| source/firewall/firewall.h | Declares a new helper API for checking LAN SSH port support. |
| source/firewall/firewall.c | Implements the feature check and conditionally adds the LAN→WAN-IP SSH block rule. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+10837
to
+10849
| int isLanSshPortSupportEnabled() | ||
| { | ||
| char LanSshPortVal[7] = {'\0'}; | ||
| if (sysevent_get(sysevent_fd, sysevent_token, SYSEVENT_LANSSHPORT_SUPPORT, LanSshPortVal, sizeof(LanSshPortVal)) != 0) | ||
| { | ||
| FIREWALL_DEBUG("ERROR: Failed to get LanSshPortSupport value from sysevent\n"); | ||
| return RET_ERR; | ||
| } | ||
| if (strcmp(LanSshPortVal, "true") == 0) | ||
| return 1; | ||
| else | ||
| return 0; | ||
| } |
Comment on lines
+125
to
127
| /* isLanSshPortSupportEnabled function is to check if the product can support LAN SSH through the port 10022 */ | ||
| int isLanSshPortSupportEnabled(); | ||
| #define CCSP_SUBSYS "eRT." |
Reason for change: Creating a firewall rule to block SSH using WAN IP from LAN client Test Procedure: NA Risks: LOW Priority: P1 Signed-off-by: usi096 <umasankar.srinivasan@sky.uk>
Comment on lines
+522
to
+526
| int LanSshPortSupport_ret = isLanSshPortSupportEnabled(); | ||
| if( LanSshPortSupport_ret == 1) | ||
| { | ||
| fprintf(fp, "-I INPUT 1 -p tcp --dport 10022 -j DROP\n"); | ||
| } |
Comment on lines
+10837
to
+10849
| int isLanSshPortSupportEnabled() | ||
| { | ||
| char LanSshPortVal[7] = {'\0'}; | ||
| if (sysevent_get(sysevent_fd, sysevent_token, SYSEVENT_LANSSHPORT_SUPPORT, LanSshPortVal, sizeof(LanSshPortVal)) != 0) | ||
| { | ||
| FIREWALL_DEBUG("ERROR: Failed to get LanSshPortSupport value from sysevent\n"); | ||
| return RET_ERR; | ||
| } | ||
| if (strcmp(LanSshPortVal, "true") == 0) | ||
| return 1; | ||
| else | ||
| return 0; | ||
| } |
Comment on lines
+125
to
+126
| /* isLanSshPortSupportEnabled function is to check if the product can support LAN SSH through the port 10022 */ | ||
| int isLanSshPortSupportEnabled(); |
Reason for change: Creating a firewall rule to block SSH using WAN IP from LAN client Test Procedure: NA Risks: LOW Priority: P1 Signed-off-by: usi096 <umasankar.srinivasan@sky.uk>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
source/firewall/firewall_ipv6.c:526
- The IPv6 rule being added drops all TCP traffic to port 10022 regardless of source interface or destination address. That blocks legitimate LAN SSH over IPv6 too, and it does not match the stated goal of blocking SSH to the WAN IP from LAN clients. Consider scoping the rule to LAN ingress and the current WAN IPv6 address (similar to the IPv4 rule), and guard against an empty WAN IPv6 value.
int LanSshPortSupport_ret = isLanSshPortSupportEnabled();
if( LanSshPortSupport_ret == 1)
{
fprintf(fp, "-I INPUT 1 -p tcp --dport 10022 -j DROP\n");
}
source/firewall/firewall.c:10844
- isLanSshPortSupportEnabled() calls sysevent_get() without first checking that sysevent_fd is valid (other sysevent accessors in this file guard against sysevent_fd < 0). Also the log message mentions "LanSshPortSupport" even though the sysevent key is "LanSshNewPortSupport", which can make debugging harder.
int isLanSshPortSupportEnabled()
{
char LanSshPortVal[7] = {'\0'};
if (sysevent_get(sysevent_fd, sysevent_token, SYSEVENT_LANSSHPORT_SUPPORT, LanSshPortVal, sizeof(LanSshPortVal)) != 0)
{
FIREWALL_DEBUG("ERROR: Failed to get LanSshPortSupport value from sysevent\n");
return RET_ERR;
}
LakshminarayananShenbagaraj
left a comment
LakshminarayananShenbagaraj
left a comment
Contributor
There was a problem hiding this comment.
Fine with me
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason for change: Creating a firewall rule to block SSH using WAN IP from LAN client
Test Procedure: NA
Risks: LOW
Priority: P1