[PATCH 0/7] cifs.upcall: fixes and improvements for SPNEGO upcalls

[Jeff Layton-2]

This set is a series of patches to fix several problems with
cifs.upcall. There are a few behavioral changes with this set:

1) nerf the "-c" option, which was just confusing for users. It now
tries to get a cifs/ principal first and then falls back to a host/
prefix if that fails. I think it's safe to assume that if a cifs/
principal exists then that's the one we'll want to use. If that's
incorrect, please let me know.

2) to get the hostname for the service principal, it now
reverse-resolves the IP address passed by the kernel. This makes the
code much less sensitive to the hostname used when mounting.

It also adds some cleanup to the code and some debug-level logging that
can be used for troubleshooting problems.

Review appreciated.

Jeff Layton (7):
  cifs.upcall: clean up logging and add debug messages
  cifs.upcall: formatting cleanup
  cifs.upcall: declare a structure for holding decoded args
  cifs.upcall: try getting a "cifs/" principal and fall back to "host/"
  cifs.upcall: clean up flag handling
  cifs.upcall: use ip address passed by kernel to get server's hostname
  cifs.upcall: fix IPv6 addrs sent to upcall to have colon delimiters

 client/cifs.upcall.c                  |  319 +++++++++++++++++++++------------
 docs-xml/manpages-3/cifs.upcall.8.xml |    4 +-
 2 files changed, 202 insertions(+), 121 deletions(-)

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

Change the log levels to be more appropriate to the messages being
logged. Error messages should be LOG_ERR and not LOG_WARNING, for
instance.

Add some LOG_DEBUG messages that we can use to diagnose problems with
krb5 upcalls. With these, someone can set up syslog to log daemon.debug
and should be able to get more info when things aren't working.

Signed-off-by: Jeff Layton <jlayton@...>
---
 client/cifs.upcall.c |   79 +++++++++++++++++++++++++++++--------------------
 1 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/client/cifs.upcall.c b/client/cifs.upcall.c
index 82b9f7b..da6b1b8 100644
--- a/client/cifs.upcall.c
+++ b/client/cifs.upcall.c
@@ -58,14 +58,20 @@ get_krb5_ccname(pid_t pid)
  buf[4095] = '\0';
  snprintf(buf, 4095, "/proc/%d/environ", pid);
  fd = open(buf, O_RDONLY);
- if (fd < 0)
+ if (fd < 0) {
+ syslog(LOG_DEBUG, "%s: unable to open %s: %d", __func__, buf,
+ errno);
  return NULL;
+ }
 
  /* FIXME: don't assume that we get it all in the first read? */
  len = read(fd, buf, 4096);
  close(fd);
- if (len < 0)
+ if (len < 0) {
+ syslog(LOG_DEBUG, "%s: unable to read from /proc/%d/environ: "
+  "%d", __func__, pid, errno);
  return NULL;
+ }
 
  left = len;
  p = buf;
@@ -83,6 +89,8 @@ get_krb5_ccname(pid_t pid)
  value = SMB_STRNDUP(p, left);
  break;
  }
+ syslog(LOG_DEBUG, "%s: KRB5CCNAME=%s", __func__,
+ value ? value : "(null)");
  return value;
 }
 
@@ -113,12 +121,20 @@ handle_krb5_mech(const char *oid, const char *principal, DATA_BLOB *secblob,
  int retval;
  DATA_BLOB tkt, tkt_wrapped;
 
+ syslog(LOG_DEBUG, "%s: getting service ticket for %s", __func__,
+  principal);
+
  /* get a kerberos ticket for the service and extract the session key */
  retval = cli_krb5_get_ticket(principal, 0, &tkt, sess_key, 0, ccname,
      NULL);
 
- if (retval)
+ if (retval) {
+ syslog(LOG_DEBUG, "%s: failed to obtain service ticket (%d)",
+  __func__, retval);
  return retval;
+ }
+
+ syslog(LOG_DEBUG, "%s: obtained service ticket", __func__);
 
  /* wrap that up in a nice GSS-API wrapping */
  tkt_wrapped = spnego_gen_krb5_wrap(tkt, TOK_ID_KRB_AP_REQ);
@@ -131,13 +147,13 @@ handle_krb5_mech(const char *oid, const char *principal, DATA_BLOB *secblob,
  return retval;
 }
 
-#define DKD_HAVE_HOSTNAME 1
-#define DKD_HAVE_VERSION 2
-#define DKD_HAVE_SEC 4
-#define DKD_HAVE_IPV4 8
-#define DKD_HAVE_IPV6 16
-#define DKD_HAVE_UID 32
-#define DKD_HAVE_PID 64
+#define DKD_HAVE_HOSTNAME 0x1
+#define DKD_HAVE_VERSION 0x2
+#define DKD_HAVE_SEC 0x4
+#define DKD_HAVE_IPV4 0x8
+#define DKD_HAVE_IPV6 0x10
+#define DKD_HAVE_UID 0x20
+#define DKD_HAVE_PID 0x40
 #define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC)
 
 static int
@@ -171,7 +187,7 @@ decode_key_description(const char *desc, int *ver, secType_t *sec,
  errno = 0;
  *pid = strtol(tkn + 4, NULL, 0);
  if (errno != 0) {
- syslog(LOG_WARNING, "Invalid pid format: %s",
+ syslog(LOG_ERR, "Invalid pid format: %s",
        strerror(errno));
  return 1;
  } else {
@@ -189,7 +205,7 @@ decode_key_description(const char *desc, int *ver, secType_t *sec,
  errno = 0;
  *uid = strtol(tkn + 4, NULL, 16);
  if (errno != 0) {
- syslog(LOG_WARNING, "Invalid uid format: %s",
+ syslog(LOG_ERR, "Invalid uid format: %s",
        strerror(errno));
  return 1;
  } else {
@@ -199,8 +215,7 @@ decode_key_description(const char *desc, int *ver, secType_t *sec,
  errno = 0;
  *ver = strtol(tkn + 4, NULL, 16);
  if (errno != 0) {
- syslog(LOG_WARNING,
-       "Invalid version format: %s",
+ syslog(LOG_ERR, "Invalid version format: %s",
        strerror(errno));
  return 1;
  } else {
@@ -226,7 +241,7 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
  for (c = 1; c <= 4; c++) {
  keyend = index(keyend+1, ';');
  if (!keyend) {
- syslog(LOG_WARNING, "invalid key description: %s",
+ syslog(LOG_ERR, "invalid key description: %s",
  key_descr);
  return 1;
  }
@@ -236,7 +251,7 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
  /* resolve name to ip */
  c = getaddrinfo(keyend, NULL, NULL, &addr);
  if (c) {
- syslog(LOG_WARNING, "unable to resolve hostname: %s [%s]",
+ syslog(LOG_ERR, "unable to resolve hostname: %s [%s]",
  keyend, gai_strerror(c));
  return 1;
  }
@@ -248,8 +263,7 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
  p = &(((struct sockaddr_in6 *)addr->ai_addr)->sin6_addr);
  }
  if (!inet_ntop(addr->ai_family, p, ip, sizeof(ip))) {
- syslog(LOG_WARNING, "%s: inet_ntop: %s",
- __FUNCTION__, strerror(errno));
+ syslog(LOG_ERR, "%s: inet_ntop: %s", __func__, strerror(errno));
  freeaddrinfo(addr);
  return 1;
  }
@@ -257,8 +271,8 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
  /* setup key */
  c = keyctl_instantiate(key, ip, strlen(ip)+1, 0);
  if (c == -1) {
- syslog(LOG_WARNING, "%s: keyctl_instantiate: %s",
- __FUNCTION__, strerror(errno));
+ syslog(LOG_ERR, "%s: keyctl_instantiate: %s", __func__,
+ strerror(errno));
  freeaddrinfo(addr);
  return 1;
  }
@@ -270,7 +284,7 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
 static void
 usage(void)
 {
- syslog(LOG_WARNING, "Usage: %s [-c] [-v] key_serial", prog);
+ syslog(LOG_INFO, "Usage: %s [-c] [-v] key_serial", prog);
  fprintf(stderr, "Usage: %s [-c] [-v] key_serial\n", prog);
 }
 
@@ -303,7 +317,7 @@ int main(const int argc, char *const argv[])
  goto out;
  }
  default:{
- syslog(LOG_WARNING, "unknown option: %c", c);
+ syslog(LOG_ERR, "unknown option: %c", c);
  goto out;
  }
  }
@@ -320,18 +334,20 @@ int main(const int argc, char *const argv[])
  key = strtol(argv[optind], NULL, 10);
  if (errno != 0) {
  key = 0;
- syslog(LOG_WARNING, "Invalid key format: %s", strerror(errno));
+ syslog(LOG_ERR, "Invalid key format: %s", strerror(errno));
  goto out;
  }
 
  rc = keyctl_describe_alloc(key, &buf);
  if (rc == -1) {
- syslog(LOG_WARNING, "keyctl_describe_alloc failed: %s",
+ syslog(LOG_ERR, "keyctl_describe_alloc failed: %s",
        strerror(errno));
  rc = 1;
  goto out;
  }
 
+ syslog(LOG_DEBUG, "key description: %s", buf);
+
  if ((strncmp(buf, "cifs.resolver", sizeof("cifs.resolver")-1) == 0) ||
     (strncmp(buf, "dns_resolver", sizeof("dns_resolver")-1) == 0)) {
  rc = cifs_resolver(key, buf);
@@ -341,8 +357,8 @@ int main(const int argc, char *const argv[])
  rc = decode_key_description(buf, &kernel_upcall_version, §ype,
     &hostname, &uid, &pid);
  if ((rc & DKD_MUSTHAVE_SET) != DKD_MUSTHAVE_SET) {
- syslog(LOG_WARNING,
-       "unable to get from description necessary params");
+ syslog(LOG_ERR, "unable to get necessary params from key "
+ "description (0x%x)", rc);
  rc = 1;
  SAFE_FREE(buf);
  goto out;
@@ -350,9 +366,8 @@ int main(const int argc, char *const argv[])
  SAFE_FREE(buf);
 
  if (kernel_upcall_version > CIFS_SPNEGO_UPCALL_VERSION) {
- syslog(LOG_WARNING,
-       "incompatible kernel upcall version: 0x%x",
-       kernel_upcall_version);
+ syslog(LOG_ERR, "incompatible kernel upcall version: 0x%x",
+ kernel_upcall_version);
  rc = 1;
  goto out;
  }
@@ -363,7 +378,7 @@ int main(const int argc, char *const argv[])
  if (rc & DKD_HAVE_UID) {
  rc = setuid(uid);
  if (rc == -1) {
- syslog(LOG_WARNING, "setuid: %s", strerror(errno));
+ syslog(LOG_ERR, "setuid: %s", strerror(errno));
  goto out;
  }
  }
@@ -400,7 +415,7 @@ int main(const int argc, char *const argv[])
  break;
  }
  default:{
- syslog(LOG_WARNING, "sectype: %d is not implemented",
+ syslog(LOG_ERR, "sectype: %d is not implemented",
        sectype);
  rc = 1;
  break;
@@ -430,7 +445,7 @@ int main(const int argc, char *const argv[])
  /* setup key */
  rc = keyctl_instantiate(key, keydata, datalen, 0);
  if (rc == -1) {
- syslog(LOG_WARNING, "keyctl_instantiate: %s", strerror(errno));
+ syslog(LOG_ERR, "keyctl_instantiate: %s", strerror(errno));
  goto out;
  }
 
--
1.6.0.6

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

Clean up some unneeded curly braces, and fix some indentation.

Signed-off-by: Jeff Layton <jlayton@...>
---
 client/cifs.upcall.c |   84 ++++++++++++++++++++++----------------------------
 1 files changed, 37 insertions(+), 47 deletions(-)

diff --git a/client/cifs.upcall.c b/client/cifs.upcall.c
index da6b1b8..926ec20 100644
--- a/client/cifs.upcall.c
+++ b/client/cifs.upcall.c
@@ -113,7 +113,7 @@ get_krb5_ccname(pid_t pid)
  * sess_key- pointer for SessionKey data to be stored
  *
  * ret: 0 - success, others - failure
-*/
+ */
 static int
 handle_krb5_mech(const char *oid, const char *principal, DATA_BLOB *secblob,
  DATA_BLOB *sess_key, const char *ccname)
@@ -169,11 +169,11 @@ decode_key_description(const char *desc, int *ver, secType_t *sec,
  if (strncmp(tkn, "host=", 5) == 0) {
  int len;
 
- if (pos == NULL) {
+ if (pos == NULL)
  len = strlen(tkn);
- } else {
+ else
  len = pos - tkn;
- }
+
  len -= 4;
  SAFE_FREE(*hostname);
  *hostname = SMB_XMALLOC_ARRAY(char, len);
@@ -257,11 +257,11 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
  }
 
  /* conver ip to string form */
- if (addr->ai_family == AF_INET) {
+ if (addr->ai_family == AF_INET)
  p = &(((struct sockaddr_in *)addr->ai_addr)->sin_addr);
- } else {
+ else
  p = &(((struct sockaddr_in6 *)addr->ai_addr)->sin6_addr);
- }
+
  if (!inet_ntop(addr->ai_family, p, ip, sizeof(ip))) {
  syslog(LOG_ERR, "%s: inet_ntop: %s", __func__, strerror(errno));
  freeaddrinfo(addr);
@@ -301,25 +301,22 @@ int main(const int argc, char *const argv[])
  pid_t pid = 0;
  int kernel_upcall_version = 0;
  int c, use_cifs_service_prefix = 0;
- char *buf, *ccname = NULL, *hostname = NULL;
+ char *buf, *princ, *ccname = NULL, *hostname = NULL;
  const char *oid;
 
  openlog(prog, 0, LOG_DAEMON);
 
  while ((c = getopt(argc, argv, "cv")) != -1) {
  switch (c) {
- case 'c':{
+ case 'c':
  use_cifs_service_prefix = 1;
  break;
- }
- case 'v':{
+ case 'v':
  printf("version: %s\n", CIFSSPNEGO_VERSION);
  goto out;
- }
- default:{
+ default:
  syslog(LOG_ERR, "unknown option: %c", c);
  goto out;
- }
  }
  }
 
@@ -386,45 +383,38 @@ int main(const int argc, char *const argv[])
  // do mech specific authorization
  switch (sectype) {
  case MS_KRB5:
- case KRB5:{
- char *princ;
- size_t len;
-
- /* for "cifs/" service name + terminating 0 */
- len = strlen(hostname) + 5 + 1;
- princ = SMB_XMALLOC_ARRAY(char, len);
- if (!princ) {
- rc = 1;
- break;
- }
- if (use_cifs_service_prefix) {
- strlcpy(princ, "cifs/", len);
- } else {
- strlcpy(princ, "host/", len);
- }
- strlcpy(princ + 5, hostname, len - 5);
-
- if (sectype == MS_KRB5)
- oid = OID_KERBEROS5_OLD;
- else
- oid = OID_KERBEROS5;
-
- rc = handle_krb5_mech(oid, princ, &secblob, &sess_key,
-      ccname);
- SAFE_FREE(princ);
- break;
- }
- default:{
- syslog(LOG_ERR, "sectype: %d is not implemented",
-       sectype);
+ case KRB5:
+ /* for "cifs/" service name + terminating 0 */
+ datalen = strlen(hostname) + 5 + 1;
+ princ = SMB_XMALLOC_ARRAY(char, datalen);
+ if (!princ) {
  rc = 1;
  break;
  }
+
+ if (use_cifs_service_prefix)
+ strlcpy(princ, "cifs/", datalen);
+ else
+ strlcpy(princ, "host/", datalen);
+
+ strlcpy(princ + 5, hostname, datalen - 5);
+
+ if (sectype == MS_KRB5)
+ oid = OID_KERBEROS5_OLD;
+ else
+ oid = OID_KERBEROS5;
+
+ rc = handle_krb5_mech(oid, princ, &secblob, &sess_key, ccname);
+ SAFE_FREE(princ);
+ break;
+ default:
+ syslog(LOG_ERR, "sectype: %d is not implemented", sectype);
+ rc = 1;
+ break;
  }
 
- if (rc) {
+ if (rc)
  goto out;
- }
 
  /* pack SecurityBLob and SessionKey into downcall packet */
  datalen =
--
1.6.0.6

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

The argument list for the decoder is becoming rather long. Declare an
args structure and use that for holding the args. This also simplifies
pointer handling a bit.

Signed-off-by: Jeff Layton <jlayton@...>
---
 client/cifs.upcall.c |   63 ++++++++++++++++++++++++++-----------------------
 1 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/client/cifs.upcall.c b/client/cifs.upcall.c
index 926ec20..0ddcc75 100644
--- a/client/cifs.upcall.c
+++ b/client/cifs.upcall.c
@@ -32,11 +32,11 @@ create dns_resolver * * /usr/local/sbin/cifs.upcall %k
 
 const char *CIFSSPNEGO_VERSION = "1.2";
 static const char *prog = "cifs.upcall";
-typedef enum _secType {
+typedef enum _sectype {
  NONE = 0,
  KRB5,
  MS_KRB5
-} secType_t;
+} sectype_t;
 
 /*
  * given a process ID, get the value of the KRB5CCNAME environment variable
@@ -156,9 +156,16 @@ handle_krb5_mech(const char *oid, const char *principal, DATA_BLOB *secblob,
 #define DKD_HAVE_PID 0x40
 #define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC)
 
+static struct decoded_args {
+ int ver;
+ char *hostname;
+ uid_t uid;
+ pid_t pid;
+ sectype_t sec;
+};
+
 static int
-decode_key_description(const char *desc, int *ver, secType_t *sec,
-   char **hostname, uid_t *uid, pid_t *pid)
+decode_key_description(const char *desc, struct decoded_args *arg)
 {
  int retval = 0;
  char *pos;
@@ -175,9 +182,9 @@ decode_key_description(const char *desc, int *ver, secType_t *sec,
  len = pos - tkn;
 
  len -= 4;
- SAFE_FREE(*hostname);
- *hostname = SMB_XMALLOC_ARRAY(char, len);
- strlcpy(*hostname, tkn + 5, len);
+ SAFE_FREE(arg->hostname);
+ arg->hostname = SMB_XMALLOC_ARRAY(char, len);
+ strlcpy(arg->hostname, tkn + 5, len);
  retval |= DKD_HAVE_HOSTNAME;
  } else if (strncmp(tkn, "ipv4=", 5) == 0) {
  /* BB: do we need it if we have hostname already? */
@@ -185,7 +192,7 @@ decode_key_description(const char *desc, int *ver, secType_t *sec,
  /* BB: do we need it if we have hostname already? */
  } else if (strncmp(tkn, "pid=", 4) == 0) {
  errno = 0;
- *pid = strtol(tkn + 4, NULL, 0);
+ arg->pid = strtol(tkn + 4, NULL, 0);
  if (errno != 0) {
  syslog(LOG_ERR, "Invalid pid format: %s",
        strerror(errno));
@@ -196,14 +203,14 @@ decode_key_description(const char *desc, int *ver, secType_t *sec,
  } else if (strncmp(tkn, "sec=", 4) == 0) {
  if (strncmp(tkn + 4, "krb5", 4) == 0) {
  retval |= DKD_HAVE_SEC;
- *sec = KRB5;
+ arg->sec = KRB5;
  } else if (strncmp(tkn + 4, "mskrb5", 6) == 0) {
  retval |= DKD_HAVE_SEC;
- *sec = MS_KRB5;
+ arg->sec = MS_KRB5;
  }
  } else if (strncmp(tkn, "uid=", 4) == 0) {
  errno = 0;
- *uid = strtol(tkn + 4, NULL, 16);
+ arg->uid = strtol(tkn + 4, NULL, 16);
  if (errno != 0) {
  syslog(LOG_ERR, "Invalid uid format: %s",
        strerror(errno));
@@ -213,7 +220,7 @@ decode_key_description(const char *desc, int *ver, secType_t *sec,
  }
  } else if (strncmp(tkn, "ver=", 4) == 0) { /* if version */
  errno = 0;
- *ver = strtol(tkn + 4, NULL, 16);
+ arg->ver = strtol(tkn + 4, NULL, 16);
  if (errno != 0) {
  syslog(LOG_ERR, "Invalid version format: %s",
        strerror(errno));
@@ -293,15 +300,12 @@ int main(const int argc, char *const argv[])
  struct cifs_spnego_msg *keydata = NULL;
  DATA_BLOB secblob = data_blob_null;
  DATA_BLOB sess_key = data_blob_null;
- secType_t sectype = NONE;
  key_serial_t key = 0;
  size_t datalen;
  long rc = 1;
- uid_t uid = 0;
- pid_t pid = 0;
- int kernel_upcall_version = 0;
  int c, use_cifs_service_prefix = 0;
- char *buf, *princ, *ccname = NULL, *hostname = NULL;
+ char *buf, *princ, *ccname = NULL;
+ struct decoded_args arg = { };
  const char *oid;
 
  openlog(prog, 0, LOG_DAEMON);
@@ -351,8 +355,7 @@ int main(const int argc, char *const argv[])
  goto out;
  }
 
- rc = decode_key_description(buf, &kernel_upcall_version, §ype,
-    &hostname, &uid, &pid);
+ rc = decode_key_description(buf, &arg);
  if ((rc & DKD_MUSTHAVE_SET) != DKD_MUSTHAVE_SET) {
  syslog(LOG_ERR, "unable to get necessary params from key "
  "description (0x%x)", rc);
@@ -362,18 +365,18 @@ int main(const int argc, char *const argv[])
  }
  SAFE_FREE(buf);
 
- if (kernel_upcall_version > CIFS_SPNEGO_UPCALL_VERSION) {
+ if (arg.ver > CIFS_SPNEGO_UPCALL_VERSION) {
  syslog(LOG_ERR, "incompatible kernel upcall version: 0x%x",
- kernel_upcall_version);
+ arg.ver);
  rc = 1;
  goto out;
  }
 
  if (rc & DKD_HAVE_PID)
- ccname = get_krb5_ccname(pid);
+ ccname = get_krb5_ccname(arg.pid);
 
  if (rc & DKD_HAVE_UID) {
- rc = setuid(uid);
+ rc = setuid(arg.uid);
  if (rc == -1) {
  syslog(LOG_ERR, "setuid: %s", strerror(errno));
  goto out;
@@ -381,11 +384,11 @@ int main(const int argc, char *const argv[])
  }
 
  // do mech specific authorization
- switch (sectype) {
+ switch (arg.sec) {
  case MS_KRB5:
  case KRB5:
  /* for "cifs/" service name + terminating 0 */
- datalen = strlen(hostname) + 5 + 1;
+ datalen = strlen(arg.hostname) + 5 + 1;
  princ = SMB_XMALLOC_ARRAY(char, datalen);
  if (!princ) {
  rc = 1;
@@ -397,9 +400,9 @@ int main(const int argc, char *const argv[])
  else
  strlcpy(princ, "host/", datalen);
 
- strlcpy(princ + 5, hostname, datalen - 5);
+ strlcpy(princ + 5, arg.hostname, datalen - 5);
 
- if (sectype == MS_KRB5)
+ if (arg.sec == MS_KRB5)
  oid = OID_KERBEROS5_OLD;
  else
  oid = OID_KERBEROS5;
@@ -408,7 +411,7 @@ int main(const int argc, char *const argv[])
  SAFE_FREE(princ);
  break;
  default:
- syslog(LOG_ERR, "sectype: %d is not implemented", sectype);
+ syslog(LOG_ERR, "sectype: %d is not implemented", arg.sec);
  rc = 1;
  break;
  }
@@ -424,7 +427,7 @@ int main(const int argc, char *const argv[])
  rc = 1;
  goto out;
  }
- keydata->version = kernel_upcall_version;
+ keydata->version = arg.ver;
  keydata->flags = 0;
  keydata->sesskey_len = sess_key.length;
  keydata->secblob_len = secblob.length;
@@ -453,7 +456,7 @@ out:
  data_blob_free(&secblob);
  data_blob_free(&sess_key);
  SAFE_FREE(ccname);
- SAFE_FREE(hostname);
+ SAFE_FREE(arg.hostname);
  SAFE_FREE(keydata);
  return rc;
 }
--
1.6.0.6

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

cifs.upcall takes a "-c" flag that tells the upcall to get a principal
in the form of "cifs/hostname.example.com@REALM" instead of
"host/hostname.example.com@REALM". This has turned out to be a source of
great confusion for users.

Instead of requiring this flag, have the upcall try to get a "cifs/"
principal first. If that fails, fall back to getting a "host/"
principal.

Signed-off-by: Jeff Layton <jlayton@...>
---
 client/cifs.upcall.c                  |   28 ++++++++++++++++------------
 docs-xml/manpages-3/cifs.upcall.8.xml |    4 ++--
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/client/cifs.upcall.c b/client/cifs.upcall.c
index 0ddcc75..e60fb50 100644
--- a/client/cifs.upcall.c
+++ b/client/cifs.upcall.c
@@ -30,7 +30,7 @@ create dns_resolver * * /usr/local/sbin/cifs.upcall %k
 
 #include "cifs_spnego.h"
 
-const char *CIFSSPNEGO_VERSION = "1.2";
+const char *CIFSSPNEGO_VERSION = "1.3";
 static const char *prog = "cifs.upcall";
 typedef enum _sectype {
  NONE = 0,
@@ -291,8 +291,8 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
 static void
 usage(void)
 {
- syslog(LOG_INFO, "Usage: %s [-c] [-v] key_serial", prog);
- fprintf(stderr, "Usage: %s [-c] [-v] key_serial\n", prog);
+ syslog(LOG_INFO, "Usage: %s [-v] key_serial", prog);
+ fprintf(stderr, "Usage: %s [-v] key_serial\n", prog);
 }
 
 int main(const int argc, char *const argv[])
@@ -303,7 +303,7 @@ int main(const int argc, char *const argv[])
  key_serial_t key = 0;
  size_t datalen;
  long rc = 1;
- int c, use_cifs_service_prefix = 0;
+ int c;
  char *buf, *princ, *ccname = NULL;
  struct decoded_args arg = { };
  const char *oid;
@@ -313,7 +313,7 @@ int main(const int argc, char *const argv[])
  while ((c = getopt(argc, argv, "cv")) != -1) {
  switch (c) {
  case 'c':
- use_cifs_service_prefix = 1;
+ /* legacy option -- skip it */
  break;
  case 'v':
  printf("version: %s\n", CIFSSPNEGO_VERSION);
@@ -395,19 +395,23 @@ int main(const int argc, char *const argv[])
  break;
  }
 
- if (use_cifs_service_prefix)
- strlcpy(princ, "cifs/", datalen);
- else
- strlcpy(princ, "host/", datalen);
-
- strlcpy(princ + 5, arg.hostname, datalen - 5);
-
  if (arg.sec == MS_KRB5)
  oid = OID_KERBEROS5_OLD;
  else
  oid = OID_KERBEROS5;
 
+ /*
+ * try getting a cifs/ principal first and then fall back to
+ * getting a host/ principal if that doesn't work.
+ */
+ strlcpy(princ, "cifs/", datalen);
+ strlcpy(princ + 5, arg.hostname, datalen - 5);
  rc = handle_krb5_mech(oid, princ, &secblob, &sess_key, ccname);
+ if (rc) {
+ memcpy(princ, "host/", 5);
+ rc = handle_krb5_mech(oid, princ, &secblob, &sess_key,
+ ccname);
+ }
  SAFE_FREE(princ);
  break;
  default:
diff --git a/docs-xml/manpages-3/cifs.upcall.8.xml b/docs-xml/manpages-3/cifs.upcall.8.xml
index 6e22bff..427bb44 100644
--- a/docs-xml/manpages-3/cifs.upcall.8.xml
+++ b/docs-xml/manpages-3/cifs.upcall.8.xml
@@ -48,7 +48,7 @@ to be run that way.</para>
  <variablelist>
  <varlistentry>
  <term>-c</term>
- <listitem><para>When handling a kerberos upcall, use a service principal that starts with "cifs/". The default is to use the "host/" service principal.
+ <listitem><para>This option is deprecated and is currently ignored.
  </para></listitem>
  </varlistentry>
 
@@ -86,7 +86,7 @@ to be run that way.</para>
 <programlisting>
 #OPERATION  TYPE           D C PROGRAM ARG1 ARG2...
 #=========  =============  = = ==========================================
-create    cifs.spnego    * * /usr/local/sbin/cifs.upcall -c %k
+create      cifs.spnego    * * /usr/local/sbin/cifs.upcall %k
 create      dns_resolver   * * /usr/local/sbin/cifs.upcall %k
 </programlisting>
 <para>
--
1.6.0.6

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

Add a new stack var to hold the flags returned by the decoder routine
so that we don't need to worry so much about preserving "rc".

Signed-off-by: Jeff Layton <jlayton@...>
---
 client/cifs.upcall.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/client/cifs.upcall.c b/client/cifs.upcall.c
index e60fb50..38be876 100644
--- a/client/cifs.upcall.c
+++ b/client/cifs.upcall.c
@@ -164,7 +164,7 @@ static struct decoded_args {
  sectype_t sec;
 };
 
-static int
+static unsigned int
 decode_key_description(const char *desc, struct decoded_args *arg)
 {
  int retval = 0;
@@ -302,6 +302,7 @@ int main(const int argc, char *const argv[])
  DATA_BLOB sess_key = data_blob_null;
  key_serial_t key = 0;
  size_t datalen;
+ unsigned int have;
  long rc = 1;
  int c;
  char *buf, *princ, *ccname = NULL;
@@ -355,15 +356,14 @@ int main(const int argc, char *const argv[])
  goto out;
  }
 
- rc = decode_key_description(buf, &arg);
- if ((rc & DKD_MUSTHAVE_SET) != DKD_MUSTHAVE_SET) {
+ have = decode_key_description(buf, &arg);
+ SAFE_FREE(buf);
+ if ((have & DKD_MUSTHAVE_SET) != DKD_MUSTHAVE_SET) {
  syslog(LOG_ERR, "unable to get necessary params from key "
- "description (0x%x)", rc);
+ "description (0x%x)", have);
  rc = 1;
- SAFE_FREE(buf);
  goto out;
  }
- SAFE_FREE(buf);
 
  if (arg.ver > CIFS_SPNEGO_UPCALL_VERSION) {
  syslog(LOG_ERR, "incompatible kernel upcall version: 0x%x",
@@ -372,10 +372,10 @@ int main(const int argc, char *const argv[])
  goto out;
  }
 
- if (rc & DKD_HAVE_PID)
+ if (have & DKD_HAVE_PID)
  ccname = get_krb5_ccname(arg.pid);
 
- if (rc & DKD_HAVE_UID) {
+ if (have & DKD_HAVE_UID) {
  rc = setuid(arg.uid);
  if (rc == -1) {
  syslog(LOG_ERR, "setuid: %s", strerror(errno));
--
1.6.0.6

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

Instead of using the hostname given by the upcall to get the server's
principal, take the IP address given in the upcall and reverse resolve
it to a hostname.

Signed-off-by: Jeff Layton <jlayton@...>
---
 client/cifs.upcall.c |   68 +++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/client/cifs.upcall.c b/client/cifs.upcall.c
index 38be876..1e58503 100644
--- a/client/cifs.upcall.c
+++ b/client/cifs.upcall.c
@@ -150,15 +150,15 @@ handle_krb5_mech(const char *oid, const char *principal, DATA_BLOB *secblob,
 #define DKD_HAVE_HOSTNAME 0x1
 #define DKD_HAVE_VERSION 0x2
 #define DKD_HAVE_SEC 0x4
-#define DKD_HAVE_IPV4 0x8
-#define DKD_HAVE_IPV6 0x10
-#define DKD_HAVE_UID 0x20
-#define DKD_HAVE_PID 0x40
-#define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC)
+#define DKD_HAVE_IP 0x8
+#define DKD_HAVE_UID 0x10
+#define DKD_HAVE_PID 0x20
+#define DKD_MUSTHAVE_SET (DKD_HAVE_IP|DKD_HAVE_VERSION|DKD_HAVE_SEC)
 
 static struct decoded_args {
  int ver;
  char *hostname;
+ char *ip;
  uid_t uid;
  pid_t pid;
  sectype_t sec;
@@ -167,6 +167,7 @@ static struct decoded_args {
 static unsigned int
 decode_key_description(const char *desc, struct decoded_args *arg)
 {
+ int len;
  int retval = 0;
  char *pos;
  const char *tkn = desc;
@@ -174,7 +175,6 @@ decode_key_description(const char *desc, struct decoded_args *arg)
  do {
  pos = index(tkn, ';');
  if (strncmp(tkn, "host=", 5) == 0) {
- int len;
 
  if (pos == NULL)
  len = strlen(tkn);
@@ -186,10 +186,18 @@ decode_key_description(const char *desc, struct decoded_args *arg)
  arg->hostname = SMB_XMALLOC_ARRAY(char, len);
  strlcpy(arg->hostname, tkn + 5, len);
  retval |= DKD_HAVE_HOSTNAME;
- } else if (strncmp(tkn, "ipv4=", 5) == 0) {
- /* BB: do we need it if we have hostname already? */
- } else if (strncmp(tkn, "ipv6=", 5) == 0) {
- /* BB: do we need it if we have hostname already? */
+ } else if (!strncmp(tkn, "ip4=", 4) ||
+   !strncmp(tkn, "ip6=", 4)) {
+ if (pos == NULL)
+ len = strlen(tkn);
+ else
+ len = pos - tkn;
+
+ len -= 3;
+ SAFE_FREE(arg->ip);
+ arg->ip = SMB_XMALLOC_ARRAY(char, len);
+ strlcpy(arg->ip, tkn + 4, len);
+ retval |= DKD_HAVE_IP;
  } else if (strncmp(tkn, "pid=", 4) == 0) {
  errno = 0;
  arg->pid = strtol(tkn + 4, NULL, 0);
@@ -288,6 +296,35 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
  return 0;
 }
 
+static int
+ip_to_fqdn(const char *ipaddr, char *host, size_t hostlen)
+{
+ int rc;
+ struct addrinfo hints = { .ai_flags = AI_NUMERICHOST };
+ struct addrinfo *res;
+
+ rc = getaddrinfo(ipaddr, NULL, &hints, &res);
+ if (rc) {
+ syslog(LOG_DEBUG, "%s: failed to resolve %s to ipaddr: %s",
+ __func__, ipaddr,
+ rc == EAI_SYSTEM ? strerror(errno) : gai_strerror(rc));
+ return rc;
+ }
+
+ rc = getnameinfo(res->ai_addr, res->ai_addrlen, host, hostlen,
+ NULL, 0, NI_NAMEREQD);
+ freeaddrinfo(res);
+ if (rc) {
+ syslog(LOG_DEBUG, "%s: failed to resolve %s to fqdn: %s",
+ __func__, ipaddr,
+ rc == EAI_SYSTEM ? strerror(errno) : gai_strerror(rc));
+ return rc;
+ }
+
+ syslog(LOG_DEBUG, "%s: resolved %s to %s", __func__, ipaddr, host);
+ return 0;
+}
+
 static void
 usage(void)
 {
@@ -306,6 +343,7 @@ int main(const int argc, char *const argv[])
  long rc = 1;
  int c;
  char *buf, *princ, *ccname = NULL;
+ char hostbuf[NI_MAXHOST];
  struct decoded_args arg = { };
  const char *oid;
 
@@ -383,12 +421,18 @@ int main(const int argc, char *const argv[])
  }
  }
 
+ if (have & DKD_HAVE_IP) {
+ rc = ip_to_fqdn(arg.ip, hostbuf, sizeof(hostbuf));
+ if (rc)
+ goto out;
+ }
+
  // do mech specific authorization
  switch (arg.sec) {
  case MS_KRB5:
  case KRB5:
  /* for "cifs/" service name + terminating 0 */
- datalen = strlen(arg.hostname) + 5 + 1;
+ datalen = strnlen(hostbuf, sizeof(hostbuf)) + 5 + 1;
  princ = SMB_XMALLOC_ARRAY(char, datalen);
  if (!princ) {
  rc = 1;
@@ -405,7 +449,7 @@ int main(const int argc, char *const argv[])
  * getting a host/ principal if that doesn't work.
  */
  strlcpy(princ, "cifs/", datalen);
- strlcpy(princ + 5, arg.hostname, datalen - 5);
+ strlcpy(princ + 5, hostbuf, datalen - 5);
  rc = handle_krb5_mech(oid, princ, &secblob, &sess_key, ccname);
  if (rc) {
  memcpy(princ, "host/", 5);
--
1.6.0.6

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

Current kernels don't send IPv6 addresses with the colon delimiters, add
a routine to add them when they're not present.

Signed-off-by: Jeff Layton <jlayton@...>
---
 client/cifs.upcall.c |   33 +++++++++++++++++++++++++++++----
 1 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/client/cifs.upcall.c b/client/cifs.upcall.c
index 1e58503..0cc72fc 100644
--- a/client/cifs.upcall.c
+++ b/client/cifs.upcall.c
@@ -296,18 +296,43 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
  return 0;
 }
 
+/*
+ * Older kernels sent IPv6 addresses without colons. Well, at least
+ * they're fixed-length strings. Convert these addresses to have colon
+ * delimiters to make getaddrinfo happy.
+ */
+static void
+convert_inet6_addr(const char *from, char *to)
+{
+ int i = 1;
+
+ while (*from) {
+ *to++ = *from++;
+ if (!(i++ % 4) && *from)
+ *to++ = ':';
+ }
+ *to = 0;
+}
+
 static int
-ip_to_fqdn(const char *ipaddr, char *host, size_t hostlen)
+ip_to_fqdn(const char *addrstr, char *host, size_t hostlen)
 {
  int rc;
  struct addrinfo hints = { .ai_flags = AI_NUMERICHOST };
  struct addrinfo *res;
+ const char *ipaddr = addrstr;
+ char converted[INET6_ADDRSTRLEN + 1];
+
+ if ((strlen(ipaddr) > INET_ADDRSTRLEN) && !strchr(ipaddr, ':')) {
+ convert_inet6_addr(ipaddr, converted);
+ ipaddr = converted;
+ }
 
  rc = getaddrinfo(ipaddr, NULL, &hints, &res);
  if (rc) {
- syslog(LOG_DEBUG, "%s: failed to resolve %s to ipaddr: %s",
- __func__, ipaddr,
- rc == EAI_SYSTEM ? strerror(errno) : gai_strerror(rc));
+ syslog(LOG_DEBUG, "%s: failed to resolve %s to "
+ "ipaddr: %s", __func__, ipaddr,
+ rc == EAI_SYSTEM ? strerror(errno) : gai_strerror(rc));
  return rc;
  }
 
--
1.6.0.6

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Q (Igor Mammedov)]

Jeff,
I'm sorry for a late response.

I see two possible issues with resolving ip to hostname and using it
as service principal:
1. In case of spoofed revers lookup or local dns cache or
compromised dns server or router between client and dns
server, we could be connected to a fake server without even
noticing it.
If we leave code as it is now, i.e. use hostname
supplied by user at mount time, we will get service ticket for
server we intended to connect to. In this case we at least will
get error message in logs  "Unexpected SMB signature".
As I can  remember, the current cifs code only checks signature
but doesn't tears session with wrong signature on packets
(transport.c:789) and doesn't do mutual authentication of the peer.

2.  It will break working now code In environments where there
is no dns or revers lookup doesn't match with a supplied
server name. And user will not even suspect why cifs doesn't
work anymore.

PS:
Theoretically we could have uninitialized memory read of
hostbuf if there wasn't ipv6 or ipv4 parameter in the upcall
request.

On Fri, Aug 7, 2009 at 11:43 PM, Jeff Layton<jlayton@...> wrote:

> Instead of using the hostname given by the upcall to get the server's
> principal, take the IP address given in the upcall and reverse resolve
> it to a hostname.
>
> Signed-off-by: Jeff Layton <jlayton@...>
> ---
>  client/cifs.upcall.c |   68 +++++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 56 insertions(+), 12 deletions(-)
>
> diff --git a/client/cifs.upcall.c b/client/cifs.upcall.c
> index 38be876..1e58503 100644
> --- a/client/cifs.upcall.c
> +++ b/client/cifs.upcall.c
> @@ -150,15 +150,15 @@ handle_krb5_mech(const char *oid, const char *principal, DATA_BLOB *secblob,
>  #define DKD_HAVE_HOSTNAME      0x1
>  #define DKD_HAVE_VERSION       0x2
>  #define DKD_HAVE_SEC           0x4
> -#define DKD_HAVE_IPV4          0x8
> -#define DKD_HAVE_IPV6          0x10
> -#define DKD_HAVE_UID           0x20
> -#define DKD_HAVE_PID           0x40
> -#define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC)
> +#define DKD_HAVE_IP            0x8
> +#define DKD_HAVE_UID           0x10
> +#define DKD_HAVE_PID           0x20
> +#define DKD_MUSTHAVE_SET (DKD_HAVE_IP|DKD_HAVE_VERSION|DKD_HAVE_SEC)
>
>  static struct decoded_args {
>        int             ver;
>        char            *hostname;
> +       char            *ip;
>        uid_t           uid;
>        pid_t           pid;
>        sectype_t       sec;
> @@ -167,6 +167,7 @@ static struct decoded_args {
>  static unsigned int
>  decode_key_description(const char *desc, struct decoded_args *arg)
>  {
> +       int len;
>        int retval = 0;
>        char *pos;
>        const char *tkn = desc;
> @@ -174,7 +175,6 @@ decode_key_description(const char *desc, struct decoded_args *arg)
>        do {
>                pos = index(tkn, ';');
>                if (strncmp(tkn, "host=", 5) == 0) {
> -                       int len;
>
>                        if (pos == NULL)
>                                len = strlen(tkn);
> @@ -186,10 +186,18 @@ decode_key_description(const char *desc, struct decoded_args *arg)
>                        arg->hostname = SMB_XMALLOC_ARRAY(char, len);
>                        strlcpy(arg->hostname, tkn + 5, len);
>                        retval |= DKD_HAVE_HOSTNAME;
> -               } else if (strncmp(tkn, "ipv4=", 5) == 0) {
> -                       /* BB: do we need it if we have hostname already? */
> -               } else if (strncmp(tkn, "ipv6=", 5) == 0) {
> -                       /* BB: do we need it if we have hostname already? */
> +               } else if (!strncmp(tkn, "ip4=", 4) ||
> +                          !strncmp(tkn, "ip6=", 4)) {
> +                       if (pos == NULL)
> +                               len = strlen(tkn);
> +                       else
> +                               len = pos - tkn;
> +
> +                       len -= 3;
> +                       SAFE_FREE(arg->ip);
> +                       arg->ip = SMB_XMALLOC_ARRAY(char, len);
> +                       strlcpy(arg->ip, tkn + 4, len);
> +                       retval |= DKD_HAVE_IP;
>                } else if (strncmp(tkn, "pid=", 4) == 0) {
>                        errno = 0;
>                        arg->pid = strtol(tkn + 4, NULL, 0);
> @@ -288,6 +296,35 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
>        return 0;
>  }
>
> +static int
> +ip_to_fqdn(const char *ipaddr, char *host, size_t hostlen)
> +{
> +       int rc;
> +       struct addrinfo hints = { .ai_flags = AI_NUMERICHOST };
> +       struct addrinfo *res;
> +
> +       rc = getaddrinfo(ipaddr, NULL, &hints, &res);
> +       if (rc) {
> +               syslog(LOG_DEBUG, "%s: failed to resolve %s to ipaddr: %s",
> +                       __func__, ipaddr,
> +                       rc == EAI_SYSTEM ? strerror(errno) : gai_strerror(rc));
> +               return rc;
> +       }
> +
> +       rc = getnameinfo(res->ai_addr, res->ai_addrlen, host, hostlen,
> +                        NULL, 0, NI_NAMEREQD);
> +       freeaddrinfo(res);
> +       if (rc) {
> +               syslog(LOG_DEBUG, "%s: failed to resolve %s to fqdn: %s",
> +                       __func__, ipaddr,
> +                       rc == EAI_SYSTEM ? strerror(errno) : gai_strerror(rc));
> +               return rc;
> +       }
> +
> +       syslog(LOG_DEBUG, "%s: resolved %s to %s", __func__, ipaddr, host);
> +       return 0;
> +}
> +
>  static void
>  usage(void)
>  {
> @@ -306,6 +343,7 @@ int main(const int argc, char *const argv[])
>        long rc = 1;
>        int c;
>        char *buf, *princ, *ccname = NULL;
> +       char hostbuf[NI_MAXHOST];
>        struct decoded_args arg = { };
>        const char *oid;
>
> @@ -383,12 +421,18 @@ int main(const int argc, char *const argv[])
>                }
>        }
>
> +       if (have & DKD_HAVE_IP) {
> +               rc = ip_to_fqdn(arg.ip, hostbuf, sizeof(hostbuf));
> +               if (rc)
> +                       goto out;
> +       }
> +
>        // do mech specific authorization
>        switch (arg.sec) {
>        case MS_KRB5:
>        case KRB5:
>                /* for "cifs/" service name + terminating 0 */
> -               datalen = strlen(arg.hostname) + 5 + 1;
> +               datalen = strnlen(hostbuf, sizeof(hostbuf)) + 5 + 1;
>                princ = SMB_XMALLOC_ARRAY(char, datalen);
>                if (!princ) {
>                        rc = 1;
> @@ -405,7 +449,7 @@ int main(const int argc, char *const argv[])
>                 * getting a host/ principal if that doesn't work.
>                 */
>                strlcpy(princ, "cifs/", datalen);
> -               strlcpy(princ + 5, arg.hostname, datalen - 5);
> +               strlcpy(princ + 5, hostbuf, datalen - 5);
>                rc = handle_krb5_mech(oid, princ, &secblob, &sess_key, ccname);
>                if (rc) {
>                        memcpy(princ, "host/", 5);
> --
> 1.6.0.6
>
> _______________________________________________
> linux-cifs-client mailing list
> linux-cifs-client@...
> https://lists.samba.org/mailman/listinfo/linux-cifs-client
>

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

On Sun, 16 Aug 2009 02:59:24 +0400
"Q (Igor Mammedov)" <qwerty0987654321@...> wrote:

> Jeff,
> I'm sorry for a late response.
>
> I see two possible issues with resolving ip to hostname and using it
> as service principal:
> 1. In case of spoofed revers lookup or local dns cache or
> compromised dns server or router between client and dns
> server, we could be connected to a fake server without even
> noticing it.

I have a hard time understanding this one. I don't think that we gain
any security by not trusing reverse hostname resolution.

Why? Because you still have to trust the forward resolve to give you
the correct address. You're just as likely to end up on a compromised
server that way.

> If we leave code as it is now, i.e. use hostname
> supplied by user at mount time, we will get service ticket for
> server we intended to connect to.

Only if you use the canonical hostname of the server when mounting.

> In this case we at least will
> get error message in logs  "Unexpected SMB signature".
> As I can  remember, the current cifs code only checks signature
> but doesn't tears session with wrong signature on packets
> (transport.c:789) and doesn't do mutual authentication of the peer.
>

That should probably be fixed sometime, but again, I don't see how
you're protected by not trusting reverse hostname resolution. With the
previous scheme you *had* to mount with a hostname anyway, so you
more or less have to trust your forward resolution anyway. I don't
see how trusting reverse resolution increases your security exposure.

There's a huge problem with the current scheme however. You need to
know the canonical name of the host in the service ticket and then
mount that. This is a huge source of confusion for users. Suppose I
have a host:

        server.example.com

...and I have a principal that I need to get in order to mount it:

        cifs/server.example.com@...

...now, suppose I try to mount up a share from that host, but use a
non-canonical hostname in the server section of the UNC:

        //server/share

...it doesn't work. The upcall will try to assemble a principal name
like:

        cifs/server@...

...similar problem if I try to mount a CNAME for the server. This is
also a problem when you want to mix krb5 and DFS. I don't believe the
current code sets the hostname.

The patch below fixes this (assuming you have your DNS and service
principals set up correctly).

> 2.  It will break working now code In environments where there
> is no dns or revers lookup doesn't match with a supplied
> server name. And user will not even suspect why cifs doesn't
> work anymore.
>

What about those people who are broken now? I consider the fact that
you can't mount using an alternate server hostname a bug.

I don't think it's unreasonable to expect that people with a functional
krb5 environment also have functioning hostname resolution. I think it's
more reasonable to expect that host resolution work than to insist that
people use the canonical hostname of the server when mounting. A user
may not even have any idea what that hostname actually *is*.

We should however probably note that properly functioning reverse host
resolution is required in the manpage however (the manpage is due for
some work anyway).

> PS:
> Theoretically we could have uninitialized memory read of
> hostbuf if there wasn't ipv6 or ipv4 parameter in the upcall
> request.
>

I don't think so. The patch below also changes:

#define DKD_MUSTHAVE_SET (DKD_HAVE_IP|DKD_HAVE_VERSION|DKD_HAVE_SEC)

...or am I missing something?

> On Fri, Aug 7, 2009 at 11:43 PM, Jeff Layton<jlayton@...> wrote:
> > Instead of using the hostname given by the upcall to get the server's
> > principal, take the IP address given in the upcall and reverse resolve
> > it to a hostname.
> >
> > Signed-off-by: Jeff Layton <jlayton@...>
> > ---
> >  client/cifs.upcall.c |   68 +++++++++++++++++++++++++++++++++++++++++---------
> >  1 files changed, 56 insertions(+), 12 deletions(-)
> >
> > diff --git a/client/cifs.upcall.c b/client/cifs.upcall.c
> > index 38be876..1e58503 100644
> > --- a/client/cifs.upcall.c
> > +++ b/client/cifs.upcall.c
> > @@ -150,15 +150,15 @@ handle_krb5_mech(const char *oid, const char *principal, DATA_BLOB *secblob,
> >  #define DKD_HAVE_HOSTNAME      0x1
> >  #define DKD_HAVE_VERSION       0x2
> >  #define DKD_HAVE_SEC           0x4
> > -#define DKD_HAVE_IPV4          0x8
> > -#define DKD_HAVE_IPV6          0x10
> > -#define DKD_HAVE_UID           0x20
> > -#define DKD_HAVE_PID           0x40
> > -#define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC)
> > +#define DKD_HAVE_IP            0x8
> > +#define DKD_HAVE_UID           0x10
> > +#define DKD_HAVE_PID           0x20
> > +#define DKD_MUSTHAVE_SET (DKD_HAVE_IP|DKD_HAVE_VERSION|DKD_HAVE_SEC)
> >
> >  static struct decoded_args {
> >        int             ver;
> >        char            *hostname;
> > +       char            *ip;
> >        uid_t           uid;
> >        pid_t           pid;
> >        sectype_t       sec;
> > @@ -167,6 +167,7 @@ static struct decoded_args {
> >  static unsigned int
> >  decode_key_description(const char *desc, struct decoded_args *arg)
> >  {
> > +       int len;
> >        int retval = 0;
> >        char *pos;
> >        const char *tkn = desc;
> > @@ -174,7 +175,6 @@ decode_key_description(const char *desc, struct decoded_args *arg)
> >        do {
> >                pos = index(tkn, ';');
> >                if (strncmp(tkn, "host=", 5) == 0) {
> > -                       int len;
> >
> >                        if (pos == NULL)
> >                                len = strlen(tkn);
> > @@ -186,10 +186,18 @@ decode_key_description(const char *desc, struct decoded_args *arg)
> >                        arg->hostname = SMB_XMALLOC_ARRAY(char, len);
> >                        strlcpy(arg->hostname, tkn + 5, len);
> >                        retval |= DKD_HAVE_HOSTNAME;
> > -               } else if (strncmp(tkn, "ipv4=", 5) == 0) {
> > -                       /* BB: do we need it if we have hostname already? */
> > -               } else if (strncmp(tkn, "ipv6=", 5) == 0) {
> > -                       /* BB: do we need it if we have hostname already? */
> > +               } else if (!strncmp(tkn, "ip4=", 4) ||
> > +                          !strncmp(tkn, "ip6=", 4)) {
> > +                       if (pos == NULL)
> > +                               len = strlen(tkn);
> > +                       else
> > +                               len = pos - tkn;
> > +
> > +                       len -= 3;
> > +                       SAFE_FREE(arg->ip);
> > +                       arg->ip = SMB_XMALLOC_ARRAY(char, len);
> > +                       strlcpy(arg->ip, tkn + 4, len);
> > +                       retval |= DKD_HAVE_IP;
> >                } else if (strncmp(tkn, "pid=", 4) == 0) {
> >                        errno = 0;
> >                        arg->pid = strtol(tkn + 4, NULL, 0);
> > @@ -288,6 +296,35 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
> >        return 0;
> >  }
> >
> > +static int
> > +ip_to_fqdn(const char *ipaddr, char *host, size_t hostlen)
> > +{
> > +       int rc;
> > +       struct addrinfo hints = { .ai_flags = AI_NUMERICHOST };
> > +       struct addrinfo *res;
> > +
> > +       rc = getaddrinfo(ipaddr, NULL, &hints, &res);
> > +       if (rc) {
> > +               syslog(LOG_DEBUG, "%s: failed to resolve %s to ipaddr: %s",
> > +                       __func__, ipaddr,
> > +                       rc == EAI_SYSTEM ? strerror(errno) : gai_strerror(rc));
> > +               return rc;
> > +       }
> > +
> > +       rc = getnameinfo(res->ai_addr, res->ai_addrlen, host, hostlen,
> > +                        NULL, 0, NI_NAMEREQD);
> > +       freeaddrinfo(res);
> > +       if (rc) {
> > +               syslog(LOG_DEBUG, "%s: failed to resolve %s to fqdn: %s",
> > +                       __func__, ipaddr,
> > +                       rc == EAI_SYSTEM ? strerror(errno) : gai_strerror(rc));
> > +               return rc;
> > +       }
> > +
> > +       syslog(LOG_DEBUG, "%s: resolved %s to %s", __func__, ipaddr, host);
> > +       return 0;
> > +}
> > +
> >  static void
> >  usage(void)
> >  {
> > @@ -306,6 +343,7 @@ int main(const int argc, char *const argv[])
> >        long rc = 1;
> >        int c;
> >        char *buf, *princ, *ccname = NULL;
> > +       char hostbuf[NI_MAXHOST];
> >        struct decoded_args arg = { };
> >        const char *oid;
> >
> > @@ -383,12 +421,18 @@ int main(const int argc, char *const argv[])
> >                }
> >        }
> >
> > +       if (have & DKD_HAVE_IP) {
> > +               rc = ip_to_fqdn(arg.ip, hostbuf, sizeof(hostbuf));
> > +               if (rc)
> > +                       goto out;
> > +       }
> > +
> >        // do mech specific authorization
> >        switch (arg.sec) {
> >        case MS_KRB5:
> >        case KRB5:
> >                /* for "cifs/" service name + terminating 0 */
> > -               datalen = strlen(arg.hostname) + 5 + 1;
> > +               datalen = strnlen(hostbuf, sizeof(hostbuf)) + 5 + 1;
> >                princ = SMB_XMALLOC_ARRAY(char, datalen);
> >                if (!princ) {
> >                        rc = 1;
> > @@ -405,7 +449,7 @@ int main(const int argc, char *const argv[])
> >                 * getting a host/ principal if that doesn't work.
> >                 */
> >                strlcpy(princ, "cifs/", datalen);
> > -               strlcpy(princ + 5, arg.hostname, datalen - 5);
> > +               strlcpy(princ + 5, hostbuf, datalen - 5);
> >                rc = handle_krb5_mech(oid, princ, &secblob, &sess_key, ccname);
> >                if (rc) {
> >                        memcpy(princ, "host/", 5);
> > --
> > 1.6.0.6
> >
> > _______________________________________________
> > linux-cifs-client mailing list
> > linux-cifs-client@...
> > https://lists.samba.org/mailman/listinfo/linux-cifs-client
> >

--
Jeff Layton <jlayton@...>
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Q (Igor Mammedov)]

On Sun, Aug 16, 2009 at 4:23 AM, Jeff Layton<jlayton@...> wrote:

> On Sun, 16 Aug 2009 02:59:24 +0400
> "Q (Igor Mammedov)" <qwerty0987654321@...> wrote:
>
>> Jeff,
>> I'm sorry for a late response.
>>
>> I see two possible issues with resolving ip to hostname and using it
>> as service principal:
>> 1. In case of spoofed revers lookup or local dns cache or
>> compromised dns server or router between client and dns
>> server, we could be connected to a fake server without even
>> noticing it.
>
> I have a hard time understanding this one. I don't think that we gain
> any security by not trusing reverse hostname resolution.
>
> Why? Because you still have to trust the forward resolve to give you
> the correct address. You're just as likely to end up on a compromised
> server that way.

No, we are not trusting anyone, may be except KDC a little ;)

>
>> If we leave code as it is now, i.e. use hostname
>> supplied by user at mount time, we will get service ticket for
>> server we intended to connect to.
>
> Only if you use the canonical hostname of the server when mounting.
>
>> In this case we at least will
>> get error message in logs  "Unexpected SMB signature".
>> As I can  remember, the current cifs code only checks signature
>> but doesn't tears session with wrong signature on packets
>> (transport.c:789) and doesn't do mutual authentication of the peer.
>>
>
> That should probably be fixed sometime, but again, I don't see how
> you're protected by not trusting reverse hostname resolution. With the
> previous scheme you *had* to mount with a hostname anyway, so you
> more or less have to trust your forward resolution anyway. I don't
> see how trusting reverse resolution increases your security exposure.

I'll try explain more clearly, what I've meant.

Providing that we could have compromised dns resolving, we could not
trust forward nor reverse resolution, but the goal of attacker is in
redirecting us his server.
Case 1: using host name  A.B.C supplied at mount time
   1.1 we have an IP of a  fake server because forward resolution was spoofed
   1.2 we get a valid session key from KDC for A.B.C
   Result: we can detect that packets a coming from a fake server, because
  of an incorrect signature.

Case 2: using host name F.A.K.E from revers resolution
   2.1 the same attack vector 1.1,  i.e. connect to a fake server's
IP, and doing
         revers resolution we are getting host name of a fake server.
  2.2 we get a valid session key from KDC for F.A.K.E because of fake server
        is a member of our domain.
  Result: attacker forced us to use his server, nobody will even notice it.

I prefer using 'case 1' for my files. And if cifs is fixed to drop a session
to a fake server, I will like it even more.

Case 2 are safe only in a hardened  network environment or with a secure
dns setup.

> There's a huge problem with the current scheme however. You need to
> know the canonical name of the host in the service ticket and then
> mount that. This is a huge source of confusion for users. Suppose I
> have a host:
>
>        server.example.com
>
> ...and I have a principal that I need to get in order to mount it:
>
>        cifs/server.example.com@...
>
> ...now, suppose I try to mount up a share from that host, but use a
> non-canonical hostname in the server section of the UNC:
>
>        //server/share
>
> ...it doesn't work. The upcall will try to assemble a principal name
> like:
>
>        cifs/server@...

I haven't noticed this case because in AD domain, a machine
account is registered with a fqdn and a hostname principals.
(At least it is so in my company)
So I were able to connect to //server/share

> ...similar problem if I try to mount a CNAME for the server. This is
> also a problem when you want to mix krb5 and DFS. I don't believe the
> current code sets the hostname.

The same thing, I someone wishes to provide access to a server via
CNAMEs, he/she should create corresponding principals on KDC.
This way it will not reduce security.

>
> The patch below fixes this (assuming you have your DNS and service
> principals set up correctly).
>
>> 2.  It will break working now code In environments where there
>> is no dns or revers lookup doesn't match with a supplied
>> server name. And user will not even suspect why cifs doesn't
>> work anymore.
>>
>
> What about those people who are broken now? I consider the fact that
> you can't mount using an alternate server hostname a bug.
>
> I don't think it's unreasonable to expect that people with a functional
> krb5 environment also have functioning hostname resolution. I think it's
> more reasonable to expect that host resolution work than to insist that
> people use the canonical hostname of the server when mounting. A user
> may not even have any idea what that hostname actually *is*.
>
> We should however probably note that properly functioning reverse host
> resolution is required in the manpage however (the manpage is due for
> some work anyway).

And maybe force those who hasn't dns at all, for whatever reason, to use it.

>
>> PS:
>> Theoretically we could have uninitialized memory read of
>> hostbuf if there wasn't ipv6 or ipv4 parameter in the upcall
>> request.
>>
>
> I don't think so. The patch below also changes:
>
> #define DKD_MUSTHAVE_SET (DKD_HAVE_IP|DKD_HAVE_VERSION|DKD_HAVE_SEC)
>
> ...or am I missing something?

It just caught my eyes.
Anyway I would initialized it, just in case.

>
>> On Fri, Aug 7, 2009 at 11:43 PM, Jeff Layton<jlayton@...> wrote:
>> > Instead of using the hostname given by the upcall to get the server's
>> > principal, take the IP address given in the upcall and reverse resolve
>> > it to a hostname.
>> >
>> > Signed-off-by: Jeff Layton <jlayton@...>
>> > ---
>> >  client/cifs.upcall.c |   68 +++++++++++++++++++++++++++++++++++++++++---------
>> >  1 files changed, 56 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/client/cifs.upcall.c b/client/cifs.upcall.c
>> > index 38be876..1e58503 100644
>> > --- a/client/cifs.upcall.c
>> > +++ b/client/cifs.upcall.c
>> > @@ -150,15 +150,15 @@ handle_krb5_mech(const char *oid, const char *principal, DATA_BLOB *secblob,
>> >  #define DKD_HAVE_HOSTNAME      0x1
>> >  #define DKD_HAVE_VERSION       0x2
>> >  #define DKD_HAVE_SEC           0x4
>> > -#define DKD_HAVE_IPV4          0x8
>> > -#define DKD_HAVE_IPV6          0x10
>> > -#define DKD_HAVE_UID           0x20
>> > -#define DKD_HAVE_PID           0x40
>> > -#define DKD_MUSTHAVE_SET (DKD_HAVE_HOSTNAME|DKD_HAVE_VERSION|DKD_HAVE_SEC)
>> > +#define DKD_HAVE_IP            0x8
>> > +#define DKD_HAVE_UID           0x10
>> > +#define DKD_HAVE_PID           0x20
>> > +#define DKD_MUSTHAVE_SET (DKD_HAVE_IP|DKD_HAVE_VERSION|DKD_HAVE_SEC)
>> >
>> >  static struct decoded_args {
>> >        int             ver;
>> >        char            *hostname;
>> > +       char            *ip;
>> >        uid_t           uid;
>> >        pid_t           pid;
>> >        sectype_t       sec;
>> > @@ -167,6 +167,7 @@ static struct decoded_args {
>> >  static unsigned int
>> >  decode_key_description(const char *desc, struct decoded_args *arg)
>> >  {
>> > +       int len;
>> >        int retval = 0;
>> >        char *pos;
>> >        const char *tkn = desc;
>> > @@ -174,7 +175,6 @@ decode_key_description(const char *desc, struct decoded_args *arg)
>> >        do {
>> >                pos = index(tkn, ';');
>> >                if (strncmp(tkn, "host=", 5) == 0) {
>> > -                       int len;
>> >
>> >                        if (pos == NULL)
>> >                                len = strlen(tkn);
>> > @@ -186,10 +186,18 @@ decode_key_description(const char *desc, struct decoded_args *arg)
>> >                        arg->hostname = SMB_XMALLOC_ARRAY(char, len);
>> >                        strlcpy(arg->hostname, tkn + 5, len);
>> >                        retval |= DKD_HAVE_HOSTNAME;
>> > -               } else if (strncmp(tkn, "ipv4=", 5) == 0) {
>> > -                       /* BB: do we need it if we have hostname already? */
>> > -               } else if (strncmp(tkn, "ipv6=", 5) == 0) {
>> > -                       /* BB: do we need it if we have hostname already? */
>> > +               } else if (!strncmp(tkn, "ip4=", 4) ||
>> > +                          !strncmp(tkn, "ip6=", 4)) {
>> > +                       if (pos == NULL)
>> > +                               len = strlen(tkn);
>> > +                       else
>> > +                               len = pos - tkn;
>> > +
>> > +                       len -= 3;
>> > +                       SAFE_FREE(arg->ip);
>> > +                       arg->ip = SMB_XMALLOC_ARRAY(char, len);
>> > +                       strlcpy(arg->ip, tkn + 4, len);
>> > +                       retval |= DKD_HAVE_IP;
>> >                } else if (strncmp(tkn, "pid=", 4) == 0) {
>> >                        errno = 0;
>> >                        arg->pid = strtol(tkn + 4, NULL, 0);
>> > @@ -288,6 +296,35 @@ cifs_resolver(const key_serial_t key, const char *key_descr)
>> >        return 0;
>> >  }
>> >
>> > +static int
>> > +ip_to_fqdn(const char *ipaddr, char *host, size_t hostlen)
>> > +{
>> > +       int rc;
>> > +       struct addrinfo hints = { .ai_flags = AI_NUMERICHOST };
>> > +       struct addrinfo *res;
>> > +
>> > +       rc = getaddrinfo(ipaddr, NULL, &hints, &res);
>> > +       if (rc) {
>> > +               syslog(LOG_DEBUG, "%s: failed to resolve %s to ipaddr: %s",
>> > +                       __func__, ipaddr,
>> > +                       rc == EAI_SYSTEM ? strerror(errno) : gai_strerror(rc));
>> > +               return rc;
>> > +       }
>> > +
>> > +       rc = getnameinfo(res->ai_addr, res->ai_addrlen, host, hostlen,
>> > +                        NULL, 0, NI_NAMEREQD);
>> > +       freeaddrinfo(res);
>> > +       if (rc) {
>> > +               syslog(LOG_DEBUG, "%s: failed to resolve %s to fqdn: %s",
>> > +                       __func__, ipaddr,
>> > +                       rc == EAI_SYSTEM ? strerror(errno) : gai_strerror(rc));
>> > +               return rc;
>> > +       }
>> > +
>> > +       syslog(LOG_DEBUG, "%s: resolved %s to %s", __func__, ipaddr, host);
>> > +       return 0;
>> > +}
>> > +
>> >  static void
>> >  usage(void)
>> >  {
>> > @@ -306,6 +343,7 @@ int main(const int argc, char *const argv[])
>> >        long rc = 1;
>> >        int c;
>> >        char *buf, *princ, *ccname = NULL;
>> > +       char hostbuf[NI_MAXHOST];
>> >        struct decoded_args arg = { };
>> >        const char *oid;
>> >
>> > @@ -383,12 +421,18 @@ int main(const int argc, char *const argv[])
>> >                }
>> >        }
>> >
>> > +       if (have & DKD_HAVE_IP) {
>> > +               rc = ip_to_fqdn(arg.ip, hostbuf, sizeof(hostbuf));
>> > +               if (rc)
>> > +                       goto out;
>> > +       }
>> > +
>> >        // do mech specific authorization
>> >        switch (arg.sec) {
>> >        case MS_KRB5:
>> >        case KRB5:
>> >                /* for "cifs/" service name + terminating 0 */
>> > -               datalen = strlen(arg.hostname) + 5 + 1;
>> > +               datalen = strnlen(hostbuf, sizeof(hostbuf)) + 5 + 1;
>> >                princ = SMB_XMALLOC_ARRAY(char, datalen);
>> >                if (!princ) {
>> >                        rc = 1;
>> > @@ -405,7 +449,7 @@ int main(const int argc, char *const argv[])
>> >                 * getting a host/ principal if that doesn't work.
>> >                 */
>> >                strlcpy(princ, "cifs/", datalen);
>> > -               strlcpy(princ + 5, arg.hostname, datalen - 5);
>> > +               strlcpy(princ + 5, hostbuf, datalen - 5);
>> >                rc = handle_krb5_mech(oid, princ, &secblob, &sess_key, ccname);
>> >                if (rc) {
>> >                        memcpy(princ, "host/", 5);
>> > --
>> > 1.6.0.6
>> >
>> > _______________________________________________
>> > linux-cifs-client mailing list
>> > linux-cifs-client@...
>> > https://lists.samba.org/mailman/listinfo/linux-cifs-client
>> >
>
>
> --
> Jeff Layton <jlayton@...>
>

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

On Sun, 16 Aug 2009 23:52:46 +0400
"Q (Igor Mammedov)" <qwerty0987654321@...> wrote:

>
> I'll try explain more clearly, what I've meant.
>
> Providing that we could have compromised dns resolving, we could not
> trust forward nor reverse resolution, but the goal of attacker is in
> redirecting us his server.
> Case 1: using host name  A.B.C supplied at mount time
>    1.1 we have an IP of a  fake server because forward resolution was spoofed
>    1.2 we get a valid session key from KDC for A.B.C
>    Result: we can detect that packets a coming from a fake server, because
>   of an incorrect signature.
>
> Case 2: using host name F.A.K.E from revers resolution
>    2.1 the same attack vector 1.1,  i.e. connect to a fake server's
> IP, and doing
>          revers resolution we are getting host name of a fake server.
>   2.2 we get a valid session key from KDC for F.A.K.E because of fake server
>         is a member of our domain.
>   Result: attacker forced us to use his server, nobody will even notice it.
>
> I prefer using 'case 1' for my files. And if cifs is fixed to drop a session
> to a fake server, I will like it even more.
>
> Case 2 are safe only in a hardened  network environment or with a secure
> dns setup.
>

Ok, that makes sense. The attack vector is a little contrived, but I'm
ok with preventing it. Still though, the fact that you need prior
knowledge of what the host portion of the service principal should be
is terribly problematic.

Perhaps we should consider a "--trust-dns" flag or something that
allows people to enable using reverse resolves?

Any thoughts on other ways to prevent the problem where the hostname
needed isn't known?

> > There's a huge problem with the current scheme however. You need to
> > know the canonical name of the host in the service ticket and then
> > mount that. This is a huge source of confusion for users. Suppose I
> > have a host:
> >
> >        server.example.com
> >
> > ...and I have a principal that I need to get in order to mount it:
> >
> >        cifs/server.example.com@...
> >
> > ...now, suppose I try to mount up a share from that host, but use a
> > non-canonical hostname in the server section of the UNC:
> >
> >        //server/share
> >
> > ...it doesn't work. The upcall will try to assemble a principal name
> > like:
> >
> >        cifs/server@...
>
> I haven't noticed this case because in AD domain, a machine
> account is registered with a fqdn and a hostname principals.
> (At least it is so in my company)
> So I were able to connect to //server/share
>
> > ...similar problem if I try to mount a CNAME for the server. This is
> > also a problem when you want to mix krb5 and DFS. I don't believe the
> > current code sets the hostname.
>
> The same thing, I someone wishes to provide access to a server via
> CNAMEs, he/she should create corresponding principals on KDC.
> This way it will not reduce security.
>

The problem here is that it's not necessarily evident *why* the mount
is failing at this point. If there were a way to spew a printk or log
message that makes this evident then I'd be more conducive to it.

> >
> > I don't think so. The patch below also changes:
> >
> > #define DKD_MUSTHAVE_SET (DKD_HAVE_IP|DKD_HAVE_VERSION|DKD_HAVE_SEC)
> >
> > ...or am I missing something?
>
> It just caught my eyes.
> Anyway I would initialized it, just in case.
>

Yeah, defensive coding doesn't hurt. I'll plan to initialize that once we
come to consensus on what to do about the other problem.

--
Jeff Layton <jlayton@...>
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client