[Buildroot] [PATCH 1/1] package/certmonger: add certmonger package

[Buildroot] [PATCH 1/1] package/certmonger: add certmonger package

[Ayrton Leyssens]

[-- Attachment #1.1: Type: text/plain, Size: 3697 bytes --]

Added support for the certmonger package (version 0.79.20).
Certmonger is used to monitor certificates and can refresh the certificate with the help of a CA.
In today's age, cybersecurity is a critical need for network connected devices.
Certmonger can help maintain all the certificate stuff so the user itself has less to worry about.

Signed-off-by: Ayrton aleyssens@idtech.be<mailto:aleyssens@idtech.be>
---
package/Config.in                  |  1 +
package/certmonger/Config.in       | 19 +++++++++++++++++++
package/certmonger/certmonger.hash |  2 ++
package/certmonger/certmonger.mk   | 25 +++++++++++++++++++++++++
4 files changed, 47 insertions(+)
create mode 100644 package/certmonger/Config.in
create mode 100644 package/certmonger/certmonger.hash
create mode 100644 package/certmonger/certmonger.mk

diff --git a/package/Config.in b/package/Config.in
index d4cc03dcdb..73efaae622 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2688,6 +2688,7 @@ endif
               source "package/zabbix/Config.in"
               source "package/zeek/Config.in"
               source "package/znc/Config.in"
+             source "package/certmonger/Config.in"
 endmenu
diff --git a/package/certmonger/Config.in b/package/certmonger/Config.in
new file mode 100644
index 0000000000..9d0e112b5a
--- /dev/null
+++ b/package/certmonger/Config.in
@@ -0,0 +1,19 @@
+config BR2_PACKAGE_CERTMONGER
+             bool "Certmonger"
+             select BR2_PACKAGE_LIBTALLOC
+             select BR2_PACKAGE_LIBTEVENT
+             select BR2_PACKAGE_DBUS
+             select BR2_PACKAGE_LIBNSS
+             select BR2_PACKAGE_LIBXML2
+             select BR2_PACKAGE_LIBCURL
+             select BR2_PACKAGE_JANSSON
+             select BR2_PACKAGE_LIBKRB5
+             select BR2_PACKAGE_POPT
+             select BR2_PACKAGE_LIBOPENSSL
+             select BR2_PACKAGE_POPT
+             select BR2_PACKAGE_OPENLDAP
+             help
+               Certmonger is a service which attempts to simplify
+               interaction with certifying authorities (CAs) on
+               networks which use public-key infrastructure (PKI).
+
diff --git a/package/certmonger/certmonger.hash b/package/certmonger/certmonger.hash
new file mode 100644
index 0000000000..653ba1dcfc
--- /dev/null
+++ b/package/certmonger/certmonger.hash
@@ -0,0 +1,2 @@
+# Locally calculated
+sha256  1e66094ef4130fc15c8acf5abfca097c2f8f40f57f699f80bb14e9941260d473  certmonger-0.79.20-git4.tar.gz
diff --git a/package/certmonger/certmonger.mk b/package/certmonger/certmonger.mk
new file mode 100644
index 0000000000..c6a9c179e4
--- /dev/null
+++ b/package/certmonger/certmonger.mk
@@ -0,0 +1,25 @@
+################################################################################
+#
+# CERTMONGER
+#
+################################################################################
+
+CERTMONGER_VERSION = 0.79.20
+CERTMONGER_SITE = https://pagure.io/certmonger.git
+CERTMONGER_LICENSE_FILES = LICENSE
+CERTMONGER_NAME = certmonger
+CERTMONGER_SITE_METHOD = git
+CERTMONGER_DEPENDENCIES = openssl dbus libtalloc libtevent libnss libcurl jansson libkrb5 openldap popt libxml2
+
+MY_CFLAGS = $(TARGET_CFLAGS) -lssl -ldbus-1 -ltalloc -ltevent -lnss -lcurl -ljansson -lkrb5 -lldap -lpopt -lxml2 -lcom_err
+
+define CERTMONGER_RUN_AUTOGEN
+             cd $(@D) && PATH=$(BR_PATH) ./autogen.sh
+endef
+CERTMONGER_PRE_CONFIGURE_HOOKS += CERTMONGER_RUN_AUTOGEN
+
+define CERTMONGER_BUILD_CMDS
+             $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) CXX="$(TARGET_CXX)" CFLAGS="$(MY_CFLAGS)"
+endef
+
+$(eval $(autotools-package))
--
2.43.0

[-- Attachment #1.2: Type: text/html, Size: 12545 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/certmonger: add certmonger package

[Thomas Petazzoni via buildroot]

Hello Ayrton,

Thanks for your patch! Here as well, I have a number of comments.

First, same as the other patch, the commit title should be:

	package/certmonger: new package

On Mon, 21 Oct 2024 11:56:54 +0000
Ayrton Leyssens <aleyssens@idtech.be> wrote:

> Added support for the certmonger package (version 0.79.20).
> Certmonger is used to monitor certificates and can refresh the certificate with the help of a CA.
> In today's age, cybersecurity is a critical need for network connected devices.
> Certmonger can help maintain all the certificate stuff so the user itself has less to worry about.

Please wrap the commit log to ~80 columns.

I think the sentence "In today's age, cybersecurity is a critical need
for network connected devices." is quite useless in such a commit log.

> Signed-off-by: Ayrton aleyssens@idtech.be<mailto:aleyssens@idtech.be>

Please fix your Signed-off-by line, like explained on the libevent
patch.

> ---
> package/Config.in                  |  1 +
> package/certmonger/Config.in       | 19 +++++++++++++++++++
> package/certmonger/certmonger.hash |  2 ++
> package/certmonger/certmonger.mk   | 25 +++++++++++++++++++++++++
> 4 files changed, 47 insertions(+)

Please add an entry in the DEVELOPERS file.

> create mode 100644 package/certmonger/Config.in
> create mode 100644 package/certmonger/certmonger.hash
> create mode 100644 package/certmonger/certmonger.mk
> 
> diff --git a/package/Config.in b/package/Config.in
> index d4cc03dcdb..73efaae622 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -2688,6 +2688,7 @@ endif
>                source "package/zabbix/Config.in"
>                source "package/zeek/Config.in"
>                source "package/znc/Config.in"
> +             source "package/certmonger/Config.in"

Indentation and alphabetic ordering should be fixed here.

>  endmenu
> diff --git a/package/certmonger/Config.in b/package/certmonger/Config.in
> new file mode 100644
> index 0000000000..9d0e112b5a
> --- /dev/null
> +++ b/package/certmonger/Config.in
> @@ -0,0 +1,19 @@
> +config BR2_PACKAGE_CERTMONGER
> +             bool "Certmonger"
> +             select BR2_PACKAGE_LIBTALLOC
> +             select BR2_PACKAGE_LIBTEVENT
> +             select BR2_PACKAGE_DBUS
> +             select BR2_PACKAGE_LIBNSS
> +             select BR2_PACKAGE_LIBXML2
> +             select BR2_PACKAGE_LIBCURL
> +             select BR2_PACKAGE_JANSSON
> +             select BR2_PACKAGE_LIBKRB5
> +             select BR2_PACKAGE_POPT
> +             select BR2_PACKAGE_LIBOPENSSL
> +             select BR2_PACKAGE_POPT
> +             select BR2_PACKAGE_OPENLDAP
> +             help

Indentation of bool, select and help should be one tab.

The select entries should be alphabetically sorted.

Also, are you sure all those dependencies are mandatory dependencies,
i.e certmonger cannot be built without them?

Finally, many of those options that you select have "depends on". You
need to replicate them here. For example, BR2_PACKAGE_OPENLDAP has:

        depends on BR2_USE_WCHAR
        depends on BR2_USE_MMU # needs fork()

so you need to add:

        depends on BR2_USE_WCHAR # openldap
        depends on BR2_USE_MMU # openldap

to this BR2_PACKAGE_CERTMONGER option. And do that for all other
options you are select-ing.


> +               Certmonger is a service which attempts to simplify
> +               interaction with certifying authorities (CAs) on
> +               networks which use public-key infrastructure (PKI).

Indentation of help text should be one tab + two spaces.

(Make sure to run "make check-package" to find all those small coding
style details).

The help text should also be followed by the upstream URL of the
project, see all other packages.

> +
> diff --git a/package/certmonger/certmonger.hash b/package/certmonger/certmonger.hash
> new file mode 100644
> index 0000000000..653ba1dcfc
> --- /dev/null
> +++ b/package/certmonger/certmonger.hash
> @@ -0,0 +1,2 @@
> +# Locally calculated
> +sha256  1e66094ef4130fc15c8acf5abfca097c2f8f40f57f699f80bb14e9941260d473  certmonger-0.79.20-git4.tar.gz

Please add the hash of the license file.

> diff --git a/package/certmonger/certmonger.mk b/package/certmonger/certmonger.mk
> new file mode 100644
> index 0000000000..c6a9c179e4
> --- /dev/null
> +++ b/package/certmonger/certmonger.mk
> @@ -0,0 +1,25 @@
> +################################################################################
> +#
> +# CERTMONGER

lower case.

> +#
> +################################################################################
> +
> +CERTMONGER_VERSION = 0.79.20
> +CERTMONGER_SITE = https://pagure.io/certmonger.git
> +CERTMONGER_LICENSE_FILES = LICENSE

Please add CERTMONGER_LICENSE, which it seems is "GPL-3.0+ with OpenSSL
exception".

> +CERTMONGER_NAME = certmonger

Not useful.

> +CERTMONGER_SITE_METHOD = git
> +CERTMONGER_DEPENDENCIES = openssl dbus libtalloc libtevent libnss libcurl jansson libkrb5 openldap popt libxml2

Please sort alphabetically.

> +
> +MY_CFLAGS = $(TARGET_CFLAGS) -lssl -ldbus-1 -ltalloc -ltevent -lnss -lcurl -ljansson -lkrb5 -lldap -lpopt -lxml2 -lcom_err

This variable shouldn't be named MY_CFLAGS: all variables in a package
should start with the package name, i.e here CERTMONGER_<something>.
However, why do you need this special CFLAGS variable? It only contains
libraries that should be detected by the configure script of
certmonger, so this variable really shouldn't be needed.

> +define CERTMONGER_RUN_AUTOGEN
> +             cd $(@D) && PATH=$(BR_PATH) ./autogen.sh
> +endef
> +CERTMONGER_PRE_CONFIGURE_HOOKS += CERTMONGER_RUN_AUTOGEN

Please use instead CERTMONGER_AUTORECONF = YES instead. Indeed what you
did doesn't work, because it doesn't add host-autoconf, host-automake
and host-libtool to certmonger's dependencies.

> +define CERTMONGER_BUILD_CMDS
> +             $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) CXX="$(TARGET_CXX)" CFLAGS="$(MY_CFLAGS)"

Why is this needed?

certmonger seems like a quite complicated package. Is there a way to
easily test it? If so, it would be good to add a runtime test in
support/testing/.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/certmonger: add certmonger package

[Ayrton Leyssens]

Hi Thomas

Thanks for the feedback, I will send in a new patch series soon.

> Hello Ayrton,
> 
> Thanks for your patch! Here as well, I have a number of comments.
> 
> First, same as the other patch, the commit title should be:
> 
> 	package/certmonger: new package
> 
> On Mon, 21 Oct 2024 11:56:54 +0000
> Ayrton Leyssens <aleyssens@idtech.be> wrote:
> 
> > Added support for the certmonger package (version 0.79.20).
> > Certmonger is used to monitor certificates and can refresh the certificate with the help of a CA.
> > In today's age, cybersecurity is a critical need for network connected devices.
> > Certmonger can help maintain all the certificate stuff so the user itself has less to worry about.
> 
> Please wrap the commit log to ~80 columns.
> 
> I think the sentence "In today's age, cybersecurity is a critical need for network connected devices." is quite useless in such a commit log.
> 
> > Signed-off-by: Ayrton aleyssens@idtech.be<mailto:aleyssens@idtech.be>
> 
> Please fix your Signed-off-by line, like explained on the libevent patch.
> 
> > ---
> > package/Config.in                  |  1 +
> > package/certmonger/Config.in       | 19 +++++++++++++++++++
> > package/certmonger/certmonger.hash |  2 ++
> > package/certmonger/certmonger.mk   | 25 +++++++++++++++++++++++++
> > 4 files changed, 47 insertions(+)
> 
> Please add an entry in the DEVELOPERS file.
> 
> > create mode 100644 package/certmonger/Config.in create mode 100644 
> > package/certmonger/certmonger.hash
> > create mode 100644 package/certmonger/certmonger.mk
> > 
> > diff --git a/package/Config.in b/package/Config.in index 
> > d4cc03dcdb..73efaae622 100644
> > --- a/package/Config.in
> > +++ b/package/Config.in
> > @@ -2688,6 +2688,7 @@ endif
> >                source "package/zabbix/Config.in"
> >                source "package/zeek/Config.in"
> >                source "package/znc/Config.in"
> > +             source "package/certmonger/Config.in"
> 
> Indentation and alphabetic ordering should be fixed here.
> 
> >  endmenu
> > diff --git a/package/certmonger/Config.in 
> > b/package/certmonger/Config.in new file mode 100644 index 
> > 0000000000..9d0e112b5a
> > --- /dev/null
> > +++ b/package/certmonger/Config.in
> > @@ -0,0 +1,19 @@
> > +config BR2_PACKAGE_CERTMONGER
> > +             bool "Certmonger"
> > +             select BR2_PACKAGE_LIBTALLOC
> > +             select BR2_PACKAGE_LIBTEVENT
> > +             select BR2_PACKAGE_DBUS
> > +             select BR2_PACKAGE_LIBNSS
> > +             select BR2_PACKAGE_LIBXML2
> > +             select BR2_PACKAGE_LIBCURL
> > +             select BR2_PACKAGE_JANSSON
> > +             select BR2_PACKAGE_LIBKRB5
> > +             select BR2_PACKAGE_POPT
> > +             select BR2_PACKAGE_LIBOPENSSL
> > +             select BR2_PACKAGE_POPT
> > +             select BR2_PACKAGE_OPENLDAP
> > +             help
> 
> Indentation of bool, select and help should be one tab.
> 
> The select entries should be alphabetically sorted.
> 
> Also, are you sure all those dependencies are mandatory dependencies, i.e certmonger cannot be built without them?
Unfortunately, yes.
Like you said, Certmonger is quite a complex package and has a lot of dependencies.
It might be possible to make 'sub-selections' but this should be something implemented by the Certmonger developers.
I am just the messenger adding the package to Buildroot.
> 
> Finally, many of those options that you select have "depends on". You need to replicate them here. For example, BR2_PACKAGE_OPENLDAP has:
> 
>         depends on BR2_USE_WCHAR
>         depends on BR2_USE_MMU # needs fork()
> 
> so you need to add:
> 
>         depends on BR2_USE_WCHAR # openldap
>         depends on BR2_USE_MMU # openldap
> 
> to this BR2_PACKAGE_CERTMONGER option. And do that for all other options you are select-ing.
> 
> 
> > +               Certmonger is a service which attempts to simplify
> > +               interaction with certifying authorities (CAs) on
> > +               networks which use public-key infrastructure (PKI).
> 
> Indentation of help text should be one tab + two spaces.
> 
> (Make sure to run "make check-package" to find all those small coding style details).
> 
> The help text should also be followed by the upstream URL of the project, see all other packages.
> 
> > +
> > diff --git a/package/certmonger/certmonger.hash 
> > b/package/certmonger/certmonger.hash
> > new file mode 100644
> > index 0000000000..653ba1dcfc
> > --- /dev/null
> > +++ b/package/certmonger/certmonger.hash
> > @@ -0,0 +1,2 @@
> > +# Locally calculated
> > +sha256  
> > +1e66094ef4130fc15c8acf5abfca097c2f8f40f57f699f80bb14e9941260d473  
> > +certmonger-0.79.20-git4.tar.gz
> 
> Please add the hash of the license file.
> 
> > diff --git a/package/certmonger/certmonger.mk 
> > b/package/certmonger/certmonger.mk
> > new file mode 100644
> > index 0000000000..c6a9c179e4
> > --- /dev/null
> > +++ b/package/certmonger/certmonger.mk
> > @@ -0,0 +1,25 @@
> > +#####################################################################
> > +###########
> > +#
> > +# CERTMONGER
> 
> lower case.
> 
> > +#
> > +#####################################################################
> > +###########
> > +
> > +CERTMONGER_VERSION = 0.79.20
> > +CERTMONGER_SITE = https://pagure.io/certmonger.git 
> > +CERTMONGER_LICENSE_FILES = LICENSE
> 
> Please add CERTMONGER_LICENSE, which it seems is "GPL-3.0+ with OpenSSL exception".
> 
> > +CERTMONGER_NAME = certmonger
> 
> Not useful.
> 
> > +CERTMONGER_SITE_METHOD = git
> > +CERTMONGER_DEPENDENCIES = openssl dbus libtalloc libtevent libnss 
> > +libcurl jansson libkrb5 openldap popt libxml2
> 
> Please sort alphabetically.
> 
> > +
> > +MY_CFLAGS = $(TARGET_CFLAGS) -lssl -ldbus-1 -ltalloc -ltevent -lnss 
> > +-lcurl -ljansson -lkrb5 -lldap -lpopt -lxml2 -lcom_err
> 
> This variable shouldn't be named MY_CFLAGS: all variables in a package should start with the package name, i.e here CERTMONGER_<something>.
> However, why do you need this special CFLAGS variable? It only contains libraries that should be detected by the configure script of certmonger, so this variable really shouldn't be needed.
> 
> > +define CERTMONGER_RUN_AUTOGEN
> > +             cd $(@D) && PATH=$(BR_PATH) ./autogen.sh endef 
> > +CERTMONGER_PRE_CONFIGURE_HOOKS += CERTMONGER_RUN_AUTOGEN
> 
> Please use instead CERTMONGER_AUTORECONF = YES instead. Indeed what you did doesn't work, because it doesn't add host-autoconf, host-automake and host-libtool to certmonger's dependencies.
> 
> > +define CERTMONGER_BUILD_CMDS
> > +             $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) CXX="$(TARGET_CXX)" CFLAGS="$(MY_CFLAGS)"
> 
> Why is this needed?
> 
> certmonger seems like a quite complicated package. Is there a way to easily test it? If so, it would be good to add a runtime test in support/testing/.
I will look into this but at the moment I have no idea of a runtime test.
Testing certificate exchange might be a difficult one ;).
> 
> Thanks a lot!
> 
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin Embedded Linux and Kernel engineering and training https://bootlin.com
>

I will soon send a new patchseries containing certmonger and libtevent with resolved remarks.

Thanks

Ayrton
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/certmonger: add certmonger package

[Thomas Petazzoni via buildroot]

Hello Ayrton,

On Thu, 24 Oct 2024 10:14:04 +0000
Ayrton Leyssens <aleyssens@idtech.be> wrote:

> > Also, are you sure all those dependencies are mandatory dependencies, i.e certmonger cannot be built without them?  
>
> Unfortunately, yes.
> Like you said, Certmonger is quite a complex package and has a lot of dependencies.
> It might be possible to make 'sub-selections' but this should be something implemented by the Certmonger developers.
> I am just the messenger adding the package to Buildroot.

Sure, if those dependencies are mandatory, they are, and the Buildroot
package should reflect that. Since they were really numerous, I thought
I would ask to make sure you had looked into this carefully.

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot