[ovs-dev] [PATCH v2] Windows: Secure the namedpipe implementation
Sairam Venugopal
vsairam at vmware.com
Sat May 6 00:15:51 UTC 2017
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" <aserdean at cloudbasesolutions.com> 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: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
>> bounces at openvswitch.org] On Behalf Of Sairam Venugopal
>> Sent: Friday, May 5, 2017 9:30 PM
>> To: dev at openvswitch.org
>> 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 <vsairam at vmware.com>
>> ---
>> 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
>> dev at openvswitch.org
>> 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=
More information about the dev
mailing list