[ovs-dev] [PATCH] Grant Access Privilege of Named Pipe to Creator

Alin Serdean aserdean at cloudbasesolutions.com
Wed Jan 22 04:19:15 UTC 2020


Hi,

Sorry I missed the email.

The direction sounds ok with me. It will surely help with unit tests, since right now
they require elevated permissions.

Adding also Anand in the loop.
Anand do you like the idea?

Please also add a few lines to the documentation so users are aware of the
change.

The patch as is, fails to apply. Rebase on master.

Also please change the title so it is in compliance with:
http://docs.openvswitch.org/en/latest/internals/contributing/submitting-patches/#email-subject 

Thanks,
Alin.

> -----Original Message-----
> From: Ning Wu <nwu at vmware.com>
> Sent: Monday, January 20, 2020 4:16 AM
> To: dev at openvswitch.org; Alin Serdean <aserdean at cloudbasesolutions.com>
> Cc: Lina Li <linali at vmware.com>; Roy Luo <luoroy at vmware.com>
> Subject: RE: [PATCH] Grant Access Privilege of Named Pipe to Creator
> 
> Hi Alin,
> 
> 
> 
> Could you please help to give some comment?
> 
> Thanks.
> 
> 
> 
> From: Ning Wu
> Sent: Friday, January 17, 2020 15:15
> To: dev at openvswitch.org; Alin Serdean <aserdean at cloudbasesolutions.com>
> Cc: Lina Li <linali at vmware.com>; Roy Luo <luoroy at vmware.com>
> Subject: [PATCH] Grant Access Privilege of Named Pipe to Creator
> 
> 
> 
> Current implementation of ovs on windows only allows LocalSystem and
> 
> Administrators to access the named pipe created with API of ovs.
> 
> Thus any service that needs to invoke the API to create named pipe
> 
> has to run as System account to interactive with ovs. It causes the
> 
> system more vulnerable if one of those services was break into.
> 
> The patch adds the creator owner account to allowed ACLs.
> 
> 
> 
> Signed-off-by: Ning Wu <nwu at vmware.com <mailto:nwu at vmware.com> >
> 
> ---
> 
> lib/stream-windows.c | 33 ++++++++++++++++++++++++++++++++-
> 
> 1 file changed, 32 insertions(+), 1 deletion(-)
> 
> 
> 
> diff --git a/lib/stream-windows.c b/lib/stream-windows.c
> 
> index 34bc610..0cad927 100644
> 
> --- a/lib/stream-windows.c
> 
> +++ b/lib/stream-windows.c
> 
> @@ -41,7 +41,7 @@ static void maybe_unlink_and_free(char *path);
> 
> #define LOCAL_PREFIX "\\\\.\\pipe\\ <file://.//pipe/> "
> 
> 
> 
> /* Size of the allowed PSIDs for securing Named Pipe. */
> 
> -#define ALLOWED_PSIDS_SIZE 2
> 
> +#define ALLOWED_PSIDS_SIZE 3
> 
> 
> 
> /* This function has the purpose to remove all the slashes received in s. */
> 
> static char *
> 
> @@ -412,6 +412,9 @@ create_pnpipe(char *name)
> 
>      PACL acl = NULL;
> 
>      PSECURITY_DESCRIPTOR psd = NULL;
> 
>      HANDLE npipe;
> 
> +    HANDLE hToken = NULL;
> 
> +    DWORD dwBufSize = 0;
> 
> +    PTOKEN_USER pTokenUsr = NULL;
> 
> 
> 
>      /* Disable access over network. */
> 
>      if (!AllocateAndInitializeSid(&sia, 1, SECURITY_NETWORK_RID,
> 
> @@ -438,6 +441,32 @@ create_pnpipe(char *name)
> 
>          goto handle_error;
> 
>      }
> 
> 
> 
> +    /* Open the access token of calling process */
> 
> +    if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &hToken)) {
> 
> +       VLOG_ERR_RL(&rl, "Error opening access token of calling process.");
> 
> +        goto handle_error;
> 
> +    }
> 
> +
> 
> +    /* get the buffer size buffer needed for SID */
> 
> +    GetTokenInformation(hToken, TokenUser, NULL, 0, &dwBufSize);
> 
> +
> 
> +    pTokenUsr = xmalloc(dwBufSize);
> 
> +    memset(pTokenUsr, 0, dwBufSize);
> 
> +
> 
> +    /* Retrieve the token information in a TOKEN_USER structure. */
> 
> +    if (!GetTokenInformation(hToken, TokenUser, pTokenUsr, dwBufSize,
> 
> +        &dwBufSize)) {
> 
> +        VLOG_ERR_RL(&rl, "Error retrieving token information.");
> 
> +        goto handle_error;
> 
> +    }
> 
> +    CloseHandle(hToken);
> 
> +
> 
> +    if (!IsValidSid(pTokenUsr->User.Sid)) {
> 
> +        VLOG_ERR_RL(&rl, "Invalid SID.");
> 
> +        goto handle_error;
> 
> +    }
> 
> +    allowedPsid[2] = pTokenUsr->User.Sid;
> 
> +
> 
>      for (int i = 0; i < ALLOWED_PSIDS_SIZE; i++) {
> 
>          aclSize += sizeof(ACCESS_ALLOWED_ACE) +
> 
>                     GetLengthSid(allowedPsid[i]) -
> 
> @@ -490,11 +519,13 @@ create_pnpipe(char *name)
> 
>      npipe = CreateNamedPipe(name, PIPE_ACCESS_DUPLEX |
> FILE_FLAG_OVERLAPPED,
> 
>                              PIPE_TYPE_MESSAGE | PIPE_READMODE_BYTE | PIPE_WAIT,
> 
>                              64, BUFSIZE, BUFSIZE, 0, &sa);
> 
> +    free(pTokenUsr);
> 
>      free(acl);
> 
>      free(psd);
> 
>      return npipe;
> 
> 
> 
> handle_error:
> 
> +    free(pTokenUsr);
> 
>      free(acl);
> 
>      free(psd);
> 
>      return INVALID_HANDLE_VALUE;
> 
> --
> 
> 2.6.2
> 
> 



More information about the dev mailing list