feat: Firewall ipv6 implementation #4802

pierantoniomerlino · 2023-08-10T14:12:48Z

Note: We are using the Conventional Commits convention for our pull request titles. Please take a look at the PR title format document for the supported types and scopes.

After the merge of #4793, this PR adds the support for the IPv6 firewall. The implementation was done tring to re-use as much code as possible, creating abstract classes to share common methods. The new feature is added to the org.eclipse.kura.net.admin.firewall package used only in generics profiles.
Standard profiles will not support the IPv6 firewall.

Since we decided to keep the old firewall implementation in the org.eclipse.kura.net.admin bundle, some degree of code duplication should be tolerated.

pierantoniomerlino · 2023-08-11T13:09:30Z

To test the IPv6 firewall implementation, try to apply a snapshot containing the configuration. The code below is an example (the syntax for the rules are the same than the IPv4 firewall):

  <esf:configuration pid="org.eclipse.kura.net.admin.ipv6.FirewallConfigurationServiceIPv6">
      <esf:properties>
          <esf:property array="false" encrypted="false" name="firewall.ipv6.open.ports" type="String">
              <esf:value>8001,tcp,::0/0,wlan0,,,,#</esf:value>
          </esf:property>
          <esf:property array="false" encrypted="false" name="firewall.ipv6.nat" type="String">
              <esf:value>eth0,wlan0,tcp,::/0,::/0,true,#;</esf:value>
          </esf:property>
          <esf:property array="false" encrypted="false" name="firewall.ipv6.port.forwarding" type="String">
              <esf:value>eth0,wlan0,2a00:1450:400c:c01::71,tcp,2020,1010,true,2001:db8::/64,aa:bb:cc:dd:ee:ff,10100:10200,#</esf:value>
          </esf:property>
          <esf:property array="false" encrypted="false" name="kura.service.pid" type="String">
              <esf:value>org.eclipse.kura.net.admin.ipv6.FirewallConfigurationServiceIPv6</esf:value>
          </esf:property>
          <esf:property array="false" encrypted="false" name="service.pid" type="String">
              <esf:value>org.eclipse.kura.net.admin.ipv6.FirewallConfigurationServiceIPv6</esf:value>
          </esf:property>
      </esf:properties>
  </esf:configuration>

marcellorinaldo · 2023-08-14T07:29:34Z

...g.eclipse.kura.api/src/main/java/org/eclipse/kura/net/firewall/FirewallOpenPortConfigIP.java

@@ -172,10 +244,70 @@ public String getSourcePortRange() {
        return this.sourcePortRange;
    }

+    /**
+     * @deprecated since 2.6. se the FirewallOpenPortConfigIP builder


MIspelling on the comment, se -> see. Same applies for setPermittedMac, setUnpermittedInterfaceName, setUnpermittedInterfaceName, setPermittedInterfaceName, setPermittedNetwork, setProtocol, and setPortRange.

marcellorinaldo · 2023-08-14T13:14:23Z

...ewall/src/main/java/org/eclipse/kura/net/admin/AbstractFirewallConfigurationServiceImpl.java

+
+    public synchronized void updated(Map<String, Object> properties) {
+        logger.debug("updated()");
+        for (Entry<String, Object> entry : properties.entrySet()) {


We should check the log level before entering this for loop. Something like:

if (logger.getLogLevel() == LogLevel.DEBUG) { for (Entry ... }

marcellorinaldo · 2023-08-14T14:34:11Z

kura/org.eclipse.kura.net.admin.firewall/OSGI-INF/firewallConfigurationServiceIPv6.xml

+      <provide interface="org.eclipse.kura.net.admin.ipv6.FirewallConfigurationServiceIPv6"/>
+      <provide interface="org.eclipse.kura.configuration.SelfConfiguringComponent"/>
+   </service>
+   <reference bind="setEventAdmin" cardinality="1..1" interface="org.osgi.service.event.EventAdmin" name="EventAdmin" policy="static" unbind="unsetEventAdmin"/>


Unbind methods are not necessary for static service references

marcellorinaldo · 2023-08-14T15:07:26Z

.../org.eclipse.kura.linux.net/src/main/java/org/eclipse/kura/linux/net/iptables/LocalRule.java

-            String unpermittedInterfaceName, String permittedMAC, String sourcePortRange) {
+    public LocalRule(int port, String protocol, NetworkPair<? extends IPAddress> permittedNetwork,
+            String permittedInterfaceName, String unpermittedInterfaceName, String permittedMAC,
+            String sourcePortRange) {
        this.port = port;
        this.portRange = null;


Can we use an optional here? Or does it affect old implementations?

marcellorinaldo · 2023-08-16T08:20:20Z

...org.eclipse.kura.core.net/src/main/java/org/eclipse/kura/core/net/FirewallConfiguration.java

@@ -68,8 +68,8 @@ public FirewallConfiguration(Map<String, Object> properties) {
    private void parseNatRules(Map<String, Object> properties) {
        String str;
        String[] astr;


I would suggest to use more meaningful names for some variables.

str -> openPorts
astr -> rules
private void parseOpenPortRule(String sop) -> private void parseOpenPortRule(String **rule**)

nicolatimeus · 2023-08-21T09:57:00Z

...org.eclipse.kura.core.net/src/main/java/org/eclipse/kura/core/net/FirewallConfiguration.java


-    public FirewallConfiguration() {


This is a breaking change in terms of API

Should we rename this to AbstractFirewallConfigration and rename FirewallConfigurationIPv4 to FirewallConfiguration and keep the same public interface of this class? otherwise we have to bump the exported version of the org.eclipse.kura.core.net package to 2.0.0

nicolatimeus · 2023-08-21T10:07:19Z

kura/org.eclipse.kura.linux.net/META-INF/MANIFEST.MF

@@ -55,7 +55,7 @@ Export-Package: org.eclipse.kura.internal.board;version="1.0.0";x-internal:=true
   org.eclipse.kura.usb",
 org.eclipse.kura.linux.net.dhcp;version="2.1.0";uses:="org.eclipse.kura.net.dhcp",
 org.eclipse.kura.linux.net.dns;version="1.1.0";uses:="org.eclipse.kura.net,org.eclipse.kura.net.dns",
- org.eclipse.kura.linux.net.iptables;version="1.1.0";uses:="org.eclipse.kura.net",


This PR adds breaking changes in this package.

nicolatimeus

Please address the lates issues in the API PR, then we can merge

Signed-off-by: pierantoniomerlino <[email protected]> Updated firewall implementation classes Signed-off-by: pierantoniomerlino <[email protected]> Added static methods to IPXaddress classes; aligned default values to old behavior for rules builder Signed-off-by: pierantoniomerlino <[email protected]> Fixed imports versions Signed-off-by: pierantoniomerlino <[email protected]> Fixed tests Signed-off-by: pierantoniomerlino <[email protected]> Improved builders for firewall classes Signed-off-by: pierantoniomerlino <[email protected]> Updated FirewallConfigurationServiceImpl with the new APIs Signed-off-by: pierantoniomerlino <[email protected]> Fixed tests Signed-off-by: pierantoniomerlino <[email protected]> Fixed tests Signed-off-by: pierantoniomerlino <[email protected]> Fixed tests again Signed-off-by: pierantoniomerlino <[email protected]> Updated copyright headers Signed-off-by: pierantoniomerlino <[email protected]> Updated copyright headers again Signed-off-by: pierantoniomerlino <[email protected]> Added since annotations Signed-off-by: pierantoniomerlino <[email protected]> Added APIs for firewall ipv6; refactored APIs for firewall ipv4 Signed-off-by: pierantoniomerlino <[email protected]> Updated firewall implementation classes Signed-off-by: pierantoniomerlino <[email protected]> Added static methods to IPXaddress classes; aligned default values to old behavior for rules builder Signed-off-by: pierantoniomerlino <[email protected]> Fixed imports versions Signed-off-by: pierantoniomerlino <[email protected]> Fixed tests Signed-off-by: pierantoniomerlino <[email protected]> First attempto to implement firewall ipv6 Signed-off-by: pierantoniomerlino <[email protected]> Updated net.admin.firewall classes for ipv6 support Signed-off-by: pierantoniomerlino <[email protected]> Updated core.net and linux.net for ipv6 firewall support Signed-off-by: pierantoniomerlino <[email protected]> Moved firewall ipv6 in net.admin.firewall bundle Signed-off-by: pierantoniomerlino <[email protected]> Added abstract class for FirewallConfigurationServices Signed-off-by: pierantoniomerlino <[email protected]> Added missing rules for creating mangle chains Signed-off-by: pierantoniomerlino <[email protected]> Fixed ipv6 properties and forwarding file Signed-off-by: pierantoniomerlino <[email protected]> Fixed icmp rules for ipv6 Signed-off-by: pierantoniomerlino <[email protected]> Updated rules classes Signed-off-by: pierantoniomerlino <[email protected]> Fixed typo Signed-off-by: pierantoniomerlino <[email protected]> Fixed warnings on generics; updated tests Signed-off-by: pierantoniomerlino <[email protected]> Added checkstyle suppression; fixed old net.admin firewall classes Signed-off-by: pierantoniomerlino <[email protected]> Fixed Sonar complains Signed-off-by: pierantoniomerlino <[email protected]> Small refactors Signed-off-by: pierantoniomerlino <[email protected]> Recovered FirewallConfiguration file Signed-off-by: pierantoniomerlino <[email protected]> Recovered IptablesConfigConstants file Signed-off-by: pierantoniomerlino <[email protected]> Updated missing file Signed-off-by: pierantoniomerlino <[email protected]> Hide firewall config in web ui Signed-off-by: pierantoniomerlino <[email protected]> Updated icmpv6 default rules Signed-off-by: pierantoniomerlino <[email protected]> Updated javadoc for Sonar complains Signed-off-by: pierantoniomerlino <[email protected]>

Signed-off-by: pierantoniomerlino <[email protected]>

pierantoniomerlino · 2023-08-29T14:21:17Z

The build fails due to code duplication. This is accepted by the team: the IPv4 code has been duplicated and adapted to support IPv6. The duplication is accepted for now as we are planning to rework this in a more organic way in a later release.

* Added APIs for firewall ipv6; refactored APIs for firewall ipv4 Signed-off-by: pierantoniomerlino <[email protected]> Updated firewall implementation classes Signed-off-by: pierantoniomerlino <[email protected]> Added static methods to IPXaddress classes; aligned default values to old behavior for rules builder Signed-off-by: pierantoniomerlino <[email protected]> Fixed imports versions Signed-off-by: pierantoniomerlino <[email protected]> Fixed tests Signed-off-by: pierantoniomerlino <[email protected]> Improved builders for firewall classes Signed-off-by: pierantoniomerlino <[email protected]> Updated FirewallConfigurationServiceImpl with the new APIs Signed-off-by: pierantoniomerlino <[email protected]> Fixed tests Signed-off-by: pierantoniomerlino <[email protected]> Fixed tests Signed-off-by: pierantoniomerlino <[email protected]> Fixed tests again Signed-off-by: pierantoniomerlino <[email protected]> Updated copyright headers Signed-off-by: pierantoniomerlino <[email protected]> Updated copyright headers again Signed-off-by: pierantoniomerlino <[email protected]> Added since annotations Signed-off-by: pierantoniomerlino <[email protected]> Added APIs for firewall ipv6; refactored APIs for firewall ipv4 Signed-off-by: pierantoniomerlino <[email protected]> Updated firewall implementation classes Signed-off-by: pierantoniomerlino <[email protected]> Added static methods to IPXaddress classes; aligned default values to old behavior for rules builder Signed-off-by: pierantoniomerlino <[email protected]> Fixed imports versions Signed-off-by: pierantoniomerlino <[email protected]> Fixed tests Signed-off-by: pierantoniomerlino <[email protected]> First attempto to implement firewall ipv6 Signed-off-by: pierantoniomerlino <[email protected]> Updated net.admin.firewall classes for ipv6 support Signed-off-by: pierantoniomerlino <[email protected]> Updated core.net and linux.net for ipv6 firewall support Signed-off-by: pierantoniomerlino <[email protected]> Moved firewall ipv6 in net.admin.firewall bundle Signed-off-by: pierantoniomerlino <[email protected]> Added abstract class for FirewallConfigurationServices Signed-off-by: pierantoniomerlino <[email protected]> Added missing rules for creating mangle chains Signed-off-by: pierantoniomerlino <[email protected]> Fixed ipv6 properties and forwarding file Signed-off-by: pierantoniomerlino <[email protected]> Fixed icmp rules for ipv6 Signed-off-by: pierantoniomerlino <[email protected]> Updated rules classes Signed-off-by: pierantoniomerlino <[email protected]> Fixed typo Signed-off-by: pierantoniomerlino <[email protected]> Fixed warnings on generics; updated tests Signed-off-by: pierantoniomerlino <[email protected]> Added checkstyle suppression; fixed old net.admin firewall classes Signed-off-by: pierantoniomerlino <[email protected]> Fixed Sonar complains Signed-off-by: pierantoniomerlino <[email protected]> Small refactors Signed-off-by: pierantoniomerlino <[email protected]> Recovered FirewallConfiguration file Signed-off-by: pierantoniomerlino <[email protected]> Recovered IptablesConfigConstants file Signed-off-by: pierantoniomerlino <[email protected]> Updated missing file Signed-off-by: pierantoniomerlino <[email protected]> Hide firewall config in web ui Signed-off-by: pierantoniomerlino <[email protected]> Updated icmpv6 default rules Signed-off-by: pierantoniomerlino <[email protected]> Updated javadoc for Sonar complains Signed-off-by: pierantoniomerlino <[email protected]> * Fixed checkstyle issues Signed-off-by: pierantoniomerlino <[email protected]> --------- Signed-off-by: pierantoniomerlino <[email protected]>

pierantoniomerlino requested a review from nicolatimeus August 10, 2023 16:17

pierantoniomerlino marked this pull request as ready for review August 10, 2023 16:20

pierantoniomerlino mentioned this pull request Aug 11, 2023

feat(distrib): Updated firewall.init script for ipv6 in generic profiles #4803

Merged

pierantoniomerlino self-assigned this Aug 11, 2023

pierantoniomerlino requested a review from marcellorinaldo August 11, 2023 15:04

marcellorinaldo reviewed Aug 14, 2023

View reviewed changes

marcellorinaldo reviewed Aug 16, 2023

View reviewed changes

nicolatimeus reviewed Aug 21, 2023

View reviewed changes

pierantoniomerlino force-pushed the firewall_ipv6_implementation branch from 2fffe0a to 4cb0b89 Compare August 22, 2023 16:19

nicolatimeus approved these changes Aug 24, 2023

View reviewed changes

marcellorinaldo approved these changes Aug 24, 2023

View reviewed changes

pierantoniomerlino force-pushed the firewall_ipv6_implementation branch from 4cb0b89 to 1996e95 Compare August 24, 2023 15:09

pierantoniomerlino added 2 commits August 28, 2023 15:07

Fixed checkstyle issues

ecfba36

Signed-off-by: pierantoniomerlino <[email protected]>

pierantoniomerlino force-pushed the firewall_ipv6_implementation branch from 2d45754 to ecfba36 Compare August 28, 2023 13:26

nicolatimeus merged commit 578313d into develop Aug 29, 2023

nicolatimeus deleted the firewall_ipv6_implementation branch August 29, 2023 14:10

This was referenced Nov 23, 2023

fix(linux.net): prefer dnsmasq over udhcpd #5017

Closed

fix(nm): load dhcp server configuration after change #5018

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Firewall ipv6 implementation #4802

feat: Firewall ipv6 implementation #4802

pierantoniomerlino commented Aug 10, 2023 •

edited

Loading

pierantoniomerlino commented Aug 11, 2023

marcellorinaldo Aug 14, 2023

marcellorinaldo Aug 14, 2023

marcellorinaldo Aug 14, 2023

marcellorinaldo Aug 14, 2023

marcellorinaldo Aug 16, 2023

nicolatimeus Aug 21, 2023

nicolatimeus Aug 21, 2023

nicolatimeus Aug 21, 2023

nicolatimeus left a comment

pierantoniomerlino commented Aug 29, 2023

feat: Firewall ipv6 implementation #4802

feat: Firewall ipv6 implementation #4802

Conversation

pierantoniomerlino commented Aug 10, 2023 • edited Loading

pierantoniomerlino commented Aug 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolatimeus left a comment

Choose a reason for hiding this comment

pierantoniomerlino commented Aug 29, 2023

pierantoniomerlino commented Aug 10, 2023 •

edited

Loading