Nathan,
On 5/10/16 2:01 AM, Nathan Scott wrote:
diff --git a/src/libpcp/src/connect.c b/src/libpcp/src/connect.c
index 0386396..2548868 100644
--- a/src/libpcp/src/connect.c
+++ b/src/libpcp/src/connect.c
@@ -103,6 +103,18 @@ check_feature_flags(int ctxflags, int features)
*/
pduflags |= PDU_FLAG_CREDS_REQD;
+ if (features & PDU_FLAG_CERT_REQD){
+ /*
+ * This is a mandatory connection feature - 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;
+ }
Logic is exactly what I was expecting to see, glad that worked out as
anticipated.
OK, but based on your comment below. This above section may then need
to be cut?
@@ -660,6 +669,11 @@ 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;
I think there's no harm setting the protocol bit for all connections
(local and remote) saying that certs are required by this pmcd - but
just not enforce for local: connections. Keeps the pmcd code simpler
here and the protocol message the same for all clients.
So, If I do this, I think I need to dump the check in connect.c. Since
in all cases a client will now get this flag, even local ones. Then
enforcement will happen in pmcd and pmproxy as in the existing check below.
+ /*
+ * 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;
+ }
+ }
Quirky use of whitespace there again - to answer the question though,
yes, definitely enforce this here, as with pmcd.
I think either way is probably fine, just need to do some testing to be
sure. This solution would just have bad connections error out a little
later in the handshake. But maybe getting rid of some code is worth it.
I will be offline for a few days but let me know what you'd prefer.
Thanks
Martins
|