Hi Alin, I think the existing behavior requires an elevated Command Prompt/powershell to execute OVS commands.
Eg: Running 'ovs-appctl list-commands’ on non-Adminsitrator CMD will complain that access is denied to the namedpipe. Are you thinking of other cases where the current user is non-administrator and can still access the namedpipe? Thanks, Sairam On 5/5/17, 4:17 PM, "Alin Serdean" <[email protected]> wrote: >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://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIFAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=flY_QW-glmQUGjnT5sA0B0K1I1ZHWrHkrFOcF5m4OlE&s=RqQn3FpK7VLCO4cBnRcdKjlGMz7rEVanZV6qowAWJ2I&e= >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
