Bug#982847: monitoring-plugins-standard: check_pgsql check of database name is too strict

2021-02-15 Thread Dan Langille
The patch I came up with is at:

https://git.langille.org/dvl/gist/src/branch/master/patch-plugins_check__pgsql.c

It solves the immediate problem.

See also https://github.com/bucardo/check_postgres which doesn't have this 
issue.
— 
Dan Langille
http://langille.org/



Bug#982847: [Pkg-nagios-devel] Bug#982847: monitoring-plugins-standard: check_pgsql check of database name is too strict

2021-02-15 Thread Florian Lohoff
On Mon, Feb 15, 2021 at 03:23:53PM +0100, Jan Wagner wrote:
> forwarded 982847 
> https://github.com/monitoring-plugins/monitoring-plugins/issues/1660
> forwarded 982847 
> https://github.com/monitoring-plugins/monitoring-plugins/issues/1661
> 
> Hi Florian,
> > -   break;
> > +   snprintf(dbName, NAMEDATALEN, "%s", optarg);
> > case 'l': /* login name */
> > if (!is_pg_logname (optarg))
> > usage2 (_("User name is not valid"), 
> > optarg);
> 
> same here.

No - good catch. 

Flo
-- 
Florian Lohoff f...@zz.de
"Autoritaetsduselei ist der groesste Feind der Wahrheit" - Albert Einstein


signature.asc
Description: PGP signature


Bug#982847: [Pkg-nagios-devel] Bug#982847: monitoring-plugins-standard: check_pgsql check of database name is too strict

2021-02-15 Thread Jan Wagner
forwarded 982847 
https://github.com/monitoring-plugins/monitoring-plugins/issues/1660
forwarded 982847 
https://github.com/monitoring-plugins/monitoring-plugins/issues/1661
thanks

Hi Florian,

thanks for bringing this to our attention.

Am 15.02.21 um 11:16 schrieb Florian Lohoff:
> diff --git a/plugins/check_pgsql.c b/plugins/check_pgsql.c
> index 11ce691..88cd029 100644
> --- a/plugins/check_pgsql.c
> +++ b/plugins/check_pgsql.c
> @@ -69,7 +69,6 @@ int process_arguments (int, char **);
>  int validate_arguments (void);
>  void print_usage (void);
>  void print_help (void);
> -int is_pg_dbname (char *);
>  int is_pg_logname (char *);
>  int do_query (PGconn *, char *);
>  
> @@ -344,11 +343,7 @@ process_arguments (int argc, char **argv)
>   pgport = optarg;
>   break;
>   case 'd': /* database name */
> - if (!is_pg_dbname (optarg)) /* checks length and valid 
> chars */
> - usage2 (_("Database name is not valid"), 
> optarg);
> - else /* we know length, and know optarg is terminated, 
> so us strcpy */
> - strcpy (dbName, optarg);
> - break;
> + strcpy (dbName, optarg);
>   case 'l': /* login name */
>   if (!is_pg_logname (optarg))
>   usage2 (_("User name is not valid"), optarg);
> @@ -408,45 +403,6 @@ validate_arguments ()
>   return OK;
>  }

are you sure it's a good idea to remove the break?

> @@ -344,11 +343,7 @@ process_arguments (int argc, char **argv)
>pgport = optarg;
> break;
> case 'd': /* database name */
> -   if (!is_pg_dbname (optarg)) /* checks length and 
> valid chars */
> -   usage2 (_("Database name is not valid"), 
> optarg);
> -   else /* we know length, and know optarg is 
> terminated, so us strcpy */
> -   strcpy (dbName, optarg);
> -   break;
> +   snprintf(dbName, NAMEDATALEN, "%s", optarg);
> case 'l': /* login name */
> if (!is_pg_logname (optarg))
> usage2 (_("User name is not valid"), optarg);

same here.

Many thanks, Jan.



Bug#982847: monitoring-plugins-standard: check_pgsql check of database name is too strict

2021-02-15 Thread Florian Lohoff
Package: monitoring-plugins-standard
Version: 2.3-1
Severity: normal

Hi *,
to reproduce create a Database called freshports.git and try to
use it with check_pgsql:

flo@p5:/tmp/monitoring-plugins-2.3$ /usr/lib/nagios/plugins/check_pgsql -d 
freshports.devgit -l flo
check_pgsql: Database name is not valid - freshports.devgit
Usage:
check_pgsql [-H ] [-P ] [-c ] [-w ]
 [-t ] [-d ] [-l ] [-p ]
[-q ] [-C ] [-W ]


flo@p5:/tmp/monitoring-plugins-2.3$ psql freshports.devgit
psql (13.1 (Debian 13.1-1+b1))
Type "help" for help.

freshports.devgit=# \d
Did not find any relations.
freshports.devgit=# 


The problem is that check_pgsql validates the Database name and has different 
assumptions
that postgres itself.


I fail to see a reason to validate the database name here. Postgres'es API 
should
do this - So i would suggest a fix like this by removing is_pg_dbname 
alltogether.



diff --git a/plugins/check_pgsql.c b/plugins/check_pgsql.c
index 11ce691..88cd029 100644
--- a/plugins/check_pgsql.c
+++ b/plugins/check_pgsql.c
@@ -69,7 +69,6 @@ int process_arguments (int, char **);
 int validate_arguments (void);
 void print_usage (void);
 void print_help (void);
-int is_pg_dbname (char *);
 int is_pg_logname (char *);
 int do_query (PGconn *, char *);
 
@@ -344,11 +343,7 @@ process_arguments (int argc, char **argv)
pgport = optarg;
break;
case 'd': /* database name */
-   if (!is_pg_dbname (optarg)) /* checks length and valid 
chars */
-   usage2 (_("Database name is not valid"), 
optarg);
-   else /* we know length, and know optarg is terminated, 
so us strcpy */
-   strcpy (dbName, optarg);
-   break;
+   strcpy (dbName, optarg);
case 'l': /* login name */
if (!is_pg_logname (optarg))
usage2 (_("User name is not valid"), optarg);
@@ -408,45 +403,6 @@ validate_arguments ()
return OK;
 }
 
-
-/**
-
-@@-
-
-is_pg_dbname
-
-_is_pg_dbname;
-
-Given a database name, this function returns TRUE if the string
-is a valid PostgreSQL database name, and returns false if it is
-not.
-
-Valid PostgreSQL database names are less than 
-characters long and consist of letters, numbers, and underscores. The
-first character cannot be a number, however.
-
-
--@@
-**/
-
-
-
-int
-is_pg_dbname (char *dbname)
-{
-   char txt[NAMEDATALEN];
-   char tmp[NAMEDATALEN];
-   if (strlen (dbname) > NAMEDATALEN - 1)
-   return (FALSE);
-   strncpy (txt, dbname, NAMEDATALEN - 1);
-   txt[NAMEDATALEN - 1] = 0;
-   if (sscanf (txt, "%[_a-zA-Z]%[^_a-zA-Z0-9-]", tmp, tmp) == 1)
-   return (TRUE);
-   if (sscanf (txt, "%[_a-zA-Z]%[_a-zA-Z0-9-]%[^_a-zA-Z0-9-]", tmp, tmp, 
tmp) ==
-   2) return (TRUE);
-   return (FALSE);
-}
-
 /**
 
 the tango program should eventually create an entity here based on the


-- System Information:
Debian Release: bullseye/sid
  APT prefers testing
  APT policy: (500, 'testing')
Architecture: amd64 (x86_64)

Kernel: Linux 5.10.0-3-amd64 (SMP w/16 CPU threads)
Kernel taint flags: TAINT_WARN, TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=de_DE.utf-8 (charmap=UTF-8), 
LANGUAGE=en_US:en
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages monitoring-plugins-standard depends on:
ii  libc6 2.31-9
ii  monitoring-plugins-basic  2.3-1
ii  ucf   3.0043

Versions of packages monitoring-plugins-standard recommends:
ii  bind9-dnsutils [dnsutils]  1:9.16.11-2
ii  bind9-host [host]  1:9.16.11-2
ii  dnsutils   1:9.16.11-2
ii  libcurl4   7.74.0-1
ii  libdbi10.9.0-6
ii  libldap-2.4-2  2.4.57+dfsg-1
ii  libmariadb31:10.5.8-3
ii  libnet-snmp-perl   6.0.1-6
ii  libpq5 13.1-1+b1
ii  libradcli4 1.2.11-1+b2
ii  libssl1.1  1.1.1i-3
ii  liburiparser1  0.9.4+dfsg-1
ii  rpcbind1.2.5-9
ii  smbclient  2:4.13.4+dfsg-1
ii  snmp   5.9+dfsg-3+b1
ii  sudo   1.9.5p2-2

Versions of packages monitoring-plugins-standard suggests:
ii  fping5.0-1
pn  icinga2  
ii  postfix  3.5.6-1
pn  qstat

-- no debconf information