NagiosEnterprises / nrpe

NRPE Agent
GNU General Public License v2.0
257 stars 133 forks source link

SSL cleanups (with a few BSD odds & ends) #276

Open dougnazar opened 1 year ago

dougnazar commented 1 year ago

Various patches cleaning up the SSL handling:

This has been compile tested on the following configurations:

It's been running in production (along with my other patches) on one of my Gentoo boxes. The others have been compile (and minimally run) tested only.

tsadpbb commented 3 weeks ago

Bike shedding a little bit, but could you add the ssl.o and the utils.o to the gitignore? Also, I noticed ssl.o doesn't get removed from make clean.

Edit: Could you also add generate_dh_params to the gitignore?

tsadpbb commented 3 weeks ago

With the Makefile command

check_nrpe: 
        $(srcdir)/check_nrpe.c utils.o $(SRC_INCLUDE)/utils.h $(CFG_INCLUDE)/common.h $(CFG_INCLUDE)/config.h $(SSL_OBJS)
    $(CC) $(CFLAGS) -o $@ $(srcdir)/check_nrpe.c utils.o $(SSL_OBJS) $(LDFLAGS) $(SOCKETLIBS) $(SNPRINTF_O) $(OTHERLIBS)

Do you think you would ever need to specify the directory that utils.o is in? Something like this

check_nrpe: $(srcdir)/check_nrpe.c $(srcdir)/utils.o $(SRC_INCLUDE)/utils.h $(CFG_INCLUDE)/common.h $(CFG_INCLUDE)/config.h $(srcdir)/$(SSL_OBJS)
    $(CC) $(CFLAGS) -o $@ $(srcdir)/check_nrpe.c $(srcdir)/utils.o $(srcdir)/$(SSL_OBJS) $(LDFLAGS) $(SOCKETLIBS) $(SNPRINTF_O) $(OTHERLIBS)

This applies to the nrpe command as well

tsadpbb commented 3 weeks ago

Should the auto_dh be enabled by default or should it be disabled by default to conform with backwards compatibility?

dougnazar commented 3 weeks ago

Bike shedding a little bit, but could you add the ssl.o and the utils.o to the gitignore? Also, I noticed ssl.o doesn't get removed from make clean.

Edit: Could you also add generate_dh_params to the gitignore?

I did most of my testing in separate build directories so I didn't notice. I had just assumed all *.o files were ignored. I'll add a wild card and generate_dh_params.

I missed the ssl.o on clean. Easy enough to fix.

Do you think you would ever need to specify the directory that utils.o is in? Something like this

No. utils.o is in the build (current) directory, we only need to refer back to the source directory for *.c, *.h or any other shipped file.

Should the auto_dh be enabled by default or should it be disabled by default to conform with backwards compatibility?

It's a bit of an effort to remember, but I don't think I changed backwards compatibility here. It defaulted to always generating a DH key, I just switched to auto keying on setups that supported it. It should support all the same ciphers suites and connections to older versions as before.

I believe (sorry, it's been awhile) I tested various settings of ssl_use_adh as well as a few older versions of nrpe to check the legacy cases, but my testing definitely wasn't exhaustive.