Nathan,
OK, hopefully getting close here.
On 5/10/16 6:33 PM, Nathan Scott wrote:
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)
OK, I unconditionally set the flag now. and do the check on the client side.
+ /*
+ * 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.
I left all of these other checks in as well. I don't think they ever
get hit, but the logic seems OK to me.
Also included some QA and tutorial page. The QA checks that local
connections continue to function when we enforce remote client certificates.
For the tutorial, I tried to leave out any duplicate information that
may already be in the existing secure connections document. This only
focuses on setting up the new functionality. I'd appreciate any edits
here from someone else, as I've been looking at this too long and there
may be some additional text that would be helpful to a new user of this
feature.
I did run through it end to end with fresh VM installs and it works for
me as documented.
Existing QA all seems to be fine. All the "-g secure" tests pass. A
few random failures that seem to be in line with Ken's recent posts. I
didn't see anything that might be related to my changes.
In reality, if you don't use the new "-Q" flag to pmcd, there should be
no noticeable change in functionality.
I had to rebase in order to fix those 2 bad commit messages, so you'll
need a fresh pull.
https://github.com/ubccr/pcp/tree/client_certs
Thanks
Martins
|