Re: snmp(1): Fix unsafe defaults
On Wed, 2021-08-11 at 18:59 +0100, Stuart Henderson wrote:
> On 2021/08/11 19:34, Martijn van Duren wrote:
> > On Wed, 2021-08-11 at 18:03 +0100, Stuart Henderson wrote:
> > > On 2021/08/11 16:35, Martijn van Duren wrote:
> > > > Following snmpd, remove the public default community and move to snmpv3
> > > > by default. This is also what net-snmp does. I originally chose this
> > > > default because that's what snmpctl did and it allowed for easier
> > > > interoperability with snmpd(8).
> > >
> > > v3 by default makes sense to me.
> > >
> > > I'm not sure how much it buys to remove the default community in snmp(1),
> > > though, there doesn't seem a lot of benefit to removing it?
> >
> > My reasoning being that setting having public the default in snmp(1)
> > might encourage users to set public in snmpd(8) as well, which is what
> > we tried to discourage.
>
> Hmm maybe. I won't object to that.
>
>
Forgot the manpage bits.
OK?
martijn@
Index: snmp.1
===
RCS file: /cvs/src/usr.bin/snmp/snmp.1,v
retrieving revision 1.19
diff -u -p -r1.19 snmp.1
--- snmp.1 8 Aug 2021 13:41:26 - 1.19
+++ snmp.1 11 Aug 2021 18:22:18 -
@@ -303,12 +303,11 @@ Show how long it took to walk the entire
Set the
.Ar community
string.
-Defaults to
-.Cm public .
This option is only used by
.Fl v Cm 1
and
-.Fl v Cm 2c .
+.Fl v Cm 2c
+and has no default.
.It Fl e Ar secengineid
The USM security engine id.
Under normal circumstances this value is discovered via snmpv3 discovery and
@@ -425,7 +424,7 @@ to either
or
.Cm 3 .
Currently defaults to
-.Cm 2c .
+.Cm 3 .
.It Fl X Ar privpass
The privacy password for the user.
This will be tansformed to
Index: snmpc.c
===
RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
retrieving revision 1.35
diff -u -p -r1.35 snmpc.c
--- snmpc.c 8 Aug 2021 13:41:26 - 1.35
+++ snmpc.c 11 Aug 2021 18:22:18 -
@@ -84,12 +84,12 @@ struct snmp_app snmp_apps[] = {
};
struct snmp_app *snmp_app = NULL;
-char *community = "public";
+char *community = NULL;
struct snmp_v3 *v3;
char *mib = "mib_2";
int retries = 5;
int timeout = 1;
-enum snmp_version version = SNMP_V2C;
+enum snmp_version version = SNMP_V3;
int print_equals = 1;
int print_varbind_only = 0;
int print_summary = 0;
@@ -468,7 +468,10 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
- if (version == SNMP_V3) {
+ if (version == SNMP_V1 || version == SNMP_V2C) {
+ if (community == NULL || community[0] == '\0')
+ errx(1, "No community name specified.");
+ } else if (version == SNMP_V3) {
/* Setup USM */
if (user == NULL || user[0] == '\0')
errx(1, "No securityName specified");
Re: snmp(1): Fix unsafe defaults
On Wed, 2021-08-11 at 18:59 +0100, Stuart Henderson wrote: > On 2021/08/11 19:34, Martijn van Duren wrote: > > On Wed, 2021-08-11 at 18:03 +0100, Stuart Henderson wrote: > > > On 2021/08/11 16:35, Martijn van Duren wrote: > > > > Following snmpd, remove the public default community and move to snmpv3 > > > > by default. This is also what net-snmp does. I originally chose this > > > > default because that's what snmpctl did and it allowed for easier > > > > interoperability with snmpd(8). > > > > > > v3 by default makes sense to me. > > > > > > I'm not sure how much it buys to remove the default community in snmp(1), > > > though, there doesn't seem a lot of benefit to removing it? > > > > My reasoning being that setting having public the default in snmp(1) > > might encourage users to set public in snmpd(8) as well, which is what > > we tried to discourage. > > Hmm maybe. I won't object to that. > > > And it's easy enough to do something like > > alias snmp_get="snmp get -v2c -ccommunity" > > in .profile for interactive use > > and walk, bulkwalk, df, [...] > > FWIW I have this for now. > > - > #!/bin/ksh > if [[ -z $2 ]]; then > /usr/bin/snmp 2>&1 | sed "s/snmp/`basename $0`/" >&2 > exit 1 > fi > cmd=$1 > shift > exec /usr/bin/snmp $cmd -v 3 -l authPriv -u xxx [etc] $* > - > > > and in scripts you always want to be > > explicit with such parameters. > > Maybe. I do quite like keeping the secrets out of ps/top though. > > While I'm thinking about it, thoughts on this? No objection from me. OK martijn@ > > Index: snmpd.conf.5 > === > RCS file: /cvs/src/usr.sbin/snmpd/snmpd.conf.5,v > retrieving revision 1.56 > diff -u -p -r1.56 snmpd.conf.5 > --- snmpd.conf.510 Aug 2021 07:53:57 - 1.56 > +++ snmpd.conf.511 Aug 2021 17:57:53 - > @@ -402,12 +402,13 @@ Example configuration file. > .Sh EXAMPLES > The following example will tell > .Xr snmpd 8 > -to listen on localhost for SNMPv2c messages only with the public community, > -override the default system OID, set the magic services value and provides > some > -custom OID values: > +to listen on localhost for SNMPv2c messages only with the community > +.Dq 8LHQtm1QLGzk , > +override the default system OID, set the magic services value, > +and provide some custom OID values: > .Bd -literal -offset indent > listen on 127.0.0.1 snmpv2c > -read-only community public > +read-only community 8LHQtm1QLGzk > > system oid 1.3.6.1.4.1.30155.23.2 > system services 74 >
Re: snmp(1): Fix unsafe defaults
On 2021/08/11 19:34, Martijn van Duren wrote: > On Wed, 2021-08-11 at 18:03 +0100, Stuart Henderson wrote: > > On 2021/08/11 16:35, Martijn van Duren wrote: > > > Following snmpd, remove the public default community and move to snmpv3 > > > by default. This is also what net-snmp does. I originally chose this > > > default because that's what snmpctl did and it allowed for easier > > > interoperability with snmpd(8). > > > > v3 by default makes sense to me. > > > > I'm not sure how much it buys to remove the default community in snmp(1), > > though, there doesn't seem a lot of benefit to removing it? > > My reasoning being that setting having public the default in snmp(1) > might encourage users to set public in snmpd(8) as well, which is what > we tried to discourage. Hmm maybe. I won't object to that. > And it's easy enough to do something like > alias snmp_get="snmp get -v2c -ccommunity" > in .profile for interactive use and walk, bulkwalk, df, [...] FWIW I have this for now. - #!/bin/ksh if [[ -z $2 ]]; then /usr/bin/snmp 2>&1 | sed "s/snmp/`basename $0`/" >&2 exit 1 fi cmd=$1 shift exec /usr/bin/snmp $cmd -v 3 -l authPriv -u xxx [etc] $* - > and in scripts you always want to be > explicit with such parameters. Maybe. I do quite like keeping the secrets out of ps/top though. While I'm thinking about it, thoughts on this? Index: snmpd.conf.5 === RCS file: /cvs/src/usr.sbin/snmpd/snmpd.conf.5,v retrieving revision 1.56 diff -u -p -r1.56 snmpd.conf.5 --- snmpd.conf.510 Aug 2021 07:53:57 - 1.56 +++ snmpd.conf.511 Aug 2021 17:57:53 - @@ -402,12 +402,13 @@ Example configuration file. .Sh EXAMPLES The following example will tell .Xr snmpd 8 -to listen on localhost for SNMPv2c messages only with the public community, -override the default system OID, set the magic services value and provides some -custom OID values: +to listen on localhost for SNMPv2c messages only with the community +.Dq 8LHQtm1QLGzk , +override the default system OID, set the magic services value, +and provide some custom OID values: .Bd -literal -offset indent listen on 127.0.0.1 snmpv2c -read-only community public +read-only community 8LHQtm1QLGzk system oid 1.3.6.1.4.1.30155.23.2 system services 74
Re: snmp(1): Fix unsafe defaults
On Wed, 2021-08-11 at 18:03 +0100, Stuart Henderson wrote:
> On 2021/08/11 16:35, Martijn van Duren wrote:
> > Following snmpd, remove the public default community and move to snmpv3
> > by default. This is also what net-snmp does. I originally chose this
> > default because that's what snmpctl did and it allowed for easier
> > interoperability with snmpd(8).
>
> v3 by default makes sense to me.
>
> I'm not sure how much it buys to remove the default community in snmp(1),
> though, there doesn't seem a lot of benefit to removing it?
My reasoning being that setting having public the default in snmp(1)
might encourage users to set public in snmpd(8) as well, which is what
we tried to discourage.
And it's easy enough to do something like
alias snmp_get="snmp get -v2c -ccommunity"
in .profile for interactive use and in scripts you always want to be
explicit with such parameters.
>
> (net-snmp tools do have that, but they also have /etc/snmp/snmp.conf or
> .snmp/snmp.conf so there's less to type on the command line).
>
> > Now that snmpd(8) moved on, so should snmp(1).
> >
> > OK?
> >
> > martijn@
> >
> > Index: snmpc.c
> > ===
> > RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
> > retrieving revision 1.35
> > diff -u -p -r1.35 snmpc.c
> > --- snmpc.c 8 Aug 2021 13:41:26 - 1.35
> > +++ snmpc.c 11 Aug 2021 14:34:08 -
> > @@ -84,12 +84,12 @@ struct snmp_app snmp_apps[] = {
> > };
> > struct snmp_app *snmp_app = NULL;
> >
> > -char *community = "public";
> > +char *community = NULL;
> > struct snmp_v3 *v3;
> > char *mib = "mib_2";
> > int retries = 5;
> > int timeout = 1;
> > -enum snmp_version version = SNMP_V2C;
> > +enum snmp_version version = SNMP_V3;
> > int print_equals = 1;
> > int print_varbind_only = 0;
> > int print_summary = 0;
> > @@ -468,7 +468,10 @@ main(int argc, char *argv[])
> > argc -= optind;
> > argv += optind;
> >
> > - if (version == SNMP_V3) {
> > + if (version == SNMP_V1 || version == SNMP_V2C) {
> > + if (community == NULL || community[0] == '\0')
> > + errx(1, "No community name specified.");
> > + } else if (version == SNMP_V3) {
> > /* Setup USM */
> > if (user == NULL || user[0] == '\0')
> > errx(1, "No securityName specified");
> >
> >
Re: snmp(1): Fix unsafe defaults
On 2021/08/11 16:35, Martijn van Duren wrote:
> Following snmpd, remove the public default community and move to snmpv3
> by default. This is also what net-snmp does. I originally chose this
> default because that's what snmpctl did and it allowed for easier
> interoperability with snmpd(8).
v3 by default makes sense to me.
I'm not sure how much it buys to remove the default community in snmp(1),
though, there doesn't seem a lot of benefit to removing it?
(net-snmp tools do have that, but they also have /etc/snmp/snmp.conf or
.snmp/snmp.conf so there's less to type on the command line).
> Now that snmpd(8) moved on, so should snmp(1).
>
> OK?
>
> martijn@
>
> Index: snmpc.c
> ===
> RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
> retrieving revision 1.35
> diff -u -p -r1.35 snmpc.c
> --- snmpc.c 8 Aug 2021 13:41:26 - 1.35
> +++ snmpc.c 11 Aug 2021 14:34:08 -
> @@ -84,12 +84,12 @@ struct snmp_app snmp_apps[] = {
> };
> struct snmp_app *snmp_app = NULL;
>
> -char *community = "public";
> +char *community = NULL;
> struct snmp_v3 *v3;
> char *mib = "mib_2";
> int retries = 5;
> int timeout = 1;
> -enum snmp_version version = SNMP_V2C;
> +enum snmp_version version = SNMP_V3;
> int print_equals = 1;
> int print_varbind_only = 0;
> int print_summary = 0;
> @@ -468,7 +468,10 @@ main(int argc, char *argv[])
> argc -= optind;
> argv += optind;
>
> - if (version == SNMP_V3) {
> + if (version == SNMP_V1 || version == SNMP_V2C) {
> + if (community == NULL || community[0] == '\0')
> + errx(1, "No community name specified.");
> + } else if (version == SNMP_V3) {
> /* Setup USM */
> if (user == NULL || user[0] == '\0')
> errx(1, "No securityName specified");
>
>
snmp(1): Fix unsafe defaults
Following snmpd, remove the public default community and move to snmpv3
by default. This is also what net-snmp does. I originally chose this
default because that's what snmpctl did and it allowed for easier
interoperability with snmpd(8).
Now that snmpd(8) moved on, so should snmp(1).
OK?
martijn@
Index: snmpc.c
===
RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
retrieving revision 1.35
diff -u -p -r1.35 snmpc.c
--- snmpc.c 8 Aug 2021 13:41:26 - 1.35
+++ snmpc.c 11 Aug 2021 14:34:08 -
@@ -84,12 +84,12 @@ struct snmp_app snmp_apps[] = {
};
struct snmp_app *snmp_app = NULL;
-char *community = "public";
+char *community = NULL;
struct snmp_v3 *v3;
char *mib = "mib_2";
int retries = 5;
int timeout = 1;
-enum snmp_version version = SNMP_V2C;
+enum snmp_version version = SNMP_V3;
int print_equals = 1;
int print_varbind_only = 0;
int print_summary = 0;
@@ -468,7 +468,10 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
- if (version == SNMP_V3) {
+ if (version == SNMP_V1 || version == SNMP_V2C) {
+ if (community == NULL || community[0] == '\0')
+ errx(1, "No community name specified.");
+ } else if (version == SNMP_V3) {
/* Setup USM */
if (user == NULL || user[0] == '\0')
errx(1, "No securityName specified");
