Hi Sai, Thanks a lot for the patch!
Could you please add the current user to the accepted list? We need them for testing and users to be able to run the binaries w/o elevation (i.e. make check on own laptop without elevated prompt). Let me know if I can help you with that! Some small nits inlined. Thanks, Alin. > -----Original Message----- > From: [email protected] [mailto:ovs-dev- > [email protected]] On Behalf Of Sairam Venugopal > Sent: Friday, May 5, 2017 9:30 PM > To: [email protected] > Subject: [ovs-dev] [PATCH v2] Windows: Secure the namedpipe > implementation > > Update the security policies around the creation of the namedpipe. The > current security updates restrict how the namedpipe can be accessed. > > - Disable Network access > - Windows Services can access the namedpipe > - Administrators can access the namedpipe > > Signed-off-by: Sairam Venugopal <[email protected]> > --- > lib/stream-windows.c | 91 > ++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 84 insertions(+), 7 deletions(-) > > diff --git a/lib/stream-windows.c b/lib/stream-windows.c index > 44b88bf..7cc6023 100644 > --- a/lib/stream-windows.c > +++ b/lib/stream-windows.c > @@ -40,6 +40,9 @@ static void maybe_unlink_and_free(char *path); > /* Default prefix of a local named pipe. */ #define LOCAL_PREFIX > "\\\\.\\pipe\\" > > +/* Size of the allowed PSIDs for securing Named Pipe. */ #define > +ALLOWED_PSIDS_SIZE 2 > + > /* This function has the purpose to remove all the slashes received in s. */ > static char * remove_slashes(char *s) @@ -402,16 +405,90 @@ static > HANDLE create_pnpipe(char *name) { > SECURITY_ATTRIBUTES sa; > - sa.nLength = sizeof(sa); > - sa.lpSecurityDescriptor = NULL; > + SID_IDENTIFIER_AUTHORITY sia = SECURITY_NT_AUTHORITY; > + DWORD aclSize; > + PSID allowedPsid[ALLOWED_PSIDS_SIZE]; > + PSID remoteAccessSid; > + PACL acl = NULL; > + PSECURITY_DESCRIPTOR psd = NULL; > + HANDLE npipe; > + > + /* Disable access over network. */ > + if (!AllocateAndInitializeSid(&sia, 1, SECURITY_NETWORK_RID, > + 0, 0, 0, 0, 0, 0, 0, &remoteAccessSid)) { [Alin Serdean] add print error pls. > + goto handle_error; > + } > + > + aclSize = sizeof(ACL) + sizeof(ACCESS_DENIED_ACE) + > + GetLengthSid(remoteAccessSid) - sizeof(DWORD); > + > + /* Allow Windows Services to access the Named Pipe. */ > + if (!AllocateAndInitializeSid(&sia, 1, SECURITY_LOCAL_SYSTEM_RID, > + 0, 0, 0, 0, 0, 0, 0, &allowedPsid[0])) { [Alin Serdean] add print error pls. > + goto handle_error; > + } > + > + /* Allow Administrators to access the Named Pipe. */ > + if (!AllocateAndInitializeSid(&sia, 2, SECURITY_BUILTIN_DOMAIN_RID, > + DOMAIN_ALIAS_RID_ADMINS, 0, 0, 0, 0, 0, 0, > + &allowedPsid[1])) { > + return INVALID_HANDLE_VALUE; [Alin Serdean] add print error pls. [Alin Serdean] goto handle_error ? > + } > + > + for (int i = 0; i < ALLOWED_PSIDS_SIZE; i++) { > + aclSize += sizeof(ACCESS_ALLOWED_ACE) + > + GetLengthSid(allowedPsid[i]) - > + sizeof(DWORD); > + } > + > + acl = xmalloc(aclSize); > + if (!InitializeAcl(acl, aclSize, ACL_REVISION)) { > + goto handle_error; > + } > + > + /* Add denied ACL. */ > + if (!AddAccessDeniedAce(acl, ACL_REVISION, > + GENERIC_ALL, remoteAccessSid)) { > + goto handle_error; > + } > + > + /* Add allowed ACLs. */ > + for (int i = 0; i < 2;i++) { [Alin Serdean] i< ALLOWED_PSIDS_SIZE? > + if (!AddAccessAllowedAce(acl, ACL_REVISION, > + GENERIC_ALL, allowedPsid[i])) { [Alin Serdean] add print error > + goto handle_error; > + } > + } > + > + psd = xmalloc(SECURITY_DESCRIPTOR_MIN_LENGTH); > + if (!InitializeSecurityDescriptor(psd, SECURITY_DESCRIPTOR_REVISION)) { [Alin Serdean] add print error > + goto handle_error; > + } > + > + /* Set DACL. */ > + if (!SetSecurityDescriptorDacl(psd, TRUE, acl, FALSE)) { [Alin Serdean] add print error > + goto handle_error; > + } > + > + sa.nLength = sizeof sa; > sa.bInheritHandle = TRUE; > + sa.lpSecurityDescriptor = psd; > + > if (strlen(name) > 256) { > - VLOG_ERR_RL(&rl, "Named pipe name too long."); [Alin Serdean] leave print error > - return INVALID_HANDLE_VALUE; > + goto handle_error; > } > - return CreateNamedPipe(name, PIPE_ACCESS_DUPLEX | > FILE_FLAG_OVERLAPPED, > - PIPE_TYPE_MESSAGE | PIPE_READMODE_BYTE | > PIPE_WAIT, > - 64, BUFSIZE, BUFSIZE, 0, &sa); > + > + npipe = CreateNamedPipe(name, PIPE_ACCESS_DUPLEX | > FILE_FLAG_OVERLAPPED, > + PIPE_TYPE_MESSAGE | PIPE_READMODE_BYTE | > PIPE_WAIT, > + 64, BUFSIZE, BUFSIZE, 0, &sa); > + free(acl); > + free(psd); > + return npipe; > + > +handle_error: > + free(acl); > + free(psd); > + return INVALID_HANDLE_VALUE; > } > > /* Passive named pipe connect. This function creates a new named pipe > and > -- > 2.9.0.windows.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
