snmp(1) better handling of key material

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

snmp(1) better handling of key material

Martijn van Duren-5
In copying over the net-snmp semantics for snmp-v3/USM I got a bit too
lenient in handling the keys. This diff tries to rectify that.

This diff does basically two things:
1) Try to push people to use passphrases from stdin.
2) Clean up keying material (including from the ps listing) as soon as
   possible,

$ ./snmp get -v3 -u test -l authPriv -a SHA -x AES 127.0.0.1 \
> sysServices.0 < /home/martijn/.snmp/localhost
sysServices.0 = INTEGER: 74

OK?

martijn@

Index: snmpc.c
===================================================================
RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
retrieving revision 1.17
diff -u -p -r1.17 snmpc.c
--- snmpc.c 26 Oct 2019 19:34:15 -0000 1.17
+++ snmpc.c 30 Oct 2019 05:40:53 -0000
@@ -31,6 +31,7 @@
 #include <errno.h>
 #include <netdb.h>
 #include <poll.h>
+#include <readpassphrase.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -122,9 +123,19 @@ main(int argc, char *argv[])
  char *strtolp;
  int ch;
  size_t i;
+ char passphrase[128];
+ int pwdflags = 0;
 
- if (pledge("stdio inet dns", NULL) == -1)
- err(1, "pledge");
+ if (isatty(STDIN_FILENO)) {
+ if (unveil("/dev/tty", "rw") == -1)
+ err(1, "unveil");
+ if (pledge("stdio inet dns rpath wpath tty", NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (pledge("stdio inet dns", NULL) == -1)
+ err(1, "pledge");
+ pwdflags = RPP_STDIN;
+ }
 
  if (argc <= 1)
  usage();
@@ -161,6 +172,7 @@ main(int argc, char *argv[])
  authkey = optarg;
  authkeylen = strlen(authkey);
  authkeylevel = USM_KEY_PASSWORD;
+ warnx("Use of -A is deprecated");
  break;
  case 'a':
  if (strcasecmp(optarg, "MD5") == 0)
@@ -211,7 +223,8 @@ main(int argc, char *argv[])
  errx(1, "-3K");
  }
  privkeylevel = USM_KEY_LOCALIZED;
- break;
+ warnx("Use of -K is deprecated");
+ break;
  case 'k':
  authkey = snmpc_hex2bin(optarg, &authkeylen);
  if (authkey == NULL) {
@@ -220,6 +233,7 @@ main(int argc, char *argv[])
  err(1, "-k");
  }
  authkeylevel = USM_KEY_LOCALIZED;
+ warnx("Use of -k is deprecated");
  break;
  case 'l':
  if (strcasecmp(optarg, "noAuthNoPriv") == 0)
@@ -395,6 +409,7 @@ main(int argc, char *argv[])
  privkey = optarg;
  privkeylen = strlen(privkey);
  privkeylevel = USM_KEY_PASSWORD;
+ warnx("Use of -X is deprecated");
  break;
  case 'x':
  if (strcasecmp(optarg, "DES") == 0)
@@ -434,20 +449,38 @@ main(int argc, char *argv[])
  if (seclevel & SNMP_MSGFLAG_AUTH) {
  if (md == NULL)
  md = EVP_md5();
- if (authkey == NULL)
- errx(1, "No authKey or authPassword specified");
+ if (authkey == NULL) {
+ if ((authkey = readpassphrase("\rAuthpass: ",
+    passphrase, sizeof(passphrase),
+    pwdflags)) == NULL)
+ err(1, "Couldn't read Authpass");
+ authkeylen = strlen(authkey);
+ authkeylevel = USM_KEY_PASSWORD;
+ }
  if (usm_setauth(sec, md, authkey, authkeylen,
-    authkeylevel) == -1)
+    authkeylevel) == -1) {
+ explicit_bzero(authkey, authkeylen);
  err(1, "Can't set authkey");
+ }
+ explicit_bzero(authkey, authkeylen);
  }
  if (seclevel & SNMP_MSGFLAG_PRIV) {
  if (cipher == NULL)
  cipher = EVP_des_cbc();
- if (privkey == NULL)
- errx(1, "No privKey or privPassword specified");
+ if (privkey == NULL) {
+ if ((privkey = readpassphrase("\rPrivpass: ",
+    passphrase, sizeof(passphrase),
+    pwdflags)) == NULL)
+ err(1, "Couldn't read passphrase");
+ privkeylen = strlen(privkey);
+ privkeylevel = USM_KEY_PASSWORD;
+ }
  if (usm_setpriv(sec, cipher, privkey, privkeylen,
-    privkeylevel) == -1)
+    privkeylevel) == -1) {
+ explicit_bzero(privkey, privkeylen);
  err(1, "Can't set authkey");
+ }
+ explicit_bzero(privkey, privkeylen);
  }
  if (secengineid != NULL) {
  if (usm_setengineid(sec, secengineid,
Index: usm.c
===================================================================
RCS file: /cvs/src/usr.bin/snmp/usm.c,v
retrieving revision 1.5
diff -u -p -r1.5 usm.c
--- usm.c 24 Oct 2019 12:39:26 -0000 1.5
+++ usm.c 30 Oct 2019 05:40:53 -0000
@@ -44,9 +44,11 @@ struct usm_sec {
  enum usm_key_level authlevel;
  const EVP_MD *digest;
  char *authkey;
+ size_t authkeylen;
  enum usm_key_level privlevel;
  const EVP_CIPHER *cipher;
  char *privkey;
+ size_t privkeylen;
  int bootsset;
  uint32_t boots;
  int timeset;
@@ -319,7 +321,7 @@ usm_finalparams(struct snmp_agent *agent
  if (usm->authlevel != USM_KEY_LOCALIZED)
  return -1;
 
- if (HMAC(usm->digest, usm->authkey, EVP_MD_size(usm->digest), buf,
+ if (HMAC(usm->digest, usm->authkey, usm->authkeylen, buf,
     buflen, digest, NULL) == NULL)
  return -1;
 
@@ -427,7 +429,7 @@ usm_parseparams(struct snmp_agent *agent
  }
  if ((agent->v3->level & SNMP_MSGFLAG_AUTH)) {
  bzero(packet + secparamsoffset + digestoffset, digestlen);
- if (HMAC(usm->digest, usm->authkey, EVP_MD_size(usm->digest), packet,
+ if (HMAC(usm->digest, usm->authkey, usm->authkeylen, packet,
     packetlen, exp_digest, NULL) == NULL)
  goto fail;
 
@@ -484,7 +486,9 @@ usm_free(void *data)
  struct usm_sec *usm = data;
 
  free(usm->user);
+ explicit_bzero(usm->authkey, usm->authkeylen);
  free(usm->authkey);
+ explicit_bzero(usm->privkey, usm->privkeylen);
  free(usm->privkey);
  free(usm->engineid);
  free(usm);
@@ -507,7 +511,7 @@ usm_setauth(struct snmp_sec *sec, const
  if ((usm->authkey = usm_passwd2mkey(digest, key)) == NULL)
  return -1;
  level = USM_KEY_MASTER;
- keylen = EVP_MD_size(digest);
+ usm->authkeylen = EVP_MD_size(digest);
  } else {
  if (keylen != (size_t)EVP_MD_size(digest)) {
  errno = EINVAL;
@@ -517,6 +521,7 @@ usm_setauth(struct snmp_sec *sec, const
  return -1;
  memcpy(lkey, key, keylen);
  usm->authkey = lkey;
+ usm->authkeylen = keylen;
  }
  usm->digest = digest;
  usm->authlevel = level;
@@ -544,7 +549,7 @@ usm_setpriv(struct snmp_sec *sec, const
  if ((usm->privkey = usm_passwd2mkey(usm->digest, key)) == NULL)
  return -1;
  level = USM_KEY_MASTER;
- keylen = EVP_MD_size(usm->digest);
+ usm->privkeylen = EVP_MD_size(usm->digest);
  } else {
  if (keylen != (size_t)EVP_MD_size(usm->digest)) {
  errno = EINVAL;
@@ -554,6 +559,7 @@ usm_setpriv(struct snmp_sec *sec, const
  return -1;
  memcpy(lkey, key, keylen);
  usm->privkey = lkey;
+ usm->privkeylen = keylen;
  }
  usm->cipher = cipher;
  usm->privlevel = level;
@@ -581,7 +587,9 @@ usm_setengineid(struct snmp_sec *sec, ch
  usm->authkey = mkey;
  return -1;
  }
+ explicit_bzero(mkey, usm->authkeylen);
  free(mkey);
+ usm->authkeylen = EVP_MD_size(usm->digest);
  usm->authlevel = USM_KEY_LOCALIZED;
  }
  if (usm->privlevel == USM_KEY_MASTER) {
@@ -591,7 +599,9 @@ usm_setengineid(struct snmp_sec *sec, ch
  usm->privkey = mkey;
  return -1;
  }
+ explicit_bzero(mkey, usm->privkeylen);
  free(mkey);
+ usm->privkeylen = EVP_MD_size(usm->digest);
  usm->privlevel = USM_KEY_LOCALIZED;
  }
 

Reply | Threaded
Open this post in threaded view
|

Re: snmp(1) better handling of key material

Martijn van Duren-5
On 10/30/19 6:45 AM, Martijn van Duren wrote:

> In copying over the net-snmp semantics for snmp-v3/USM I got a bit too
> lenient in handling the keys. This diff tries to rectify that.
>
> This diff does basically two things:
> 1) Try to push people to use passphrases from stdin.
> 2) Clean up keying material (including from the ps listing) as soon as
>    possible,
>
> $ ./snmp get -v3 -u test -l authPriv -a SHA -x AES 127.0.0.1 \
>> sysServices.0 < /home/martijn/.snmp/localhost
> sysServices.0 = INTEGER: 74
>
> OK?
>
> martijn@
>
Use malloc_conceal based on shower-realisation.

Index: snmpc.c
===================================================================
RCS file: /cvs/src/usr.bin/snmp/snmpc.c,v
retrieving revision 1.17
diff -u -p -r1.17 snmpc.c
--- snmpc.c 26 Oct 2019 19:34:15 -0000 1.17
+++ snmpc.c 30 Oct 2019 06:16:17 -0000
@@ -31,6 +31,7 @@
 #include <errno.h>
 #include <netdb.h>
 #include <poll.h>
+#include <readpassphrase.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -43,6 +44,7 @@
 #include "usm.h"
 
 #define GETOPT_COMMON "A:a:c:E:e:K:k:l:n:O:r:t:u:v:X:x:Z:"
+#define PWLEN 128
 
 int snmpc_get(int, char *[]);
 int snmpc_walk(int, char *[]);
@@ -122,9 +124,19 @@ main(int argc, char *argv[])
  char *strtolp;
  int ch;
  size_t i;
+ char *passphrase = NULL;
+ int pwdflags = 0;
 
- if (pledge("stdio inet dns", NULL) == -1)
- err(1, "pledge");
+ if (isatty(STDIN_FILENO)) {
+ if (unveil("/dev/tty", "rw") == -1)
+ err(1, "unveil");
+ if (pledge("stdio inet dns rpath wpath tty", NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (pledge("stdio inet dns", NULL) == -1)
+ err(1, "pledge");
+ pwdflags = RPP_STDIN;
+ }
 
  if (argc <= 1)
  usage();
@@ -161,6 +173,7 @@ main(int argc, char *argv[])
  authkey = optarg;
  authkeylen = strlen(authkey);
  authkeylevel = USM_KEY_PASSWORD;
+ warnx("Use of -A is deprecated");
  break;
  case 'a':
  if (strcasecmp(optarg, "MD5") == 0)
@@ -211,7 +224,8 @@ main(int argc, char *argv[])
  errx(1, "-3K");
  }
  privkeylevel = USM_KEY_LOCALIZED;
- break;
+ warnx("Use of -K is deprecated");
+ break;
  case 'k':
  authkey = snmpc_hex2bin(optarg, &authkeylen);
  if (authkey == NULL) {
@@ -220,6 +234,7 @@ main(int argc, char *argv[])
  err(1, "-k");
  }
  authkeylevel = USM_KEY_LOCALIZED;
+ warnx("Use of -k is deprecated");
  break;
  case 'l':
  if (strcasecmp(optarg, "noAuthNoPriv") == 0)
@@ -395,6 +410,7 @@ main(int argc, char *argv[])
  privkey = optarg;
  privkeylen = strlen(privkey);
  privkeylevel = USM_KEY_PASSWORD;
+ warnx("Use of -X is deprecated");
  break;
  case 'x':
  if (strcasecmp(optarg, "DES") == 0)
@@ -434,21 +450,46 @@ main(int argc, char *argv[])
  if (seclevel & SNMP_MSGFLAG_AUTH) {
  if (md == NULL)
  md = EVP_md5();
- if (authkey == NULL)
- errx(1, "No authKey or authPassword specified");
+ if (authkey == NULL) {
+ if ((passphrase =
+    malloc_conceal(PWLEN)) == NULL)
+ err(1 , NULL);
+ if ((authkey = readpassphrase("\rAuthpass: ",
+    passphrase, PWLEN, pwdflags)) == NULL)
+ err(1, "Couldn't read Authpass");
+ authkeylen = strlen(authkey);
+ authkeylevel = USM_KEY_PASSWORD;
+ }
  if (usm_setauth(sec, md, authkey, authkeylen,
-    authkeylevel) == -1)
+    authkeylevel) == -1) {
+ explicit_bzero(authkey, authkeylen);
  err(1, "Can't set authkey");
+ }
+ explicit_bzero(authkey, authkeylen);
  }
  if (seclevel & SNMP_MSGFLAG_PRIV) {
  if (cipher == NULL)
  cipher = EVP_des_cbc();
- if (privkey == NULL)
- errx(1, "No privKey or privPassword specified");
+ if (privkey == NULL) {
+ if (passphrase == NULL) {
+ if ((passphrase =
+    malloc_conceal(PWLEN)) == NULL)
+ err(1, NULL);
+ }
+ if ((privkey = readpassphrase("\rPrivpass: ",
+    passphrase, PWLEN, pwdflags)) == NULL)
+ err(1, "Couldn't read Privpass");
+ privkeylen = strlen(privkey);
+ privkeylevel = USM_KEY_PASSWORD;
+ }
  if (usm_setpriv(sec, cipher, privkey, privkeylen,
-    privkeylevel) == -1)
+    privkeylevel) == -1) {
+ explicit_bzero(privkey, privkeylen);
  err(1, "Can't set authkey");
+ }
+ explicit_bzero(privkey, privkeylen);
  }
+ free(passphrase);
  if (secengineid != NULL) {
  if (usm_setengineid(sec, secengineid,
     secengineidlen) == -1)
Index: usm.c
===================================================================
RCS file: /cvs/src/usr.bin/snmp/usm.c,v
retrieving revision 1.5
diff -u -p -r1.5 usm.c
--- usm.c 24 Oct 2019 12:39:26 -0000 1.5
+++ usm.c 30 Oct 2019 06:16:17 -0000
@@ -44,9 +44,11 @@ struct usm_sec {
  enum usm_key_level authlevel;
  const EVP_MD *digest;
  char *authkey;
+ size_t authkeylen;
  enum usm_key_level privlevel;
  const EVP_CIPHER *cipher;
  char *privkey;
+ size_t privkeylen;
  int bootsset;
  uint32_t boots;
  int timeset;
@@ -319,7 +321,7 @@ usm_finalparams(struct snmp_agent *agent
  if (usm->authlevel != USM_KEY_LOCALIZED)
  return -1;
 
- if (HMAC(usm->digest, usm->authkey, EVP_MD_size(usm->digest), buf,
+ if (HMAC(usm->digest, usm->authkey, usm->authkeylen, buf,
     buflen, digest, NULL) == NULL)
  return -1;
 
@@ -427,7 +429,7 @@ usm_parseparams(struct snmp_agent *agent
  }
  if ((agent->v3->level & SNMP_MSGFLAG_AUTH)) {
  bzero(packet + secparamsoffset + digestoffset, digestlen);
- if (HMAC(usm->digest, usm->authkey, EVP_MD_size(usm->digest), packet,
+ if (HMAC(usm->digest, usm->authkey, usm->authkeylen, packet,
     packetlen, exp_digest, NULL) == NULL)
  goto fail;
 
@@ -507,16 +509,17 @@ usm_setauth(struct snmp_sec *sec, const
  if ((usm->authkey = usm_passwd2mkey(digest, key)) == NULL)
  return -1;
  level = USM_KEY_MASTER;
- keylen = EVP_MD_size(digest);
+ usm->authkeylen = EVP_MD_size(digest);
  } else {
  if (keylen != (size_t)EVP_MD_size(digest)) {
  errno = EINVAL;
  return -1;
  }
- if ((lkey = malloc(keylen)) == NULL)
+ if ((lkey = malloc_conceal(keylen)) == NULL)
  return -1;
  memcpy(lkey, key, keylen);
  usm->authkey = lkey;
+ usm->authkeylen = keylen;
  }
  usm->digest = digest;
  usm->authlevel = level;
@@ -544,16 +547,17 @@ usm_setpriv(struct snmp_sec *sec, const
  if ((usm->privkey = usm_passwd2mkey(usm->digest, key)) == NULL)
  return -1;
  level = USM_KEY_MASTER;
- keylen = EVP_MD_size(usm->digest);
+ usm->privkeylen = EVP_MD_size(usm->digest);
  } else {
  if (keylen != (size_t)EVP_MD_size(usm->digest)) {
  errno = EINVAL;
  return -1;
  }
- if ((lkey = malloc(keylen)) == NULL)
+ if ((lkey = malloc_conceal(keylen)) == NULL)
  return -1;
  memcpy(lkey, key, keylen);
  usm->privkey = lkey;
+ usm->privkeylen = keylen;
  }
  usm->cipher = cipher;
  usm->privlevel = level;
@@ -582,6 +586,7 @@ usm_setengineid(struct snmp_sec *sec, ch
  return -1;
  }
  free(mkey);
+ usm->authkeylen = EVP_MD_size(usm->digest);
  usm->authlevel = USM_KEY_LOCALIZED;
  }
  if (usm->privlevel == USM_KEY_MASTER) {
@@ -592,6 +597,7 @@ usm_setengineid(struct snmp_sec *sec, ch
  return -1;
  }
  free(mkey);
+ usm->privkeylen = EVP_MD_size(usm->digest);
  usm->privlevel = USM_KEY_LOCALIZED;
  }
 
@@ -639,7 +645,7 @@ usm_passwd2mkey(const EVP_MD *md, const
  EVP_DigestFinal_ex(&ctx, keybuf, &dlen);
  EVP_MD_CTX_cleanup(&ctx);
 
- if ((key = malloc(dlen)) == NULL)
+ if ((key = malloc_conceal(dlen)) == NULL)
  return NULL;
  memcpy(key, keybuf, dlen);
  return key;
@@ -663,7 +669,7 @@ usm_mkey2lkey(struct usm_sec *usm, const
  EVP_DigestFinal_ex(&ctx, buf, &lklen);
  EVP_MD_CTX_cleanup(&ctx);
 
- if ((lkey = malloc(lklen)) == NULL)
+ if ((lkey = malloc_conceal(lklen)) == NULL)
  return NULL;
  memcpy(lkey, buf, lklen);
  return lkey;