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

fix(freebsd): make kernel module name configurable #149

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alxwr
Copy link
Member

@alxwr alxwr commented Feb 12, 2022

With FreeBSD if_tap das merged into if_tuntap.
https://reviews.freebsd.org/D20044

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

none

Describe the changes you're proposing

Make the name of the kernel module configurable and set sane defaults.

Pillar / config required to test the proposed changes

none, unless you're using your own kernel module.

Debug log showing how the proposed changes work

----------                                                                                                                                  
          ID: openvpn_kldload_if_tap                                                                                                        
    Function: kmod.present                                                                                                                  
        Name: if_tuntap                                                                                                                     
      Result: True                                                                                                                          
     Comment: Loaded kernel module if_tuntap                                                                                                
     Started: 02:48:43.724240                                                                                                               
    Duration: 98.336 ms                                                                                                                     
     Changes:                                                                                                                               
              ----------                                                                                                                    
              if_tuntap:                                                                                                                    
                  loaded

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

@alxwr alxwr requested a review from myii February 12, 2022 01:54
@alxwr alxwr requested a review from dafyddj as a code owner February 12, 2022 01:54
@alxwr alxwr self-assigned this Feb 12, 2022
@myii
Copy link
Member

myii commented Feb 24, 2022

@alxwr Thanks for this PR (and apologies for the delay). This reminded me to try out the FreeBSD CI that I was planning to add to this formula. When I first tried it almost a year ago, it failed on 2 states. After trying it again (without this PR at first), it failed a lot more -- but after adding your commit, I was back to failing 2 states.

I resolved the first one (directory didn't exist), which was around this state:

openvpn_create_dh_{{ dh }}:
cmd.run:
- name: '"{{ map.bin_dir | default('', true) }}openssl" dhparam {% if map.dsaparam %}-dsaparam {% endif %}-out "{{ dh_file }}" {{ dh }}'
- creates: {{ dh_file }}
- require:
- pkg: openvpn_pkgs

Adding this to the above was enough:

     - require:
       - pkg: openvpn_pkgs
+      {%- if grains.os_family in ["FreeBSD"] %}
+      - file: openvpn_config_dir
+      {%- endif %}
  • @dafyddj Should this requisite actually be used for all platforms?

The second failure was to do with the client service failing to start (server service starts up fine). Looking at the log files:

root@default-freebsd-130-master-py3:/var/log/openvpn # cat myserver1.log
2022-02-24 16:41:53 Cipher negotiation is disabled since neither P2MP client nor server mode is enabled
2022-02-24 16:41:53 OpenVPN 2.5.5 amd64-portbld-freebsd13.0 [SSL (OpenSSL)] [LZO] [LZ4] [PKCS11] [MH/RECVDA] [AEAD] built on Feb  3 2022
2022-02-24 16:41:53 library versions: OpenSSL 1.1.1k-freebsd  24 Aug 2021, LZO 2.10
2022-02-24 16:41:53 WARNING: INSECURE cipher (BF-CBC) with block size less than 128 bit (64 bit).  This allows attacks like SWEET32.  Mitigate by using a --cipher with a larger block size (e.g. AES-256-CBC). Support for these insecure ciphers will be removed in OpenVPN 2.7.
2022-02-24 16:41:53 Outgoing Static Key Encryption: Cipher 'BF-CBC' initialized with 128 bit key
2022-02-24 16:41:53 WARNING: INSECURE cipher (BF-CBC) with block size less than 128 bit (64 bit).  This allows attacks like SWEET32.  Mitigate by using a --cipher with a larger block size (e.g. AES-256-CBC). Support for these insecure ciphers will be removed in OpenVPN 2.7.
2022-02-24 16:41:53 Outgoing Static Key Encryption: Using 160 bit message hash 'SHA1' for HMAC authentication
2022-02-24 16:41:53 Incoming Static Key Encryption: Cipher 'BF-CBC' initialized with 128 bit key
2022-02-24 16:41:53 WARNING: INSECURE cipher (BF-CBC) with block size less than 128 bit (64 bit).  This allows attacks like SWEET32.  Mitigate by using a --cipher with a larger block size (e.g. AES-256-CBC). Support for these insecure ciphers will be removed in OpenVPN 2.7.
2022-02-24 16:41:53 Incoming Static Key Encryption: Using 160 bit message hash 'SHA1' for HMAC authentication
2022-02-24 16:41:53 TUN/TAP device /dev/tun0 opened
2022-02-24 16:41:53 /sbin/ifconfig tun0 169.254.0.1 169.254.0.2 mtu 1500 netmask 255.255.255.255 up
2022-02-24 16:41:53 Could not determine IPv4/IPv6 protocol. Using AF_INET
2022-02-24 16:41:53 Socket Buffers: R=[42080->42080] S=[9216->9216]
2022-02-24 16:41:53 UDPv4 link local (bound): [AF_INET]127.0.0.1:2000
2022-02-24 16:41:53 UDPv4 link remote: [AF_UNSPEC]
2022-02-24 16:41:53 GID set to openvpn
2022-02-24 16:41:53 UID set to openvpn

root@default-freebsd-130-master-py3:/var/log/openvpn # cat myclient1.log
2022-02-24 16:41:54 Cipher negotiation is disabled since neither P2MP client nor server mode is enabled
2022-02-24 16:41:54 OpenVPN 2.5.5 amd64-portbld-freebsd13.0 [SSL (OpenSSL)] [LZO] [LZ4] [PKCS11] [MH/RECVDA] [AEAD] built on Feb  3 2022
2022-02-24 16:41:54 library versions: OpenSSL 1.1.1k-freebsd  24 Aug 2021, LZO 2.10
2022-02-24 16:41:54 WARNING: INSECURE cipher (BF-CBC) with block size less than 128 bit (64 bit).  This allows attacks like SWEET32.  Mitigate by using a --cipher with a larger block size (e.g. AES-256-CBC). Support for these insecure ciphers will be removed in OpenVPN 2.7.
2022-02-24 16:41:54 Outgoing Static Key Encryption: Cipher 'BF-CBC' initialized with 128 bit key
2022-02-24 16:41:54 WARNING: INSECURE cipher (BF-CBC) with block size less than 128 bit (64 bit).  This allows attacks like SWEET32.  Mitigate by using a --cipher with a larger block size (e.g. AES-256-CBC). Support for these insecure ciphers will be removed in OpenVPN 2.7.
2022-02-24 16:41:54 Outgoing Static Key Encryption: Using 160 bit message hash 'SHA1' for HMAC authentication
2022-02-24 16:41:54 Incoming Static Key Encryption: Cipher 'BF-CBC' initialized with 128 bit key
2022-02-24 16:41:54 WARNING: INSECURE cipher (BF-CBC) with block size less than 128 bit (64 bit).  This allows attacks like SWEET32.  Mitigate by using a --cipher with a larger block size (e.g. AES-256-CBC). Support for these insecure ciphers will be removed in OpenVPN 2.7.
2022-02-24 16:41:54 Incoming Static Key Encryption: Using 160 bit message hash 'SHA1' for HMAC authentication
2022-02-24 16:41:54 TUN/TAP device /dev/tun1 opened
2022-02-24 16:41:54 /sbin/ifconfig tun1 169.254.0.2 169.254.0.1 mtu 1500 netmask 255.255.255.255 up
ifconfig: ioctl (SIOCAIFADDR): File exists
2022-02-24 16:41:54 FreeBSD ifconfig failed: external program exited with error status: 1
2022-02-24 16:41:54 Exiting due to fatal error

With your familiarity with FreeBSD, perhaps you can figure out what needs to be done to resolve this.

Perhaps it would be even more useful if I add the CI to this PR, so that the changes can be made directly in this PR -- what do you think?

@alxwr alxwr force-pushed the configurable-kernel-module-name branch from a2ab1ad to 5c11719 Compare July 22, 2022 21:55
@alxwr
Copy link
Member Author

alxwr commented Jul 22, 2022

Hi @myii!

Sorry for the looooong delay! :-)
I'll try to get your fine test suite to pass, but I can't make any promises. :-)

@alxwr alxwr force-pushed the configurable-kernel-module-name branch from 5c11719 to 3aa5103 Compare August 21, 2023 16:13
@alxwr alxwr force-pushed the configurable-kernel-module-name branch from 3aa5103 to 148f0ae Compare January 2, 2024 20:34
@alxwr
Copy link
Member Author

alxwr commented Jan 2, 2024

Adding this to the above was enough:

     - require:
       - pkg: openvpn_pkgs
+      {%- if grains.os_family in ["FreeBSD"] %}
+      - file: openvpn_config_dir
+      {%- endif %}

@myii I took care of this problem, but also accounted for the situation where the admin might want to choose a different directory for dhparams.

ifconfig: ioctl (SIOCAIFADDR): File exists

I'm guessing this is because both server and client want to use the same tun device. This may be solvable if we configure tun2 for the client.

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