More cleanups for client cert support

Add documentation

Change build process to create the nssdb dir during make

Remove conditionals so protocol message from pmcd is always
the same. Deal with enforcement later in the handshake process.

build/rpm/pcp.spec.in

@@ -2425,7 +2425,6 @@ fi
 %post
 PCP_PMNS_DIR=@pcp_var_dir@/pmns
 PCP_LOG_DIR=@pcp_log_dir@
-PCP_CONFIG_DIR=@pcp_var_dir@/config
 # restore saved configs, if any
 test -s "$PCP_LOG_DIR/configs.sh" && source "$PCP_LOG_DIR/configs.sh"
 rm -f $PCP_LOG_DIR/configs.sh
@@ -2458,8 +2457,6 @@ chown -R pcp:pcp "$PCP_LOG_DIR/pmcd" 2>/dev/null
 chown -R pcp:pcp "$PCP_LOG_DIR/pmlogger" 2>/dev/null
 chown -R pcp:pcp "$PCP_LOG_DIR/pmie" 2>/dev/null
 chown -R pcp:pcp "$PCP_LOG_DIR/pmproxy" 2>/dev/null
-mkdir -p -m 755 "$PCP_CONFIG_DIR/nssdb"
-chown pcp:pcp "$PCP_CONFIG_DIR/nssdb" 2>/dev/null
 touch "$PCP_PMNS_DIR/.NeedRebuild"
 chmod 644 "$PCP_PMNS_DIR/.NeedRebuild"
 %if "@enable_systemd@" == "true"

man/man1/pcpintro.1

@@ -858,15 +858,16 @@ set before the first call into any of the PCP libraries.
 .TP
 .B PCP_ALLOW_BAD_CERT_DOMAIN
 When set, allow clients to accept certificates with mismatched
-domain names with no prompt when they are sent by pmcd or other
-server components.
-See
+domain names with no prompt when they are sent by 
+.B pmcd
+or other server components.  See
 .B PCP_SECURE_SOCKETS.
 .TP
 .B PCP_ALLOW_SERVER_SELF_CERT
 When set, allow clients to accept self-signed certificates with
-no prompt when they are sent by pmcd or other server components.
-See
+no prompt when they are sent by
+.B pmcd
+or other server components.  See
 .B PCP_SECURE_SOCKETS.
 .TP
 .B PCP_CONSOLE
@@ -949,9 +950,9 @@ string (for the older database methods).
 .B PCP_SECURE_DB_PATH
 When set, this variable specifies an alternate certficate database
 path for client tools.  Similar to the action of the -C option for 
-.BR pmcd (3)
+.BR pmcd (1)
 and
-.BR pmproxy (3).
+.BR pmproxy (1).
 .TP
 .B PCP_STDERR
 Many PCP tools support the environment variable

src/include/pcp.conf.in

@@ -151,6 +151,10 @@ PCP_HTML_DIR=@pcp_html_dir@
 # Standard path: /usr/share/pcp/demos
 PCP_DEMOS_DIR=@pcp_demos_dir@
 
+# directory for PCP specific nssdb
+# Standard path: /var/lib/pcp/config/nssdb
+PCP_NSSDB_DIR=@pcp_var_dir@/config/nssdb
+
 # awk
 PCP_AWK_PROG="@awk@"
 

src/include/pcp/impl.h

@@ -592,7 +592,8 @@ PCP_CALL extern void __pmConnectGetPorts(pmHostSpec *);
 /*
  * SSL/TLS/IPv6 support via NSS/NSPR.
  */
-PCP_CALL extern int __pmSecureServerSetup(const char *, const char *, const char *);
+PCP_CALL extern int __pmSecureServerSetup(const char *, const char *);
+PCP_CALL extern int __pmSecureServerCertificateSetup(const char *, const char *, const char *);
 PCP_CALL extern void __pmSecureServerShutdown(void);
 PCP_CALL extern int __pmSecureServerHandshake(int, int, __pmHashCtl *);
 PCP_CALL extern int __pmSecureClientHandshake(int, int, const char *, __pmHashCtl *);
@@ -856,6 +857,8 @@ PCP_DATA extern unsigned int *__pmPDUCntIn;
 PCP_DATA extern unsigned int *__pmPDUCntOut;
 PCP_CALL extern void __pmSetPDUCntBuf(unsigned *, unsigned *);
 
+PCP_CALL unsigned int __pmServerGetFeaturesFromPDU(__pmPDU *);
+
 /* timeout options for PDU handling */
 #define TIMEOUT_NEVER	 0
 #define TIMEOUT_DEFAULT	-1

src/libpcp/src/auxserver.c

@@ -862,10 +862,35 @@ __pmServerRequestPortString(int fd, char *buffer, size_t sz)
     return NULL;
 }
 
+unsigned int
+__pmServerGetFeaturesFromPDU(__pmPDU *pb)
+{
+    int         version;
+    int         challenge;
+    __pmPDUInfo         pduinfo;
+    int		sts;
+    unsigned int server_features=0;
+
+    version = __pmDecodeXtendError(pb, &sts, &challenge);
+
+    if( version >= 0 && version == PDU_VERSION2  && sts >=0 ){
+	pduinfo = __ntohpmPDUInfo(*(__pmPDUInfo *)&challenge);
+	server_features = pduinfo.features;
+    }
+    
+    return server_features;
+}
+
 #if !defined(HAVE_SECURE_SOCKETS)
 
 int
-__pmSecureServerSetup(const char *db, const char *passwd, const char *cert_nickname)
+__pmSecureServerSetup(const char *db, const char *passwd)
+{
+    return __pmSecureServerCertificateSetup(db, passwd, SECURE_SERVER_CERTIFICATE);
+}
+
+int
+__pmSecureServerCertificateSetup(const char *db, const char *passwd, const char *cert_nickname)
 {
     (void)db;
     (void)passwd;

src/libpcp/src/connect.c

@@ -91,7 +91,7 @@ negotiate_proxy(int fd, const char *hostname, int port)
  * pmcd if we can go ahead with the connection, else error.
  */
 static int
-check_feature_flags(int ctxflags, int features)
+check_feature_flags(int ctxflags, int features, int local_conn)
 {
     int		pduflags = 0;
 
@@ -103,15 +103,15 @@ check_feature_flags(int ctxflags, int features)
 	 */
 	pduflags |= PDU_FLAG_CREDS_REQD;
 
-    if (features & PDU_FLAG_CERT_REQD){
+    if ( (features & PDU_FLAG_CERT_REQD) && !local_conn ){
 	/*
-	 * This is a mandatory connection feature - pmcd must be
-	 * sent a trusted certificate.
+	 * This is a mandatory connection feature for remote connections.
+	 * pmcd must be sent a trusted certificate.
 	 */
 	pduflags |= PDU_FLAG_CERT_REQD;
 	if( !(ctxflags & PM_CTXFLAG_SECURE) ){
-		/* PMCD requires a client cert, but we are not even setup for secure connections */
-		return PM_ERR_NEEDCLIENTCERT;
+	    /* PMCD requires a client cert, but we are not even setup for secure connections */
+	    return PM_ERR_NEEDCLIENTCERT;
 	}
      }
 
@@ -195,6 +195,26 @@ attributes_handshake(int fd, int flags, const char *host, __pmHashCtl *attrs)
     return 0;
 }
 
+static int
+is_local_connection(const char *hostname, __pmHashCtl *attrs)
+{
+   /* 
+    * If attrs has PCP_ATTR_UNIXSOCK or PCP_ATTR_LOCAL
+    * OR
+    * If hostname contains localhost
+    *	We consider this a local connection and don't enforce CERT_REQD
+    */
+
+    if ( __pmHashSearch(PCP_ATTR_UNIXSOCK, attrs) || 
+	 __pmHashSearch(PCP_ATTR_LOCAL, attrs) || 
+	 strstr( hostname, "localhost") ){
+	 return 1;
+    }
+    else{
+	return 0;
+    }
+}
+
 /*
  * client connects to pmcd handshake
  */
@@ -236,9 +256,12 @@ __pmConnectHandshake(int fd, const char *hostname, int ctxflags, __pmHashCtl *at
 	    __pmPDUInfo		pduinfo;
 	    __pmVersionCred	handshake;
 	    int			pduflags;
+	    int			local_conn;
+
+	    local_conn = is_local_connection( hostname, attrs );
 
 	    pduinfo = __ntohpmPDUInfo(*(__pmPDUInfo *)&challenge);
-	    pduflags = sts = check_feature_flags(ctxflags, pduinfo.features);
+	    pduflags = sts = check_feature_flags(ctxflags, pduinfo.features, local_conn);
 	    if (sts < 0) {
 		__pmUnpinPDUBuf(pb);
 		return sts;

src/libpcp/src/exports

@@ -522,3 +522,7 @@ PCP_3.14 {
     pmRegisterDerivedMetric;
 } PCP_3.13;
 
+PCP_3.15 {
+    __pmSecureServerCertificateSetup;
+    __pmServerGetFeaturesFromPDU;
+} PCP_3.14;

src/libpcp/src/secureserver.c

@@ -235,7 +235,13 @@ serverdb(char *path, size_t size, char *db_method)
 }
 
 int
-__pmSecureServerSetup(const char *db, const char *passwd, const char *cert_nickname)
+__pmSecureServerSetup(const char *db, const char *passwd)
+{
+    return __pmSecureServerCertificateSetup(db, passwd, SECURE_SERVER_CERTIFICATE);
+}
+
+int
+__pmSecureServerCertificateSetup(const char *db, const char *passwd, const char *cert_nickname)
 {
     PM_INIT_LOCKS();
     PM_LOCK(__pmLock_libpcp);
@@ -316,8 +322,7 @@ __pmSecureServerInit(void)
      * pmproxy acts as both a client and server. Since the
      * server init path happens first, the db previously
      * got opened readonly.  Instead try to open RW.
-     * Any downside to doing this by default?
-     * Should this be conditional on something?
+     * Fallback if there is an error.
      */
 
     secsts = NSS_InitReadWrite(secure_server.database_path);

src/pmcd/src/dopdus.c

@@ -1065,6 +1065,16 @@ ConnectionAttributes(ClientInfo *cp, int flags)
     return 0;
 }
 
+static int
+CheckCertRequired(ClientInfo *cp)
+{
+    if(__pmServerHasFeature(PM_SERVER_FEATURE_CERT_REQD))
+	if ( !__pmSockAddrIsLoopBack(cp->addr) && !__pmSockAddrIsUnix(cp->addr) )
+	    return 1;
+
+    return 0;
+}
+
 int
 DoCreds(ClientInfo *cp, __pmPDU *pb)
 {
@@ -1128,12 +1138,18 @@ DoCreds(ClientInfo *cp, __pmPDU *pb)
     if (sts >= 0 && version)
 	sts = __pmSetVersionIPC(cp->fd, version);
 
-    /* Not sure if this will ever be hit. All cases checked during handshake? */
-    if( ( __pmServerHasFeature(PM_SERVER_FEATURE_CERT_REQD) && (flags & PDU_FLAG_SECURE) == 0 )){
-	if( !__pmSockAddrIsLoopBack(cp->addr) && !__pmSockAddrIsUnix(cp->addr)){
-	    return PM_ERR_NEEDCLIENTCERT;
-	}
-    }
+    /*
+     * In normal operation, some of this code is redundant. A 
+     * remote client should error out during initial handshake
+     * if it does not support client certs.
+     *
+     * We still need to check local connections and allow those through
+     * in all cases.
+     */
+
+    if ( CheckCertRequired(cp) )
+        if ( (flags & PDU_FLAG_SECURE) == 0 )
+            return PM_ERR_NEEDCLIENTCERT;
 
     if (sts >= 0 && flags) {
 	/*

src/pmcd/src/pmcd.c

@@ -668,10 +668,8 @@ CheckNewClient(__pmFdSet * fdset, int rfd, int family)
 		cp->pduInfo.features |= PDU_FLAG_COMPRESS;
 	    if (__pmServerHasFeature(PM_SERVER_FEATURE_AUTH))       /*optional*/
 		cp->pduInfo.features |= PDU_FLAG_AUTH;
-            if (__pmServerHasFeature(PM_SERVER_FEATURE_CERT_REQD)){  /*required for remote connections only*/
-		if( !__pmSockAddrIsLoopBack(cp->addr) && !__pmSockAddrIsUnix(cp->addr)){
-                    cp->pduInfo.features |= PDU_FLAG_CERT_REQD;
-		}
+            if (__pmServerHasFeature(PM_SERVER_FEATURE_CERT_REQD)){ /* Required for remote connections only */
+		cp->pduInfo.features |= PDU_FLAG_CERT_REQD;	    /* Enforced in connect.c:check_feature_flags */
 	    }
 	    if (__pmServerHasFeature(PM_SERVER_FEATURE_CREDS_REQD)) /*required*/
 		cp->pduInfo.features |= PDU_FLAG_CREDS_REQD;
@@ -952,7 +950,7 @@ main(int argc, char *argv[])
 	    DontStart();
     }
 
-    if (__pmSecureServerSetup(certdb, dbpassfile, cert_nickname) < 0)
+    if (__pmSecureServerCertificateSetup(certdb, dbpassfile, cert_nickname) < 0)
 	DontStart();
 
     PrintAgentInfo(stderr);

src/pmproxy/pmproxy.c

@@ -183,6 +183,16 @@ ParseOptions(int argc, char *argv[], int *nports)
     }
 }
 
+static int
+CheckCertRequired(ClientInfo *cp)
+{
+    if( cp->server_features & PDU_FLAG_CERT_REQD )
+	if ( !__pmSockAddrIsLoopBack(cp->addr) && !__pmSockAddrIsUnix(cp->addr) )
+	    return 1;
+
+    return 0;
+}
+
 static void
 CleanupClient(ClientInfo *cp, int sts)
 {
@@ -208,7 +218,6 @@ VerifyClient(ClientInfo *cp, __pmPDU *pb)
     __pmPDUHdr *header = (__pmPDUHdr *)pb;
     __pmHashCtl attrs = { 0 }; /* TODO */
     __pmCred *credlist;
-    unsigned int toggle_cert_required=0;
 
     /* first check that this is a credentials PDU */
     if (header->type != PDU_CREDS)
@@ -230,26 +239,23 @@ VerifyClient(ClientInfo *cp, __pmPDU *pb)
 	free(credlist);
 
     /*
-     * Enforce PDU_FLAG_CERT_REQD for remote connections
-     *
-     * Not sure if this will ever be hit. Will all cases be caught  during handshake? See connect.c
-     */ 
-    if( ( ( cp->server_features & PDU_FLAG_CERT_REQD ) && ( (flags & PDU_FLAG_SECURE) == 0) )){
-	if( !__pmSockAddrIsLoopBack(cp->addr) && !__pmSockAddrIsUnix(cp->addr)){
-	    return PM_ERR_NEEDCLIENTCERT;
-	}
-    }
-
-    /*
      * If the server advertises PDU_FLAG_CERT_REQD, add it to flags
      * so we can setup the connection properly with the client.
-     * The client should have errored out in the initial handshake if it
-     * didn't support secure connections, so we should only end up
-     * here if both client and server support this.
+     *
+     * In normal operation, some of this code is redundant. A 
+     * remote client should error out during initial handshake
+     * if it does not support client certs.
+     *
+     * We still need to check local connections and allow those through
+     * in all cases.
      */
 
-    if( (cp->server_features & PDU_FLAG_CERT_REQD) )
-	flags |= PDU_FLAG_CERT_REQD;
+    if ( CheckCertRequired(cp) ){
+	if (flags & PDU_FLAG_SECURE)
+	    flags |= PDU_FLAG_CERT_REQD;
+	else
+	    return PM_ERR_NEEDCLIENTCERT;
+    }
 
     /* need to ensure both the pmcd and client channel use flags */
 
@@ -347,19 +353,11 @@ HandleInput(__pmFdSet *fdsPtr)
 	 */
 
 	if( (!cp->status.allowed) && (sts == PDU_ERROR) ){
-	    int         version;
-	    int         challenge;
-	    __pmPDUInfo         pduinfo;
 	    unsigned int server_features;
-	    int		lsts;
-	    version = __pmDecodeXtendError(pb, &lsts, &challenge);
-	    if( version >= 0 && version == PDU_VERSION2  && lsts >=0 ){
-		pduinfo = __ntohpmPDUInfo(*(__pmPDUInfo *)&challenge);
-		server_features = pduinfo.features;
-		if( server_features & PDU_FLAG_CERT_REQD ){
-		    /* Add as a server feature */
-		    cp->server_features |= PDU_FLAG_CERT_REQD;
-		}
+	    server_features = __pmServerGetFeaturesFromPDU( pb );
+	    if( server_features & PDU_FLAG_CERT_REQD ){
+		/* Add as a server feature */
+		cp->server_features |= PDU_FLAG_CERT_REQD;
 	    }
 	}
 
@@ -591,7 +589,7 @@ main(int argc, char *argv[])
     /* lose root privileges if we have them */
     __pmSetProcessIdentity(username);
 
-    if (__pmSecureServerSetup(certdb, dbpassfile, cert_nickname) < 0)
+    if (__pmSecureServerCertificateSetup(certdb, dbpassfile, cert_nickname) < 0)
 	DontStart();
 
     /* all the work is done here */