krb5 commit: Correctly log IPv6 addresses in kadmind

Greg Hudson ghudson at MIT.EDU
Tue Nov 26 12:12:47 EST 2013


https://github.com/krb5/krb5/commit/5384f45e728957da20ecf82d8cf567945a2bbf6e
commit 5384f45e728957da20ecf82d8cf567945a2bbf6e
Author: Greg Hudson <ghudson at mit.edu>
Date:   Mon Nov 25 11:46:47 2013 -0500

    Correctly log IPv6 addresses in kadmind
    
    Define client_addr() in server_stubs.c and use it consistently in that
    file and ipropd_svc.c to get the client address from a transport
    handle.  In it, call getpeername() on the client socket and use
    inet_ntop() on the result, instead of using inet_ntoa() on the IPv4
    socket address.  Provide a log_badauth2 callback to GSSRPC, so that we
    get a transport handle instead of an IPv4 socket address, and use
    client_addr() within it instead of inet_ntoa().
    
    ticket: 7770
    target_version: 1.12
    tags: pullup

 src/kadmin/server/ipropd_svc.c   |   22 +++++++---------------
 src/kadmin/server/kadm_rpc_svc.c |   18 ++++++------------
 src/kadmin/server/misc.h         |    9 +++------
 src/kadmin/server/ovsec_kadmd.c  |   29 ++++++++++-------------------
 src/kadmin/server/server_stubs.c |   30 ++++++++++++++++++++++++------
 5 files changed, 50 insertions(+), 58 deletions(-)

diff --git a/src/kadmin/server/ipropd_svc.c b/src/kadmin/server/ipropd_svc.c
index 4a25998..6d5ba3c 100644
--- a/src/kadmin/server/ipropd_svc.c
+++ b/src/kadmin/server/ipropd_svc.c
@@ -38,13 +38,6 @@ extern short l_port;
 extern char *kdb5_util;
 extern char *kprop;
 extern char *dump_file;
-static char abuf[33];
-
-/* Result is stored in a static buffer and is invalidated by the next call. */
-static const char *client_addr(struct svc_req *svc) {
-    strlcpy(abuf, inet_ntoa(svc->rq_xprt->xp_raddr.sin_addr), sizeof(abuf));
-    return abuf;
-}
 
 static char *reply_ok_str	= "UPDATE_OK";
 static char *reply_err_str	= "UPDATE_ERROR";
@@ -193,7 +186,7 @@ iprop_get_updates_1_svc(kdb_last_t *arg, struct svc_req *rqstp)
 
 	krb5_klog_syslog(LOG_NOTICE, LOG_UNAUTH, whoami,
 			 client_name, service_name,
-			 client_addr(rqstp));
+			 client_addr(rqstp->rq_xprt));
 	goto out;
     }
 
@@ -223,7 +216,7 @@ iprop_get_updates_1_svc(kdb_last_t *arg, struct svc_req *rqstp)
 		     obuf,
 		     ((kret == 0) ? "success" : error_message(kret)),
 		     client_name, service_name,
-		     client_addr(rqstp));
+		     client_addr(rqstp->rq_xprt));
 
 out:
     if (nofork)
@@ -320,7 +313,7 @@ ipropx_resync(uint32_t vers, struct svc_req *rqstp)
 	DPRINT("%s: Permission denied\n", whoami);
 	krb5_klog_syslog(LOG_NOTICE, LOG_UNAUTH, whoami,
 			 client_name, service_name,
-			 client_addr(rqstp));
+			 client_addr(rqstp->rq_xprt));
 	goto out;
     }
 
@@ -425,12 +418,12 @@ ipropx_resync(uint32_t vers, struct svc_req *rqstp)
 
 	DPRINT("%s: spawned resync process %d, client=%s, "
 		"service=%s, addr=%s\n", whoami, fret, client_name,
-		service_name, client_addr(rqstp));
+		service_name, client_addr(rqstp->rq_xprt));
 	krb5_klog_syslog(LOG_NOTICE,
 			 _("Request: %s, spawned resync process %d, client=%s, service=%s, addr=%s"),
 			 whoami, fret,
 			 client_name, service_name,
-			 client_addr(rqstp));
+			 client_addr(rqstp->rq_xprt));
 
 	goto out;
     }
@@ -493,8 +486,7 @@ check_iprop_rpcsec_auth(struct svc_req *rqstp)
 	  krb5_klog_syslog(LOG_ERR,
 			   _("check_rpcsec_auth: failed inquire_context, "
 			     "stat=%u"), maj_stat);
-	  log_badauth(maj_stat, min_stat,
-		      &rqstp->rq_xprt->xp_raddr, NULL);
+	  log_badauth(maj_stat, min_stat, rqstp->rq_xprt, NULL);
 	  goto fail_name;
      }
 
@@ -547,7 +539,7 @@ krb5_iprop_prog_1(struct svc_req *rqstp,
     if (!check_iprop_rpcsec_auth(rqstp)) {
 	krb5_klog_syslog(LOG_ERR, _("authentication attempt failed: %s, RPC "
 				    "authentication flavor %d"),
-			 inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr),
+			 client_addr(rqstp->rq_xprt),
 			 rqstp->rq_cred.oa_flavor);
 	svcerr_weakauth(transp);
 	return;
diff --git a/src/kadmin/server/kadm_rpc_svc.c b/src/kadmin/server/kadm_rpc_svc.c
index a75bdb8..3837931 100644
--- a/src/kadmin/server/kadm_rpc_svc.c
+++ b/src/kadmin/server/kadm_rpc_svc.c
@@ -12,9 +12,6 @@
 #include <krb5.h>
 #include <kadm5/admin.h>
 #include <adm_proto.h>
-#ifdef HAVE_ARPA_INET_H
-#include <arpa/inet.h>
-#endif
 #include "misc.h"
 #include "kadm5/server_internal.h"
 
@@ -69,9 +66,9 @@ void kadm_1(rqstp, transp)
      if (rqstp->rq_cred.oa_flavor != AUTH_GSSAPI &&
 	 !check_rpcsec_auth(rqstp)) {
 	  krb5_klog_syslog(LOG_ERR, "Authentication attempt failed: %s, "
-		 "RPC authentication flavor %d",
-		 inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr),
-		 rqstp->rq_cred.oa_flavor);
+			   "RPC authentication flavor %d",
+			   client_addr(rqstp->rq_xprt),
+			   rqstp->rq_cred.oa_flavor);
 	  svcerr_weakauth(transp);
 	  return;
      }
@@ -227,8 +224,7 @@ void kadm_1(rqstp, transp)
 
      default:
 	  krb5_klog_syslog(LOG_ERR, "Invalid KADM5 procedure number: %s, %d",
-		 inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr),
-		 rqstp->rq_proc);
+			   client_addr(rqstp->rq_xprt), rqstp->rq_proc);
 	  svcerr_noproc(transp);
 	  return;
      }
@@ -278,8 +274,7 @@ check_rpcsec_auth(struct svc_req *rqstp)
      if (maj_stat != GSS_S_COMPLETE) {
 	  krb5_klog_syslog(LOG_ERR, _("check_rpcsec_auth: failed "
 				      "inquire_context, stat=%u"), maj_stat);
-	  log_badauth(maj_stat, min_stat,
-		      &rqstp->rq_xprt->xp_raddr, NULL);
+	  log_badauth(maj_stat, min_stat, rqstp->rq_xprt, NULL);
 	  goto fail_name;
      }
 
@@ -335,8 +330,7 @@ gss_to_krb5_name_1(struct svc_req *rqstp, krb5_context ctx, gss_name_t gss_name,
      if ((status != GSS_S_COMPLETE) || (gss_type != gss_nt_krb5_name)) {
 	  krb5_klog_syslog(LOG_ERR, _("gss_to_krb5_name: failed display_name "
 				      "status %d"), status);
-	  log_badauth(status, minor_stat,
-		      &rqstp->rq_xprt->xp_raddr, NULL);
+	  log_badauth(status, minor_stat, rqstp->rq_xprt, NULL);
 	  return 0;
      }
      str = malloc(gss_str->length +1);
diff --git a/src/kadmin/server/misc.h b/src/kadmin/server/misc.h
index 3d2eda1..ea0fc7d 100644
--- a/src/kadmin/server/misc.h
+++ b/src/kadmin/server/misc.h
@@ -9,10 +9,6 @@
 
 #include "net-server.h"         /* for krb5_fulladdr */
 
-void
-log_badauth(OM_uint32 major, OM_uint32 minor,
-            struct sockaddr_in *addr, char *data);
-
 int
 setup_gss_names(struct svc_req *, gss_buffer_desc *,
                 gss_buffer_desc *);
@@ -55,8 +51,9 @@ gss_to_krb5_name_1(struct svc_req *rqstp, krb5_context ctx, gss_name_t gss_name,
 
 void reset_db(void);
 
-void log_badauth(OM_uint32 major, OM_uint32 minor,
-                 struct sockaddr_in *addr, char *data);
+void log_badauth(OM_uint32 major, OM_uint32 minor, SVCXPRT *xprt, char *data);
+
+const char *client_addr(SVCXPRT *xprt);
 
 /* network.c */
 #include "net-server.h"
diff --git a/src/kadmin/server/ovsec_kadmd.c b/src/kadmin/server/ovsec_kadmd.c
index 1273b07..87aa47a 100644
--- a/src/kadmin/server/ovsec_kadmd.c
+++ b/src/kadmin/server/ovsec_kadmd.c
@@ -43,7 +43,6 @@
 #include    <sys/socket.h>
 #include    <unistd.h>
 #include    <netinet/in.h>
-#include    <arpa/inet.h>  /* inet_ntoa */
 #include    <netdb.h>
 #include    <gssrpc/rpc.h>
 #include    <gssapi/gssapi.h>
@@ -71,8 +70,7 @@ gss_name_t gss_kadmin_name = NULL;
 void *global_server_handle;
 
 char *build_princ_name(char *name, char *realm);
-void log_badauth(OM_uint32 major, OM_uint32 minor,
-                 struct sockaddr_in *addr, char *data);
+void log_badauth(OM_uint32 major, OM_uint32 minor, SVCXPRT *xprt, char *data);
 void log_badverf(gss_name_t client_name, gss_name_t server_name,
                  struct svc_req *rqst, struct rpc_msg *msg,
                  char *data);
@@ -489,11 +487,11 @@ kterr:
     (void) gss_import_name(&OMret, &in_buf, nt_krb5_name_oid,
                            &gss_changepw_name);
 
-    svcauth_gssapi_set_log_badauth_func(log_badauth, NULL);
+    svcauth_gssapi_set_log_badauth2_func(log_badauth, NULL);
     svcauth_gssapi_set_log_badverf_func(log_badverf, NULL);
     svcauth_gssapi_set_log_miscerr_func(log_miscerr, NULL);
 
-    svcauth_gss_set_log_badauth_func(log_badauth, NULL);
+    svcauth_gss_set_log_badauth2_func(log_badauth, NULL);
     svcauth_gss_set_log_badverf_func(log_badverf, NULL);
     svcauth_gss_set_log_miscerr_func(log_miscerr, NULL);
 
@@ -770,7 +768,7 @@ void log_badverf(gss_name_t client_name, gss_name_t server_name,
     OM_uint32 minor;
     gss_buffer_desc client, server;
     gss_OID gss_type;
-    char *a;
+    const char *a;
     rpcproc_t proc;
     unsigned int i;
     const char *procname;
@@ -798,7 +796,7 @@ void log_badverf(gss_name_t client_name, gss_name_t server_name,
         slen = server.length;
     }
     trunc_name(&slen, &sdots);
-    a = inet_ntoa(rqst->rq_xprt->xp_raddr.sin_addr);
+    a = client_addr(rqst->rq_xprt);
 
     proc = msg->rm_call.cb_proc;
     procname = NULL;
@@ -844,11 +842,8 @@ void log_badverf(gss_name_t client_name, gss_name_t server_name,
 void log_miscerr(struct svc_req *rqst, struct rpc_msg *msg,
                  char *error, char *data)
 {
-    char *a;
-
-    a = inet_ntoa(rqst->rq_xprt->xp_raddr.sin_addr);
-    krb5_klog_syslog(LOG_NOTICE, _("Miscellaneous RPC error: %s, %s"), a,
-                     error);
+    krb5_klog_syslog(LOG_NOTICE, _("Miscellaneous RPC error: %s, %s"),
+                     client_addr(rqst->rq_xprt), error);
 }
 
 
@@ -870,18 +865,14 @@ void log_miscerr(struct svc_req *rqst, struct rpc_msg *msg,
  * Logs the GSS-API error via krb5_klog_syslog(); see functional spec for
  * format.
  */
-void log_badauth(OM_uint32 major, OM_uint32 minor,
-                 struct sockaddr_in *addr, char *data)
+void log_badauth(OM_uint32 major, OM_uint32 minor, SVCXPRT *xprt, char *data)
 {
-    char *a;
-
     /* Authentication attempt failed: <IP address>, <GSS-API error */
     /* strings> */
 
-    a = inet_ntoa(addr->sin_addr);
-
     krb5_klog_syslog(LOG_NOTICE, _("Authentication attempt failed: %s, "
-                                   "GSS-API error strings are:"), a);
+                                   "GSS-API error strings are:"),
+                     client_addr(xprt));
     log_badauth_display_status("   ", major, minor);
     krb5_klog_syslog(LOG_NOTICE, _("   GSS-API error strings complete."));
 }
diff --git a/src/kadmin/server/server_stubs.c b/src/kadmin/server/server_stubs.c
index 446eaca..5312792 100644
--- a/src/kadmin/server/server_stubs.c
+++ b/src/kadmin/server/server_stubs.c
@@ -5,6 +5,7 @@
  */
 
 #include <k5-platform.h>
+#include <socket-utils.h>
 #include <gssapi/gssapi.h>
 #include <gssapi/gssapi_krb5.h> /* for gss_nt_krb5_name */
 #include <krb5.h>
@@ -13,7 +14,6 @@
 #include <kadm5/server_internal.h>
 #include <kadm5/server_acl.h>
 #include <syslog.h>
-#include <arpa/inet.h>  /* inet_ntoa */
 #include <adm_proto.h>  /* krb5_klog_syslog */
 #include "misc.h"
 
@@ -143,6 +143,24 @@ static void free_server_handle(kadm5_server_handle_t handle)
     free(handle);
 }
 
+/* Result is stored in a static buffer and is invalidated by the next call. */
+const char *
+client_addr(SVCXPRT *xprt)
+{
+    static char abuf[128];
+    struct sockaddr_storage ss;
+    socklen_t len = sizeof(ss);
+    const char *p = NULL;
+
+    if (getpeername(xprt->xp_sock, ss2sa(&ss), &len) != 0)
+        return "(unknown)";
+    if (ss2sa(&ss)->sa_family == AF_INET)
+        p = inet_ntop(AF_INET, &ss2sin(&ss)->sin_addr, abuf, sizeof(abuf));
+    else if (ss2sa(&ss)->sa_family == AF_INET6)
+        p = inet_ntop(AF_INET6, &ss2sin6(&ss)->sin6_addr, abuf, sizeof(abuf));
+    return (p == NULL) ? "(unknown)" : p;
+}
+
 /*
  * Function: setup_gss_names
  *
@@ -277,7 +295,7 @@ log_unauth(
                             op, (int)tlen, target, tdots,
                             (int)clen, (char *)client->value, cdots,
                             (int)slen, (char *)server->value, sdots,
-                            inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr));
+                            client_addr(rqstp->rq_xprt));
 }
 
 static int
@@ -308,7 +326,7 @@ log_done(
                             op, (int)tlen, target, tdots, errmsg,
                             (int)clen, (char *)client->value, cdots,
                             (int)slen, (char *)server->value, sdots,
-                            inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr));
+                            client_addr(rqstp->rq_xprt));
 }
 
 generic_ret *
@@ -614,7 +632,7 @@ rename_principal_2_svc(rprinc_arg *arg, struct svc_req *rqstp)
                          (int)tlen2, prime_arg2, tdots2,
                          (int)clen, (char *)client_name.value, cdots,
                          (int)slen, (char *)service_name.value, sdots,
-                         inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr));
+                         client_addr(rqstp->rq_xprt));
     } else {
         ret.code = kadm5_rename_principal((void *)handle, arg->src,
                                           arg->dest);
@@ -631,7 +649,7 @@ rename_principal_2_svc(rprinc_arg *arg, struct svc_req *rqstp)
                          errmsg ? errmsg : _("success"),
                          (int)clen, (char *)client_name.value, cdots,
                          (int)slen, (char *)service_name.value, sdots,
-                         inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr));
+                         client_addr(rqstp->rq_xprt));
 
         if (errmsg != NULL)
             krb5_free_error_message(handle->context, errmsg);
@@ -1774,7 +1792,7 @@ generic_ret *init_2_svc(krb5_ui_4 *arg, struct svc_req *rqstp)
                      errmsg ? errmsg : _("success"),
                      (int)clen, (char *)client_name.value, cdots,
                      (int)slen, (char *)service_name.value, sdots,
-                     inet_ntoa(rqstp->rq_xprt->xp_raddr.sin_addr),
+                     client_addr(rqstp->rq_xprt),
                      ret.api_version & ~(KADM5_API_VERSION_MASK),
                      rqstp->rq_cred.oa_flavor);
     if (errmsg != NULL)


More information about the cvs-krb5 mailing list