[ovs-dev] [PATCH 1/3] Windows: Local named pipe implementation

Paul Boca pboca at cloudbasesolutions.com
Tue Jul 12 19:29:15 UTC 2016


Hi Alin!

As discussed offline, please see my comments inline.

Thanks,
Paul

> -----Original Message-----
> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Alin Serdean
> Sent: Tuesday, July 12, 2016 6:55 PM
> To: dev at openvswitch.org
> Subject: [ovs-dev] [PATCH 1/3] Windows: Local named pipe implementation
> 
> Currently in the case of command line arguments punix/unix, on Windows
> we create a file, write a TCP port number to connect. This is a security
> concern.
> 
> This patch adds support for the command line arguments punix/unix trying
> to mimic AF_UNIX behind a local named pipe.
> 
> Signed-off-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
> ---
>  lib/stream-windows.c | 559
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 559 insertions(+)
>  create mode 100644 lib/stream-windows.c
> 
> diff --git a/lib/stream-windows.c b/lib/stream-windows.c
> new file mode 100644
> index 0000000..3b242bd
> --- /dev/null
> +++ b/lib/stream-windows.c
> @@ -0,0 +1,559 @@
> +/*
> + * Copyright (c) 2016 Cloudbase Solutions Srl
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include <errno.h>
> +#include <inttypes.h>
> +#include <netdb.h>
> +#include <poll.h>
> +#include <sys/socket.h>
> +#include <sys/types.h>
> +#include <sys/un.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <strsafe.h>
> +#include <unistd.h>
> +#include "packets.h"
> +#include "poll-loop.h"
> +#include "socket-util.h"
> +#include "dirs.h"
> +#include "util.h"
> +#include "stream-provider.h"
> +#include "openvswitch/vlog.h"
> +
> +VLOG_DEFINE_THIS_MODULE(stream_windows);
> +
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 25);
> +
> +static void maybe_unlink_and_free(char *path);
> +
> +/* Buffer size suggested at the creation of the named pipe for reading and
> + * and writing operations */
> +#define BUFSIZE 65000
> +
> +/* Default local prefix of a named pipe */
> +#define LOCAL_PREFIX "\\\\.\\pipe\\"
> +
> +/* This function has the purpose to remove all the slashes received in s */
> +char*
> +remove_slashes(char *s) {
> +    char *p1, *p2;
> +    p1 = p2 = s;
> +
> +    while (*p1) {
> +        if ((*p1) == '\\' || (*p1) == '/') {
> +            p1++;
> +        } else {
> +            *p2 = *p1;
> +            p2++;
> +            p1++;
> +        }
> +    }
> +    *p2 = '\0';
> +    return s;
> +}
> +
> +/* Active named pipe descriptor stream. */
> +struct windows_stream
> +{
> +    struct stream stream;
> +    HANDLE fd;
> +    /* Overlapped operations used for reading/writing */
> +    OVERLAPPED read;
> +    OVERLAPPED write;
> +    /* Flag to check if a reading/writing is pending*/
> +    bool read_pending;
> +    bool send_pending;
> +    /* Flag to check if fd is a server HANDLE. In the case of a server handle
> +     * we have to issue a disconnect before closing the actual handle */
> +    bool server;
> +};
> +
> +static struct windows_stream *
> +stream_windows_cast(struct stream *stream)
> +{
> +    stream_assert_class(stream, &windows_stream_class);
> +    return CONTAINER_OF(stream, struct windows_stream, stream);
> +}
> +
> +/* Active named pipe open */
> +static int
> +windows_open(const char *name, char *suffix, struct stream **streamp,
> +             uint8_t dscp OVS_UNUSED)
> +{
> +    char *connect_path;
> +    HANDLE npipe;
> +    DWORD mode;
> +    char* path;
> +    FILE* file;
> +    short retries = 0;
> +    /* If the path does not contain a ':', assume it is relative to
> +     * OVS_RUNDIR. */
> +    if (!strchr(suffix, ':')) {
> +        path = xasprintf("%s/%s", ovs_rundir(), suffix);
> +    } else {
> +        path = xstrdup(suffix);
> +    }
> +
> +    /* In the punix/unix argument the assumption is that there is a file
> +     * created in the path (name) */
> +    file = fopen(path, "r");
> +    if (!file) {
> +        free(path);
> +        VLOG_DBG_RL(&rl, "%s: could not open %s (%s)", name, suffix,
> +                    ovs_strerror(errno));
> +        return errno;
> +    } else {
> +        fclose(file);
> +    }
> +
> +    /* Valid pipe names do not have slashes. The assumption is the named
> pipe
> +     * was created on the same path with the slash stripped path and the
> +     * default prefix \\.\pipe\ appended.
> +     * Strip the slashed from the parameter name and append the default
> prefix
> +     */
> +    connect_path = xasprintf("%s%s", LOCAL_PREFIX, remove_slashes(path));
> +    free(path);
> +
> +    /* Try to connect to the named pipe. In the case all pipe instances are
> +     * busy wait for the default timeout to pass and try again. We will try
> +     * the latter for 3 times */
> +    while (retries < 3) {
> +        /* Use overlapped flag and no buffering to ensure an asynchronous
> +         * operations */
> +        npipe = CreateFile(connect_path, GENERIC_READ | GENERIC_WRITE, 0,
> NULL,
> +                           OPEN_EXISTING,
> +                           FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OVERLAPPED |
> +                           FILE_FLAG_NO_BUFFERING,
> +                           NULL);
> +        if (npipe != INVALID_HANDLE_VALUE) {
> +            break;
> +        }
> +        if (GetLastError() != ERROR_PIPE_BUSY) {
> +            break;
> +        }
> +        WaitNamedPipe(connect_path, NMPWAIT_USE_DEFAULT_WAIT);
[Paul Boca] Maybe we could use only WaitNamedPipe on open, in order to check
if the pipe exists or not, without the 3 retries and move the CreateFile into connect
function. This way we would mimic the behaviour of the sockets implementation on this
without the need of the retries.
If it doesn't just return an error otherwise return success.

> +        retries++;
> +        VLOG_DBG_RL(&rl, "All pipes are currently busy waiting for one to"
> +                    "become available");
> +    }
> +
> +    if (npipe == INVALID_HANDLE_VALUE) {
> +        VLOG_DBG_RL(&rl, "Could not connect to named pipe: %s",
> +                    ovs_lasterror_to_string());
> +        free(connect_path);
> +        return ENOENT;
> +    }
> +    mode = PIPE_READMODE_BYTE;
> +    if (!SetNamedPipeHandleState(npipe, &mode, NULL, NULL)) {
> +        VLOG_DBG_RL(&rl, "Could not set named pipe options: %s",
> +                    ovs_lasterror_to_string());
> +        free(connect_path);
> +        CloseHandle(npipe);
> +        return ENOENT;
> +    }
> +    struct windows_stream *s = xmalloc(sizeof *s);
> +    stream_init(&s->stream, &windows_stream_class, 0, name);
> +    s->fd = npipe;
> +    /* This is a active stream */
> +    s->server = false;
> +    /* Create events for reading and writing to be signaled later */
> +    memset(&s->read, 0, sizeof(s->read));
> +    memset(&s->write, 0, sizeof(s->write));
> +    s->read.hEvent = CreateEvent(NULL, TRUE, TRUE, NULL);
> +    s->write.hEvent = CreateEvent(NULL, TRUE, TRUE, NULL);
> +    /* Initial read and write operations are not pending */
> +    s->read_pending = false;
> +    s->send_pending = false;
> +    *streamp = &s->stream;
> +    return 0;
> +}
> +
> +/* Active named pipe close */
> +static void
> +windows_close(struct stream *stream)
> +{
> +    struct windows_stream *s = stream_windows_cast(stream);
> +    /* Disconnect the named pipe in case it was a passive stream at first */
> +    if (s->server) {
> +        DisconnectNamedPipe(s->fd);
> +    }
> +    CloseHandle(s->fd);
> +    free(s);
> +}
> +
> +/* Active named pipe connect */
> +static int
> +windows_connect(struct stream *stream)
> +{
> +    /* Stub function for connect. We already to the connect in the open
> +     * function */
> +    return 0;
> +}
> +
> +/* Active named pipe receive */
> +static ssize_t
> +windows_recv(struct stream *stream, void *buffer, size_t n)
> +{
> +    struct windows_stream *s = stream_windows_cast(stream);
> +    ssize_t retval = 0;
> +    boolean result = false;
> +    DWORD last_error = 0;
> +    LPOVERLAPPED  ov = NULL;
> +    ov = &s->read;
> +
> +    /* If the read operation was pending we verify its result */
> +    if (s->read_pending) {
> +        if (!GetOverlappedResult(s->fd, ov, &(DWORD)retval, FALSE)) {
> +            last_error = GetLastError();
> +            if (last_error == ERROR_IO_INCOMPLETE) {
> +                /* If the operation is still pending retry again */
> +                s->read_pending = true;
> +                return -EAGAIN;
> +            } else if (last_error == ERROR_PIPE_NOT_CONNECTED ||
> +                       last_error == ERROR_BAD_PIPE ||
> +                       last_error == ERROR_NO_DATA ||
> +                       last_error == ERROR_BROKEN_PIPE) {
> +                /* If the pipe was disconnected return 0 */
> +                return 0;
> +            } else {
> +                VLOG_DBG_RL(&rl, "Could not receive data on named pipe. Last "
> +                            "error: %s", ovs_lasterror_to_string());
> +                return -EINVAL;
> +            }
> +        }
> +        s->read_pending = false;
> +        return retval;
> +    }
> +
> +    result = ReadFile(s->fd, buffer, n, &(DWORD)retval, ov);
> +
> +    if (!result && GetLastError() == ERROR_IO_PENDING) {
> +        /* Pend the read operation */
> +        s->read_pending = true;
> +        return -EAGAIN;
> +    } else if (!result) {
> +        last_error = GetLastError();
> +        if (last_error == ERROR_PIPE_NOT_CONNECTED ||
> +            last_error == ERROR_BAD_PIPE ||
> +            last_error == ERROR_NO_DATA ||
> +            last_error == ERROR_BROKEN_PIPE) {
> +            /* If the pipe was disconnected return 0 */
> +            return 0;
> +        }
> +        VLOG_DBG_RL(&rl, "Could not receive data synchronous on named
> pipe."
> +                    "Last error: %s", ovs_lasterror_to_string());
> +        return -EINVAL;
> +    }
> +
> +    return retval;
> +}
> +
> +/* Active named pipe send */
> +static ssize_t
> +windows_send(struct stream *stream, const void *buffer, size_t n)
> +{
> +    struct windows_stream *s = stream_windows_cast(stream);
> +    ssize_t retval = 0;
> +    boolean result = false;
> +    DWORD last_error = 0;
> +    LPOVERLAPPED  ov = NULL;
> +    ov = &s->write;
> +
> +    /* If the send operation was pending we verify the result */
> +    if (s->send_pending) {
> +        if (!GetOverlappedResult(s->fd, ov, &(DWORD)retval, FALSE)) {
> +            last_error = GetLastError();
> +            if (last_error == ERROR_IO_INCOMPLETE) {
> +                /* If the operation is still pending retry again */
> +                s->send_pending = true;
> +                return -EAGAIN;
> +            } else if (last_error == ERROR_PIPE_NOT_CONNECTED ||
> +                       last_error == ERROR_BAD_PIPE ||
> +                       last_error == ERROR_NO_DATA ||
> +                       last_error == ERROR_BROKEN_PIPE) {
> +                /* If the pipe was disconnected return connection reset */
> +                return -WSAECONNRESET;
> +            } else {
> +                VLOG_DBG_RL(&rl, "Could not send data on named pipe. Last "
> +                            "error: %s", ovs_lasterror_to_string());
> +                return -EINVAL;
> +            }
> +        }
> +        s->send_pending = false;
> +        return retval;
> +    }
> +
> +    result = WriteFile(s->fd, buffer, n, &(DWORD)retval, ov);
> +    last_error = GetLastError();
> +    if (!result && GetLastError() == ERROR_IO_PENDING) {
> +        /* Pend the send operation */
> +        s->send_pending = true;
> +        return -EAGAIN;
> +    } else if (!result && (last_error == ERROR_PIPE_NOT_CONNECTED ||
> +                           last_error == ERROR_BAD_PIPE ||
> +                           last_error == ERROR_NO_DATA ||
> +                           last_error == ERROR_BROKEN_PIPE)) {
> +        /* If the pipe was disconnected return connection reset */
> +        return -WSAECONNRESET;
> +    } else if (!result) {
> +        VLOG_DBG_RL(&rl, "Could not send data on synchronous named pipe.
> Last "
> +                    "error: %s", ovs_lasterror_to_string());
> +        return -EINVAL;
> +    }
> +    return (retval > 0 ? retval : -EAGAIN);
> +}
> +
> +/* Active named pipe wait */
> +static void
> +windows_wait(struct stream *stream, enum stream_wait_type wait)
> +{
> +    struct windows_stream *s = stream_windows_cast(stream);
> +    switch (wait) {
> +    case STREAM_SEND:
> +        poll_wevent_wait(s->write.hEvent);
> +        break;
> +
> +    case STREAM_CONNECT:
> +    case STREAM_RECV:
> +        poll_wevent_wait(s->read.hEvent);
> +        break;
> +
> +    default:
> +        OVS_NOT_REACHED();
> +    }
> +}
> +
> +/* Passive named pipe descriptor stream. */
> +const struct stream_class windows_stream_class = {
> +    "unix",                     /* name */
> +    false,                      /* needs_probes */
> +    windows_open,               /* open */
> +    windows_close,              /* close */
> +    windows_connect,            /* connect */
> +    windows_recv,               /* recv */
> +    windows_send,               /* send */
> +    NULL,                       /* run */
> +    NULL,                       /* run_wait */
> +    windows_wait,               /* wait */
> +};
> +
> +struct pwindows_pstream
> +{
> +    struct pstream pstream;
> +    HANDLE fd;
> +    /* Unlink path to be deleted during close */
> +    char* unlink_path;
> +    /* Overlapped operation used for connect */
> +    OVERLAPPED connect;
> +    /* Flag to check if an operation is pending */
> +    bool pending;
> +    /* Named pipe path used for creation */
> +    char* pipe_path;
> +};
> +
> +const struct pstream_class pwindows_pstream_class;
> +
> +static struct pwindows_pstream *
> +pwindows_pstream_cast(struct pstream *pstream)
> +{
> +    pstream_assert_class(pstream, &pwindows_pstream_class);
> +    return CONTAINER_OF(pstream, struct pwindows_pstream, pstream);
> +}
> +
> +/* Create a named pipe with read/write access, overlapped, message mode
> for
> + * writing, byte mode for reading with a maximum of 64 active instances */
> +HANDLE
> +create_npipe(char *name)
> +{
> +    SECURITY_ATTRIBUTES sa;
> +    sa.nLength = sizeof(sa);
> +    sa.lpSecurityDescriptor = NULL;
> +    sa.bInheritHandle = TRUE;
> +    if (strlen(name) > 256) {
> +        VLOG_DBG_RL(&rl, "Named pipe name too long. Creation
> terminated");
> +        return INVALID_HANDLE_VALUE;
> +    }
> +    return CreateNamedPipe(name, PIPE_ACCESS_DUPLEX |
> FILE_FLAG_OVERLAPPED,
> +                           PIPE_TYPE_MESSAGE | PIPE_READMODE_BYTE | PIPE_WAIT,
> +                           64, BUFSIZE, BUFSIZE, 0, &sa);
> +}
> +
> +/* Passive named pipe connect. This function creates a new named pipe
> and
> + * passes the old handle to the active stream */
> +static int
> +pwindows_accept(struct pstream *pstream, struct stream **new_streamp)
> +{
> +    struct pwindows_pstream *p = pwindows_pstream_cast(pstream);
> +    DWORD last_error = 0;
> +    DWORD cbRet;
> +    HANDLE npipe;
> +
> +    /* If the connect operation was pending verify the result */
> +    if (p->pending) {
> +        if (!GetOverlappedResult(p->fd, &p->connect, &cbRet, FALSE)) {
> +            last_error = GetLastError();
> +            if (last_error == ERROR_IO_INCOMPLETE) {
> +                /* If the operation is still pending retry again */
> +                p->pending = true;
> +                return EAGAIN;
> +            } else {
> +                VLOG_DBG_RL(&rl, "Could not connect named pipe. Last "
> +                            "error: %s", ovs_lasterror_to_string());
> +                return EINVAL;
> +            }
> +        }
> +        p->pending = false;
> +    }
> +
> +    if (!p->pending && !ConnectNamedPipe(p->fd, &p->connect)) {
> +        last_error = GetLastError();
> +        if (last_error == ERROR_IO_PENDING) {
> +            /* Pend the connect operation */
> +            p->pending = true;
> +            return EAGAIN;
> +        } else if (last_error != ERROR_PIPE_CONNECTED) {
> +            VLOG_DBG_RL(&rl, "Could not connect synchronous named pipe.
> Last "
> +                        "error: %s", ovs_lasterror_to_string());
> +            return EINVAL;
> +        } else {
> +            /* If the pipe is connected signal an event */
> +            SetEvent(&p->connect.hEvent);
> +        }
> +    }
> +
> +    npipe = create_npipe(p->pipe_path);
> +    if (npipe == INVALID_HANDLE_VALUE) {
> +        VLOG_DBG_RL(&rl, "Could not create a new named pipe after connect.
> ",
> +                    ovs_lasterror_to_string());
> +        return ENOENT;
> +    }
> +
> +    /* Give the handle p->fd to the new created active stream and specify it
> +     * was created by an active stream */
> +    struct windows_stream *p_temp = xmalloc(sizeof *p_temp);
> +    stream_init(&p_temp->stream, &windows_stream_class, 0, "unix");
> +    p_temp->fd = p->fd;
> +    /* Specify it was created by a passive stream */
> +    p_temp->server = true;
> +    /* Create events for read/write operations */
> +    memset(&p_temp->read, 0, sizeof(p_temp->read));
> +    memset(&p_temp->write, 0, sizeof(p_temp->write));
> +    p_temp->read.hEvent = CreateEvent(NULL, TRUE, TRUE, NULL);
> +    p_temp->write.hEvent = CreateEvent(NULL, TRUE, TRUE, NULL);
> +    p_temp->read_pending = false;
> +    p_temp->send_pending = false;
> +    *new_streamp = &p_temp->stream;
> +
> +    /* The passive handle p->fd will be the new created handle */
> +    p->fd = npipe;
> +    memset(&p->connect, 0, sizeof(p->connect));
> +    p->connect.hEvent = CreateEvent(NULL, TRUE, TRUE, NULL);
> +    p->pending = false;
> +    return 0;
> +}
> +
> +/* Passive named pipe close */
> +static void
> +pwindows_close(struct pstream *pstream)
> +{
> +    struct pwindows_pstream *pwin = pwindows_pstream_cast(pstream);
> +    DisconnectNamedPipe(pwin->fd);
> +    CloseHandle(pwin->fd);
[Paul Boca] Also you could call CloseHandle on write.hEvent, read.hEvent and connect.hEvent
in order to avoid handle leaks.

> +    maybe_unlink_and_free(pwin->unlink_path);
> +    free(pwin->pipe_path);
> +    free(pwin);
> +}
> +
> +/* Passive named pipe wait */
> +void
> +pwindows_wait(struct pstream *pstream)
> +{
> +    struct pwindows_pstream *pwin = pwindows_pstream_cast(pstream);
> +    poll_wevent_wait(pwin->connect.hEvent);
> +}
> +
> +/* Passive named pipe */
> +static int
> +pwindows_open(const char *name OVS_UNUSED, char *suffix,
> +              struct pstream **pstreamp, uint8_t dscp OVS_UNUSED)
> +{
> +    char *bind_path;
> +    int error;
> +    HANDLE npipe;
> +    char* orig_path;
> +
> +    char* path;
> +    if (!strchr(suffix, ':')) {
> +        path = xasprintf("%s/%s", ovs_rundir(), suffix);
> +    } else {
> +        path = xstrdup(suffix);
> +    }
> +
> +    /* Try to create a file under the path location */
> +    FILE *file = fopen(path, "w");
> +    if (!file) {
> +        free(path);
> +        error = errno;
> +        VLOG_DBG("could not open %s (%s)", path, ovs_strerror(error));
> +        return error;
> +    } else {
> +        fclose(file);
> +    }
> +
> +    orig_path = xstrdup(path);
> +    /* Strip slashes from path and create a named pipe using that name */
> +    bind_path = xasprintf("%s%s", LOCAL_PREFIX, remove_slashes(path));
> +    free(path);
> +
> +    npipe = create_npipe(bind_path);
> +
> +    if (npipe == INVALID_HANDLE_VALUE) {
> +        VLOG_DBG_RL(&rl, "Could not create named pipe. Last error: %s",
> +                    ovs_lasterror_to_string());
> +        return ENOENT;
> +    }
> +
> +    struct pwindows_pstream *p = xmalloc(sizeof *p);
> +    pstream_init(&p->pstream, &pwindows_pstream_class, name);
> +    p->fd = npipe;
> +    p->unlink_path = orig_path;
> +    memset(&p->connect, 0, sizeof(p->connect));
> +    p->connect.hEvent = CreateEvent(NULL, TRUE, TRUE, NULL);
> +    p->pending = false;
> +    p->pipe_path = bind_path;
> +    *pstreamp = &p->pstream;
> +    return 0;
> +}
> +
> +const struct pstream_class pwindows_pstream_class = {
> +    "punix",
> +    false,                   /* probes */
> +    pwindows_open,           /* open */
> +    pwindows_close,          /* close */
> +    pwindows_accept,         /* accept */
> +    pwindows_wait,           /* wait */
> +};
> +
> +/* Helper functions. */
> +static void
> +maybe_unlink_and_free(char *path)
> +{
> +    if (path) {
> +        fatal_signal_unlink_file_now(path);
> +        free(path);
> +    }
> +}
> --
> 1.9.5.msysgit.0
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev


More information about the dev mailing list