Hi Martins,
----- Original Message -----
> > [...]
> >
> > 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?
>
Hmm - it would be really good to have the protocol the same no matter what
kind of pmcd/pmproxy connection is being established. (for debugging, for
wireshark, for QA, etc).
To that end, maybe the best approach is to short-circuit the error handling
at both ends with those localhost/af_unix checks?
(CC'ing Ryan - these changes will need a Wireshark dissector update)
> >
> > + /*
> > + * 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
> > + */
> > [...]
> 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 like the safety net that code is providing - if there was some unexpected
way through through the protocol exchange (maybe with a maliciously crafted
PDU sequence, not through ordinary libpcp code paths) then that would serve
as a fallback/catch-all such that those connections could not proceed.
cheers.
--
Nathan
|