Thank you Ning and Anand.
Applied on master!
Alin.
> -----Original Message-----
> From: Anand Kumar <kumaran...@vmware.com>
> Sent: Friday, January 24, 2020 7:22 AM
> To: Ning Wu <n...@vmware.com>; Alin Serdean
> <aserd...@cloudbasesolutions.com>; d...@openvswitch.org
> Cc: Lina Li <lin...@vmware.com>; Roy Luo <luo...@vmware.com>
> Subject: Re: [PATCH]lib/stream-windows.c: Grant Access Privilege of Named
> Pipe to Creator
>
> Acked-by: Anand Kumar <kumaran...@vmware.com>
>
> Thanks,
> Anand Kumar
>
> On 1/22/20, 12:19 AM, "Ning Wu" <n...@vmware.com> wrote:
>
> From e42950665acee9aab941b26ebdd067ca0de908a3 Mon Sep 17 00:00:00
> 2001
> From: Ning Wu <n...@vmware.com>
> Date: Tue, 21 Jan 2020 23:46:58 -0800
> Subject: [PATCH]lib/stream-windows.c: 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 <n...@vmware.com>
> ---
> Documentation/ref/ovsdb.7.rst | 3 ++-
> lib/stream-windows.c | 33 ++++++++++++++++++++++++++++++++-
> 2 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ref/ovsdb.7.rst b/Documentation/ref/ovsdb.7.rst
> index b1f3f5d..da4dbed 100644
> --- a/Documentation/ref/ovsdb.7.rst
> +++ b/Documentation/ref/ovsdb.7.rst
> @@ -422,7 +422,8 @@ punix:<file>
> named <file>.
>
> On Windows, listens on a local named pipe, creating a named pipe
> - <file> to mimic the behavior of a Unix domain socket.
> + <file> to mimic the behavior of a Unix domain socket. The ACLs of the
> named
> + pipe include LocalSystem, Administrators, and Creator Owner.
>
> All IP-based connection methods accept IPv4 and IPv6 addresses. To
> specify
> an
> IPv6 address, wrap it in square brackets, e.g. ``ssl:[::1]:6640``.
> Passive
> diff --git a/lib/stream-windows.c b/lib/stream-windows.c
> index 34bc610..5c4c55e 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\\"
>
> /* 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
>
> -----Original Message-----
> From: Alin Serdean <aserd...@cloudbasesolutions.com>
> Sent: Wednesday, January 22, 2020 12:19
> To: Ning Wu <n...@vmware.com>; d...@openvswitch.org; Anand Kumar
> <kumaran...@vmware.com>
> Cc: Lina Li <lin...@vmware.com>; Roy Luo <luo...@vmware.com>
> Subject: RE: [PATCH] Grant Access Privilege of Named Pipe to Creator
>
> 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:
>
> https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdocs.ope
> nvswitch.org%2Fen%2Flatest%2Finternals%2Fcontributing%2Fsubmitting-
> patches%2F%23email-
> subject&data=02%7C01%7Cnwu%40vmware.com%7Cf87a8fa19a3d43832
> 5d008d79ef2400c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637
> 152635616314521&sdata=h73zweLknUd2m9ReaYEe94QNR4wj3q5TilBV8t
> Qmjiw%3D&reserved=0
>
> Thanks,
> Alin.
>
> > -----Original Message-----
> > From: Ning Wu <n...@vmware.com>
> > Sent: Monday, January 20, 2020 4:16 AM
> > To: d...@openvswitch.org; Alin Serdean
> > <aserd...@cloudbasesolutions.com>
> > Cc: Lina Li <lin...@vmware.com>; Roy Luo <luo...@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: d...@openvswitch.org; Alin Serdean
> > <aserd...@cloudbasesolutions.com>
> > Cc: Lina Li <lin...@vmware.com>; Roy Luo <luo...@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 <n...@vmware.com <mailto:n...@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
> >
> >
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
