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

Resolves #7905 - Asynchronous Authentication Plugin #3908

Merged
merged 9 commits into from
Jul 26, 2018

Conversation

pdemonaco
Copy link
Contributor

@pdemonaco pdemonaco commented Feb 26, 2018

This correction replaces the call to auth-user-pass-verify with a plugin that has identical symantics and implements the deferred auth hook. The source of the plugin can be found in my other repository here.

Note that this has been tested with the following OpenVPN authentication configurations:

  • Remote Access (SSL/TLS + User Auth) - Local Database
  • Remote Access (User Auth) - External RADIUS + Duo MFA
  • Remote Access (SSL/TLS) - Obviously this isn't impacted.

I'm running this in production at my company as of this afternoon on 2.4.2-RELEASE. Note that the openvpn.inc file of that build was manually modified to match this change as the current master has significant other changes.

Please note that before this change can be merged some mechanism to build and install the actual plugin shared library must be added. I'm currently thinking it would make sense to create a BSD port dedicated to this purpose but I have a few questions before I go down that road.

  1. How can a port be added to the standard install?
  2. Should I attempt to contribute the port to the upstream FreeBSD repo or can I add it directly to the pfsense fork?

Alternatively, it might make sense to add the plugin as a patch to the openvpn port.

Any feedback would be appreciated.

Initial pass at new auth script which will be called by an openvpn
plugin. See https://github.com/pdemonaco/auth-script-openvpn for detail.
Switched to printf over echo to ensure strict POSIX compliance. Also
added some comments regarding the source of two unset variables.
Initial pass at new auth script which will be called by an openvpn
plugin. See https://github.com/pdemonaco/auth-script-openvpn for detail.
Switched to printf over echo to ensure strict POSIX compliance. Also
added some comments regarding the source of two unset variables.
Because I did something out of order?
Replaces the current auth-user-pass-verify directive with the new plugin
call in the config-file generation code.

Also modifies the new asynchronous script to use "echo -n". This
probably isn't necessary but it doesn't hurt.
Minor correction to the script header. Also switches back to printf
instead of echo -n for more broad base compatibility.
@jim-p
Copy link
Contributor

jim-p commented Feb 28, 2018

About the port, the best course of action would be to submit it to FreeBSD directly for inclusion in the ports tree. We prefer to keep as much as possible upstreamed. If that doesn't work out, however, you can submit it as a pull request to our https://github.com/pfsense/FreeBSD-ports/ repository directly.

@jim-p jim-p requested a review from rbgarga February 28, 2018 20:30
@pdemonaco
Copy link
Contributor Author

Thanks Jim, I'll get that process started.

After taking a closer look at the structure of the core pfsense repository I think I'll need to add the package to the poudriere_bulk list so it's included in the base build. Is this correct?

@rbgarga
Copy link
Member

rbgarga commented Mar 2, 2018

@pdemonaco You can add it as a dependency of security/pfSense port and it will be built.

I can help you to get it added into FreeBSD ports tree before we merge this change. Let me know when you submit the bugzilla ticket.

@pdemonaco
Copy link
Contributor Author

@rbgarga I'll let you know as soon as I have it ready - it may take me a bit as I haven't created a port for FreeBSD before and I'm not very familiar with the package manager.

I'm assuming you're referring to the FreeBSD bugzilla?

@rbgarga
Copy link
Member

rbgarga commented Mar 5, 2018

Yes

@pdemonaco
Copy link
Contributor Author

@rbgarga I've submitted my patch as bug 226492

I've also redeployed the plugin on my production environment using the library as compiled by the port on my build system.

@rbgarga
Copy link
Member

rbgarga commented Mar 12, 2018

Great! I noted another committer already took it so lets wait until it's added to the tree and then we cherry-pick it to pfSense repository

@pdemonaco
Copy link
Contributor Author

Sounds good - I'll update this when it's commited!

@pdemonaco
Copy link
Contributor Author

@rbgarga Do you have any thoughts on the inclusion or exclusion of the SSP_CFLAGS= stack-protector-strong flag in the port Makefile? The committer who picked up my bug report asked about it and I haven't heard back in a few days.

@pdemonaco
Copy link
Contributor Author

@rbgarga Is there anything we can do to speed up the review process on the FreeBSD side? I'd like to get this closed out before 2.4.4 if possible so I don't have to continue manually integrating my fix on each update.

@rbgarga
Copy link
Member

rbgarga commented Apr 5, 2018

Since another FreeBSD developer already took the ticket I cannot interfere. We need to wait. It will not take too long and it will be plenty time before 2.4.4

@pdemonaco
Copy link
Contributor Author

Fair enough. Is there anything I could do to make the port more likely to be approved based on the content of comment log for the FreeBSD ticket?

@pdemonaco
Copy link
Contributor Author

@rbgarga the FreeBSD port has been committed!

I'm assuming there are a couple more changes I'll need to make in this pull request to add a requirement for the inclusion of that port. Can you point me in the right direction?

pdemonaco added a commit to pdemonaco/FreeBSD-ports that referenced this pull request May 23, 2018
Adds the library
/usr/local/lib/openvpn/plugins/openvpn-plugin-auth-script.so as a
dependency to the security/pfSense package. This ensures that the
openvpn plugin is built and present on the system to support the
following pull request to mainline pfsense:
pfsense/pfsense#3908
@rbgarga
Copy link
Member

rbgarga commented Jul 17, 2018

Port has been added as a dependency of main pfSense meta-pkg on devel branch, it will be available on 2.4.4 snapshots soon. After that we can go ahead and start testing this change

@rbgarga rbgarga requested a review from jim-p July 17, 2018 11:34
@pdemonaco
Copy link
Contributor Author

Great! Let me know if there's anything I can do to help!

Copy link
Contributor

@jim-p jim-p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK, will definitely need thorough testing

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