Add stunnel package for pfSense 2.3 #135

djmarcin · 2016-05-25T07:27:58Z

Mostly straight copies of the files from the 2.2 package with the new 2.3 package metadata files.

No attempt has been made to clean up the existing code, notably in security/pfSense-pkg-stunnel/files/usr/local/pkg/stunnel.inc:

There is code checking for pfsense versions < 2.3 which can probably be deleted
The gui attempts to render colored text, but since it gets escaped simply renders text like <font color="#AABBCC">blahblah</font>. I couldn't figure out how to get colored text in the status page tables.

I am willing to work on those issues, but since this doesn't seem to have any major issues perhaps it is OK as a first approximation to get the stunnel package working again. If the pull can't be accepted as is, any pointers on how to fix the above issues would be greatly appreciated. I couldn't find anything promising in the convert to bootstrap docs.

rbgarga · 2016-12-02T19:37:43Z

security/pfSense-pkg-stunnel/Makefile

+MAINTAINER=	[email protected]
+COMMENT=	pfSense package stunnel
+
+LICENSE=	ESF


License changed to APACHE20

rbgarga · 2016-12-02T19:38:45Z

security/pfSense-pkg-stunnel/files/etc/inc/priv/stunnel.priv.inc

+	INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+	CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+	ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+	POSSIBILITY OF SUCH DAMAGE.


License has changed to APACHE20, replace text accordingly. You can see an example at:
https://github.com/pfsense/FreeBSD-ports/blob/devel/security/pfSense-pkg-sudo/files/etc/inc/priv/sudo.priv.inc#L2

rbgarga · 2016-12-02T19:39:01Z

security/pfSense-pkg-stunnel/files/usr/local/pkg/stunnel.inc

+	INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+	CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+	ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+	POSSIBILITY OF SUCH DAMAGE.


Fix license text

rbgarga · 2016-12-02T19:39:29Z

security/pfSense-pkg-stunnel/files/usr/local/pkg/stunnel.inc

+	define('STUNNEL_LOCALBASE', '/usr/pbi/stunnel-' . php_uname("m"));
+} else {
+	define('STUNNEL_LOCALBASE', '/usr/local');
+}


We don't need to care about pfSense 2.1 or 2.2 here, this conditional can be removed

rbgarga · 2016-12-02T19:41:04Z

security/pfSense-pkg-stunnel/files/usr/local/pkg/stunnel.inc

+}
+define('STUNNEL_ETCDIR', STUNNEL_LOCALBASE . "/etc/stunnel");
+if (!isset($_GET['id']) and !isset($_POST['id'])) {
+	if ($GLOBALS['config']['installedpackages']['stunnelcerts']['savemsg']) {


Remove all use of $GLOBALS. The standard way in pfSense is to declare 'global $config' and access $config direct

rbgarga · 2016-12-02T19:41:50Z

security/pfSense-pkg-stunnel/files/usr/local/pkg/stunnel.xml

+	INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+	CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+	ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+	POSSIBILITY OF SUCH DAMAGE.


Replace license text

rbgarga · 2016-12-02T19:42:07Z

security/pfSense-pkg-stunnel/files/usr/local/pkg/stunnel_certs.xml

+	INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+	CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+	ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+	POSSIBILITY OF SUCH DAMAGE.


Replace license text

rbgarga · 2016-12-02T19:42:24Z

security/pfSense-pkg-stunnel/files/usr/local/pkg/stunnel_certs.xml

+	]]>
+	</copyright>
+	<name>stunnelcerts</name>
+	<version>5.20.2</version>


Replace 5.20.2 by macro %%PKGVERSION%%

rbgarga · 2016-12-02T19:42:49Z

security/pfSense-pkg-stunnel/files/usr/local/pkg/stunnel.xml

+	]]>
+	</copyright>
+	<name>stunnel</name>
+	<version>5.20.3</version>


Replace 5.20.3 by %%PKGVERSION%%

rbgarga · 2016-12-02T19:43:20Z

security/pfSense-pkg-stunnel/Makefile

+	${INSTALL_DATA} ${FILESDIR}${DATADIR}/info.xml \
+		${STAGEDIR}${DATADIR}
+	@${REINPLACE_CMD} -i '' -e "s|%%PKGVERSION%%|${PKGVERSION}|" \
+		${STAGEDIR}${DATADIR}/info.xml


Add XML files that contain macro %%PKGVERSION%% to this REINPLACE_CMD statement

rbgarga · 2016-12-02T19:43:53Z

security/pfSense-pkg-stunnel/Makefile

+
+PORTNAME=	pfSense-pkg-stunnel
+PORTVERSION=	5.30
+PORTREVISION=	1


New packages shouldn't contain PORTREVISION line, please remove

doktornotor · 2016-12-13T14:23:27Z

@djmarcin I fixed most of the review comments (plus additional things unnoticed by @rbgarga) in djmarcin#1

I have not touched any of the $GLOBALS stuff since I think the package should use certs.inc and store certs for the package in System - Cert Manager, rather than reimplementing it.

djmarcin · 2016-12-13T17:09:04Z

I merged in the changes from @doktornotor

rbgarga · 2016-12-14T14:08:48Z

As suggested by @doktornotor, change it to use certs.inc instead of reimplementing it

rbgarga

As suggested by @doktornotor, change it to use certs.inc instead of reimplementing it

djmarcin · 2016-12-21T19:55:49Z

I'll probably be able to look at this over the weekend, but to be honest I'm not terribly familiar with the certs part of stunnel, I'm using it without certs.

…

On Wed, Dec 21, 2016 at 11:07 AM Renato Botelho ***@***.***> wrote: ***@***.**** requested changes on this pull request. As suggested by @doktornotor <https://github.com/doktornotor>, change it to use certs.inc instead of reimplementing it — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#135 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHQF8VJGdwL-KjEs1Kw2rzXTPQOotyvks5rKXgMgaJpZM4ImPOn> .

doktornotor · 2017-01-21T22:27:44Z

@djmarcin - Need some help here?

djmarcin · 2017-01-22T15:56:53Z

This fell off my radar, but I looked at it last night. To clarify the goal here, we want to eliminate the certs page in the stunnel config UI and just allow the user to choose from the system cert manager storage?

doktornotor · 2017-01-22T16:20:26Z

Yeah, exactly, the XML can just let users pick up certificates for each tunnel from those set up in the Cert Manager, see e.g. https://github.com/pfsense/FreeBSD-ports/blob/devel/www/pfSense-pkg-squid/files/usr/local/pkg/squid_reverse_general.xml

Plus if you need code to do something with the certificates, use certs.inc and the functions in there if possible.

djmarcin · 2017-01-24T08:33:27Z

Ok -- I've removed the custom certs implementation and replaced it with certs.inc functions. As far as I can tell, stunnel needs access to .pem files in order to function, and I couldn't figure out if the system cert manager provided access to such files -- I think not? So I pull the referenced certs on save and write out .pem files in the stunnel directory and delete them when they're no longer in use by stunnel.

As to whether or not stunnel actually works correctly with these certs, I think so but I haven't tested it rigorously. Will give it some more testing tomorrow.

doktornotor · 2017-01-24T10:04:26Z

Looks much better. (And yeah, you need to write out the certificates somewhere to filesystem, nothing wrong with that.)

djmarcin · 2017-01-31T08:28:59Z

Finally got a chance to do that testing. Verified that stunnel using a Let's Encrypt cert imported to cert manager successfully acted in server mode and provided SSL access to pfsense being served over the HTTP port, so it looks like this is working properly.

Forgive my ignorance, do I need to do anything to mark the requested changes done?

rbgarga · 2017-02-02T10:04:48Z

security/pfSense-pkg-stunnel/Makefile

+	${INSTALL_DATA} ${FILESDIR}/etc/inc/priv/stunnel.priv.inc \
+		${STAGEDIR}/etc/inc/priv
+	${INSTALL_DATA} ${FILESDIR}${PREFIX}/www/shortcuts/pkg_stunnel.inc \
+                ${STAGEDIR}${PREFIX}/www/shortcuts


Use TAB instead of spaces here, otherwise portlint will complain

Done (responding so it gets marked complete)

rbgarga · 2017-02-02T10:05:07Z

security/pfSense-pkg-stunnel/pkg-plist

+www/shortcuts/pkg_stunnel.inc
+/etc/inc/priv/stunnel.priv.inc
+%%DATADIR%%/info.xml
+@dir /etc/inc/priv


It's missing '@dir /etc/inc' item here

Done (responding so it gets marked complete)

djmarcin · 2017-02-02T11:10:43Z

Done.

I noticed something while doing this. The config file is being clobbered on installation by a version without tunnel definitions. I attempted to fix this by replacing the deleted lines in c9d58d9 with stunnel_save() but it still didn't work. The pfSense UI still shows the tunnels, and after saving any tunnel, all the tunnels get written out again, however it doesn't seem like the script is aware of this config during stunnel_install().

Is there another function I can implement that would be called after install but have access to the config stored by pfSense?

doktornotor · 2017-02-02T11:28:18Z

@djmarcin: Have considered adding global $config; to the function? (And really any functions that reference $config)

jim-p

Looks OK. I made a couple comments for suggested improvements but it wasn't anything fatal or worrying.

jim-p · 2017-02-02T15:15:46Z

security/pfSense-pkg-stunnel/files/usr/local/pkg/stunnel.inc

+}
+
+function stunnel_save($config) {
+	conf_mount_rw();


conf_mount_rw() and conf_mount_ro() calls can be removed. It is a no-op on 2.3 and irrelevant on 2.4

jim-p · 2017-02-02T15:27:28Z

security/pfSense-pkg-stunnel/files/usr/local/pkg/stunnel.inc

+function stunnel_install() {
+	safe_mkdir(STUNNEL_ETCDIR);
+
+	// Generate a self-signed default certificate.


You should check here to make sure such a cert does not already exist, otherwise any time the package is upgraded or reinstalled, it will make another similar certificate.

jim-p · 2017-02-02T15:29:35Z

security/pfSense-pkg-stunnel/files/usr/local/pkg/stunnel.inc

+		}
+	}
+
+	conf_mount_ro();


conf_mount_rw() and conf_mount_ro() calls can be removed. It is a no-op on 2.3 and irrelevant on 2.4

It's a noop on 2.4 but still used on 2.3 since 2.3 still supports nanobsd. I suggest to keep them for now so this package can be cherry-picked to 2.3

kept for now

djmarcin · 2017-02-03T07:00:36Z

There's just one problem with this which is that when the package is upgraded (well, with pkg install) it doesn't seem to start the service. It will only start the service after some edit is made.

djmarcin · 2017-02-06T08:09:06Z

It appears there is a bug with either pfSense or stunnel where start_service does not work in the context of 'pkg install' or 'pkg upgrade'. The bug causes the service not to daemonize and become adopted by init -- instead the processes is adopted by the pkg command and then killed when that exits. See my post in https://forum.pfsense.org/index.php?topic=111563.0 for some details.

The service is properly started on boot or on config changes though.

Added two minor cleanup edits to the branch. Let me know if you want me to squash these or anything.

rbgarga · 2017-02-22T11:14:04Z

@djmarcin package looks OK now but I believe squashing all these commits is a good idea and it'll make it easy to add it to 2.3.3 when it's properly tested for some time. Can you do it please?

Based on stunnel package from pfSense 2.2 with refactorings to: - Use built in cert manager - Update UI for new paradigm - Simplify startup scripts Co-authored-by: doktornotor <[email protected]>

djmarcin · 2017-03-26T17:18:01Z

Sorry for the delay, commits have been squashed to one commit, attribution for the changes by doktornotor via "Co-authored-by" in this commit.

* Add request_aref plugin for configuring behavior of request [] and []= methods (jeremyevans) * Make public plugin not add Content-Type header when serving 304 response for gzipped file (jeremyevans) * Make content_for call with block convert block result to string before passing to tilt (jeremyevans) (#135)

@sruckh

ChangeLog: Core Using vlucas/valitron for user input validation Bumped FontAwesome to version 6.2.0 (#141) PHP versions 7.4 is now the minimal supported versions, older versions are not supported anymore (#143) extended support for PHP 8.1 (#147) Separated some templates into application/views/templates/partials folder (#144) Removed Composer lock file from git repository To avoid any potential issues for users using different version of PHP, composer.lock has been removed from the Git repo Fixed how MVC is implemented by using psr-15 http-handler (#145) Added router and user authentication middlewares Using single pass psr-15 middleware for application routing and user authentication Disabling user authentication does not display a blank page anymore (#140) Improved how exceptions and errors are handled (#145) PHP errors and exception handler and renderer has been refactored (#148) Instantiate Session instance from the Core Controller (#149) Disabling users authentication does not create a fatalog error nor blank page anymore (#135) Dashboard Breadcrumb navigation is now hidden on home page (Dashboard) Jobs report Fixed error with elapsed time when a job haven't been started yet if a job is in pending status, elapsed time column will display 'n/a' Docker image Provided Docker image on Docker Hub (#153) Documentation Update documentation about deprecated version and general security information (#142) Updated / fixed documentation The FAQ has been fixed / updated Security Added security policy and documented know security vulnerabilities (#135 and #140) Fixed New feature(s) Thanks to @sruckh, @skidoo23 and all community users for their feedback, tests, help and bug reports

netgate-git-updates added the CLA label Jun 2, 2016

rbgarga requested changes Dec 2, 2016

View reviewed changes

rbgarga added the Changes requested label Dec 5, 2016

rbgarga self-assigned this Dec 5, 2016

rbgarga approved these changes Dec 14, 2016

View reviewed changes

rbgarga added Pending review/testing Changes requested and removed Changes requested labels Dec 14, 2016

rbgarga removed the Pending review/testing label Dec 14, 2016

rbgarga requested changes Dec 21, 2016

View reviewed changes

djmarcin force-pushed the devel branch 2 times, most recently from 4202fde to 53ecaa9 Compare January 23, 2017 08:11

djmarcin force-pushed the devel branch from 578946c to 81b6840 Compare January 24, 2017 08:36

rbgarga added Pending review/testing and removed Changes requested labels Feb 2, 2017

rbgarga approved these changes Feb 2, 2017

View reviewed changes

rbgarga requested a review from jim-p February 2, 2017 09:48

rbgarga requested changes Feb 2, 2017

View reviewed changes

rbgarga added the Changes requested label Feb 2, 2017

djmarcin force-pushed the devel branch from 53cd755 to 0ec5bec Compare February 2, 2017 10:29

jim-p approved these changes Feb 2, 2017

View reviewed changes

rbgarga removed the Changes requested label Feb 22, 2017

rbgarga approved these changes Feb 22, 2017

View reviewed changes

rbgarga removed the Pending review/testing label Feb 22, 2017

Add stunnel package for pfSense 2.3

b8bb46b

Based on stunnel package from pfSense 2.2 with refactorings to: - Use built in cert manager - Update UI for new paradigm - Simplify startup scripts Co-authored-by: doktornotor <[email protected]>

djmarcin force-pushed the devel branch from d3ea234 to b8bb46b Compare March 26, 2017 17:13

netgate-git-updates merged commit b8bb46b into pfsense:devel Apr 12, 2017

netgate-git-updates pushed a commit that referenced this pull request Apr 12, 2017

Merge pull request #135 from djmarcin/devel

b3442b1

Add stunnel package for pfSense 2.3 #135

Add stunnel package for pfSense 2.3 #135

Conversation

djmarcin commented May 25, 2016 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doktornotor commented Dec 13, 2016 • edited Loading

djmarcin commented Dec 13, 2016

rbgarga commented Dec 14, 2016

rbgarga left a comment

Choose a reason for hiding this comment

djmarcin commented Dec 21, 2016 via email

doktornotor commented Jan 21, 2017

djmarcin commented Jan 22, 2017 via email

doktornotor commented Jan 22, 2017

djmarcin commented Jan 24, 2017

doktornotor commented Jan 24, 2017

djmarcin commented Jan 31, 2017

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

djmarcin commented Feb 2, 2017

doktornotor commented Feb 2, 2017 • edited Loading

jim-p left a 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

djmarcin commented Feb 3, 2017

djmarcin commented Feb 6, 2017

rbgarga commented Feb 22, 2017

djmarcin commented Mar 26, 2017

djmarcin commented May 25, 2016 •

edited

Loading

doktornotor commented Dec 13, 2016 •

edited

Loading

doktornotor commented Feb 2, 2017 •

edited

Loading