Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

app: verify a unix socket path length in socket.tcp_server() #4634

Closed
Totktonada opened this issue Nov 20, 2019 · 0 comments
Closed

app: verify a unix socket path length in socket.tcp_server() #4634

Totktonada opened this issue Nov 20, 2019 · 0 comments
Assignees
Labels
app bug Something isn't working good first issue Good for newcomers tarantoolctl
Milestone

Comments

@Totktonada
Copy link
Member

Tarantool version: 2.3.0-225-g8d363c439.
OS version: Linux (but should be applicable for other OSes).

tarantooctl start app can successfully start an application and report incorrect console listening socket when a path exceeds UNIX_PATH_MAX (it is 108 on Linux, 104 on Mac OS). After that tarantoolctl status / stop / enter will not work properly, because actual file name of a socket will be truncated to UNIX_PATH_MAX bytes, while commands like status will try to connect using a non-truncated path.

The cause of this is socket.tcp_server(), which does not report truncation of a path as an error. It is easy to fix:

diff --git a/src/lua/socket.c b/src/lua/socket.c
index 130378caf..022dd56bb 100644
--- a/src/lua/socket.c
+++ b/src/lua/socket.c
@@ -403,7 +403,13 @@ lbox_socket_local_resolve(const char *host, const char *port,
 {
        if (strcmp(host, "unix/") == 0) {
                struct sockaddr_un *uaddr = (struct sockaddr_un *) addr;
-               if (*socklen < sizeof(*uaddr)) {
+               /*
+                * sun_path doesn't have to be null-terminated on
+                * Linux, but we ensure it is so for maximum
+                * portability.
+                */
+               if (*socklen < sizeof(*uaddr) ||
+                   strlen(port) >= sizeof(uaddr->sun_path)) {
                        errno = ENOBUFS;
                        return -1;
                }

(Yep, I know, we can use the last byte too on Linux (left sun_path non-null-terminated), but I'm afraid to do it on Mac OS: see here and here.)

However we can do some tricks: say, chdir into a directory and fill sun_path with just name w/o a full path. It works at least on Linux. This looks weird, however will be helpful in some cases.

I met this problem when tarantoolctl start <...> was run under a java testing harness, which was run in an unusual directory when maven performs testing of a freshly created release: maven checks out a tagged revision into a deeper directory and perform all actions there. I meant that a user not always control a run directory (where console sockets are created) when tarantoolctl is used as base for some other tool (BTW, our test-run tool uses it too, but just give a warning when a unix socket path exceeds 107 symbols). So I think hacking around this limit can safe time for many users, despite that it may look bad in the code. Personally I was hit by this limit many times.

However at least a proper error would be good.

@Totktonada Totktonada added bug Something isn't working tarantoolctl good first issue Good for newcomers app labels Nov 20, 2019
@kyukhin kyukhin added this to the 2.3.2 milestone Jan 16, 2020
slumber added a commit that referenced this issue Feb 21, 2020
Providing socket pathname longer than UNIX_PATH_MAX to socket.tcp_server()
will not cause any error, lbox_socket_local_resolve will just truncate the
name according to the limit, causing bad behavior (tarantool will try to
access a socket, which doesn't exist). Thus, let's verify, that pathname
can fit into buffer.

Closes #4634
slumber added a commit that referenced this issue Feb 21, 2020
Providing socket pathname longer than UNIX_PATH_MAX to socket.tcp_server()
will not cause any error, lbox_socket_local_resolve will just truncate the
name according to the limit, causing bad behavior (tarantool will try to
access a socket, which doesn't exist). Thus, let's verify, that pathname
can fit into buffer.

Closes #4634
@slumber slumber self-assigned this Feb 22, 2020
slumber added a commit that referenced this issue Feb 22, 2020
Providing socket pathname longer than UNIX_PATH_MAX to socket.tcp_server()
will not cause any error, lbox_socket_local_resolve will just truncate the
name according to the limit, causing bad behavior (tarantool will try to
access a socket, which doesn't exist). Thus, let's verify, that pathname
can fit into buffer.

Closes #4634
kyukhin pushed a commit that referenced this issue Feb 24, 2020
Providing socket pathname longer than UNIX_PATH_MAX to socket.tcp_server()
will not cause any error, lbox_socket_local_resolve will just truncate the
name according to the limit, causing bad behavior (tarantool will try to
access a socket, which doesn't exist). Thus, let's verify, that pathname
can fit into buffer.

Closes #4634

(cherry picked from commit aae9351)
avtikhon added a commit that referenced this issue Mar 25, 2020
Fixed OSX host setup for Tarantool build:
- set brew installation from Homebrew repository instructions;
- set in use Python 2 latest commit from tapped local formula,
  since Python 2 is EOL, also removed extra pip installation with
  get-pip script, because tapped formula installs pip itself.
  python@2 was deleted from homebrew/core in commit 028f11f9e:
    python@2: delete (Homebrew/homebrew-core#49796)
    EOL 1 January 2020.
  Tapped formula created from the latest formula before its removal:
    git -C "$(brew --repo homebrew/core)" show 028f11f9e^:Formula/python@2.rb
- added upgrade packages call to avoid of fails on already
  installed packages, but with previous version;
- fixed the gitlab-ci configuration for sudo on testing hosts and removed pip
  option '--user' to avoid of use the users paths with special setup for it.

Fixed OSX host setup for Tarantool test:
- set maximum processes limit value to 2500 for testing process;
- new Mac machines are going to be added into CI and usernames on them
  are long according to internal policies. It makes a home directory to
  be long too and so a path to a unix socket created during testing can
  be longer then UNIX_PATH_MAX=108 constant which is described as issue
    #4634
  To avoid of it the short working directory for testing set by option:
    --vardir /tmp/tnt
avtikhon added a commit that referenced this issue Mar 26, 2020
Fixed OSX host setup for Tarantool build:
- set brew installation from Homebrew repository instructions;
- set in use Python 2 latest commit from tapped local formula,
  since Python 2 is EOL, also removed extra pip installation with
  get-pip script, because tapped formula installs pip itself.
  python@2 was deleted from homebrew/core in commit 028f11f9e:
    python@2: delete (Homebrew/homebrew-core#49796)
    EOL 1 January 2020.
  Tapped formula created from the latest formula before its removal:
    git -C "$(brew --repo homebrew/core)" show 028f11f9e^:Formula/python@2.rb
- added upgrade packages call to avoid of fails on already
  installed packages, but with previous version;
- fixed the gitlab-ci configuration for sudo on testing hosts and removed pip
  option '--user' to avoid of use the users paths with special setup for it.

Fixed OSX host setup for Tarantool test:
- set maximum processes limit value to 2500 for testing process;
- new Mac machines are going to be added into CI and usernames on them
  are long according to internal policies. It makes a home directory to
  be long too and so a path to a unix socket created during testing can
  be longer then UNIX_PATH_MAX=108 constant which is described as issue
    #4634
  To avoid of it the short working directory for testing set by option:
    --vardir /tmp/tnt
kyukhin pushed a commit that referenced this issue Mar 26, 2020
Fixed OSX host setup for Tarantool build:
- set brew installation from Homebrew repository instructions;
- set in use Python 2 latest commit from tapped local formula,
  since Python 2 is EOL, also removed extra pip installation with
  get-pip script, because tapped formula installs pip itself.
  python@2 was deleted from homebrew/core in commit 028f11f9e:
    python@2: delete (Homebrew/homebrew-core#49796)
    EOL 1 January 2020.
  Tapped formula created from the latest formula before its removal:
    git -C "$(brew --repo homebrew/core)" show 028f11f9e^:Formula/python@2.rb
- added upgrade packages call to avoid of fails on already
  installed packages, but with previous version;
- fixed the gitlab-ci configuration for sudo on testing hosts and removed pip
  option '--user' to avoid of use the users paths with special setup for it.

Fixed OSX host setup for Tarantool test:
- set maximum processes limit value to 2500 for testing process;
- new Mac machines are going to be added into CI and usernames on them
  are long according to internal policies. It makes a home directory to
  be long too and so a path to a unix socket created during testing can
  be longer then UNIX_PATH_MAX=108 constant which is described as issue
    #4634
  To avoid of it the short working directory for testing set by option:
    --vardir /tmp/tnt

(cherry picked from commit e4ab057)
kyukhin pushed a commit that referenced this issue Mar 26, 2020
Fixed OSX host setup for Tarantool build:
- set brew installation from Homebrew repository instructions;
- set in use Python 2 latest commit from tapped local formula,
  since Python 2 is EOL, also removed extra pip installation with
  get-pip script, because tapped formula installs pip itself.
  python@2 was deleted from homebrew/core in commit 028f11f9e:
    python@2: delete (Homebrew/homebrew-core#49796)
    EOL 1 January 2020.
  Tapped formula created from the latest formula before its removal:
    git -C "$(brew --repo homebrew/core)" show 028f11f9e^:Formula/python@2.rb
- added upgrade packages call to avoid of fails on already
  installed packages, but with previous version;
- fixed the gitlab-ci configuration for sudo on testing hosts and removed pip
  option '--user' to avoid of use the users paths with special setup for it.

Fixed OSX host setup for Tarantool test:
- set maximum processes limit value to 2500 for testing process;
- new Mac machines are going to be added into CI and usernames on them
  are long according to internal policies. It makes a home directory to
  be long too and so a path to a unix socket created during testing can
  be longer then UNIX_PATH_MAX=108 constant which is described as issue
    #4634
  To avoid of it the short working directory for testing set by option:
    --vardir /tmp/tnt

(cherry picked from commit e4ab057)
kyukhin pushed a commit that referenced this issue Mar 26, 2020
Fixed OSX host setup for Tarantool build:
- set brew installation from Homebrew repository instructions;
- set in use Python 2 latest commit from tapped local formula,
  since Python 2 is EOL, also removed extra pip installation with
  get-pip script, because tapped formula installs pip itself.
  python@2 was deleted from homebrew/core in commit 028f11f9e:
    python@2: delete (Homebrew/homebrew-core#49796)
    EOL 1 January 2020.
  Tapped formula created from the latest formula before its removal:
    git -C "$(brew --repo homebrew/core)" show 028f11f9e^:Formula/python@2.rb
- added upgrade packages call to avoid of fails on already
  installed packages, but with previous version;
- fixed the gitlab-ci configuration for sudo on testing hosts and removed pip
  option '--user' to avoid of use the users paths with special setup for it.

Fixed OSX host setup for Tarantool test:
- set maximum processes limit value to 2500 for testing process;
- new Mac machines are going to be added into CI and usernames on them
  are long according to internal policies. It makes a home directory to
  be long too and so a path to a unix socket created during testing can
  be longer then UNIX_PATH_MAX=108 constant which is described as issue
    #4634
  To avoid of it the short working directory for testing set by option:
    --vardir /tmp/tnt
kyukhin pushed a commit that referenced this issue Mar 26, 2020
Fixed OSX host setup for Tarantool build:
- set brew installation from Homebrew repository instructions;
- set in use Python 2 latest commit from tapped local formula,
  since Python 2 is EOL, also removed extra pip installation with
  get-pip script, because tapped formula installs pip itself.
  python@2 was deleted from homebrew/core in commit 028f11f9e:
    python@2: delete (Homebrew/homebrew-core#49796)
    EOL 1 January 2020.
  Tapped formula created from the latest formula before its removal:
    git -C "$(brew --repo homebrew/core)" show 028f11f9e^:Formula/python@2.rb
- added upgrade packages call to avoid of fails on already
  installed packages, but with previous version;
- fixed the gitlab-ci configuration for sudo on testing hosts and removed pip
  option '--user' to avoid of use the users paths with special setup for it.

Fixed OSX host setup for Tarantool test:
- set maximum processes limit value to 2500 for testing process;
- new Mac machines are going to be added into CI and usernames on them
  are long according to internal policies. It makes a home directory to
  be long too and so a path to a unix socket created during testing can
  be longer then UNIX_PATH_MAX=108 constant which is described as issue
    #4634
  To avoid of it the short working directory for testing set by option:
    --vardir /tmp/tnt

(cherry picked from commit e4ab057)
rosik added a commit to tarantool/cartridge that referenced this issue Jul 23, 2020
Error handling of `console.listen()` was changed recently, so we update
the test appropriately.

See: tarantool/tarantool#4634
rosik added a commit to tarantool/cartridge that referenced this issue Jul 24, 2020
Error handling of `console.listen()` was changed recently, so we update
the test appropriately.

See: tarantool/tarantool#4634
rosik added a commit to tarantool/cartridge that referenced this issue Jul 24, 2020
Error handling of `console.listen()` was changed recently, so we update
the test appropriately.

See: tarantool/tarantool#4634
rosik added a commit to tarantool/cartridge that referenced this issue Jul 24, 2020
Error handling of `console.listen()` was changed recently, so we update
the test appropriately.

See: tarantool/tarantool#4634
Steap2448 pushed a commit to tarantool/cartridge that referenced this issue Jul 27, 2020
Error handling of `console.listen()` was changed recently, so we update
the test appropriately.

See: tarantool/tarantool#4634
rosik added a commit to tarantool/cartridge that referenced this issue Aug 10, 2020
Error handling of `console.listen()` was changed recently, so we update
the test appropriately.

See: tarantool/tarantool#4634
tsafin pushed a commit to tarantool/homebrew-tap that referenced this issue Nov 7, 2020
Fixed OSX host setup for Tarantool build:
- set brew installation from Homebrew repository instructions;
- set in use Python 2 latest commit from tapped local formula,
  since Python 2 is EOL, also removed extra pip installation with
  get-pip script, because tapped formula installs pip itself.
  python@2 was deleted from homebrew/core in commit 028f11f9e:
    python@2: delete (Homebrew/homebrew-core#49796)
    EOL 1 January 2020.
  Tapped formula created from the latest formula before its removal:
    git -C "$(brew --repo homebrew/core)" show 028f11f9e^:Formula/python@2.rb
- added upgrade packages call to avoid of fails on already
  installed packages, but with previous version;
- fixed the gitlab-ci configuration for sudo on testing hosts and removed pip
  option '--user' to avoid of use the users paths with special setup for it.

Fixed OSX host setup for Tarantool test:
- set maximum processes limit value to 2500 for testing process;
- new Mac machines are going to be added into CI and usernames on them
  are long according to internal policies. It makes a home directory to
  be long too and so a path to a unix socket created during testing can
  be longer then UNIX_PATH_MAX=108 constant which is described as issue
    tarantool/tarantool#4634
  To avoid of it the short working directory for testing set by option:
    --vardir /tmp/tnt
Totktonada added a commit that referenced this issue Aug 19, 2021
First, several facts:

* The maximal unix socket file length is 108 symbols on Linux (in fact,
  107 for tarantool console socket, see #4634).
* We run testing during building of RPM packages.
* When tests are run, they use the build directory for temporary testing
  files and tarantool console unix socket files.
* Test-run uses an instance script name to form a unix socket name for
  tarantool console.
* We plan to change package version format and add .dev suffix for
  transient (non tagged) commits.
* The package version participates in the build directory name, when
  we're building an RPM package.

If we'll enable the new version format just now, we'll unable to listen
on the unix socket like the following (the length is 111):

```
/build/usr/src/debug/tarantool-2.9.0.353.dev/test/var/023_box/gh-3633-simple-tuple-size-increasing.socket-admin
```

So I changed the instance file name to make it shorter.

Of course, there are ways to solve the problem at all, this commit is
just workaround.

Part of #6184
Follows up #3633
Totktonada added a commit that referenced this issue Aug 20, 2021
First, several facts:

* The maximal unix socket file length is 108 symbols on Linux (in fact,
  107 for tarantool console socket, see #4634).
* We run testing during building of RPM packages.
* When tests are run, they use the build directory for temporary testing
  files and tarantool console unix socket files.
* Test-run uses an instance script name to form a unix socket name for
  tarantool console.
* We plan to change package version format and add .dev suffix for
  transient (non tagged) commits.
* The package version participates in the build directory name, when
  we're building an RPM package.

If we'll enable the new version format just now, we'll unable to listen
on the unix socket like the following (the length is 111):

```
/build/usr/src/debug/tarantool-2.9.0.353.dev/test/var/023_box/gh-3633-simple-tuple-size-increasing.socket-admin
```

So I changed the instance file name to make it shorter.

Of course, there are ways to solve the problem at all, this commit is
just workaround.

Part of #6184
Follows up #3633
kyukhin pushed a commit that referenced this issue Aug 20, 2021
First, several facts:

* The maximal unix socket file length is 108 symbols on Linux (in fact,
  107 for tarantool console socket, see #4634).
* We run testing during building of RPM packages.
* When tests are run, they use the build directory for temporary testing
  files and tarantool console unix socket files.
* Test-run uses an instance script name to form a unix socket name for
  tarantool console.
* We plan to change package version format and add .dev suffix for
  transient (non tagged) commits.
* The package version participates in the build directory name, when
  we're building an RPM package.

If we'll enable the new version format just now, we'll unable to listen
on the unix socket like the following (the length is 111):

```
/build/usr/src/debug/tarantool-2.9.0.353.dev/test/var/023_box/gh-3633-simple-tuple-size-increasing.socket-admin
```

So I changed the instance file name to make it shorter.

Of course, there are ways to solve the problem at all, this commit is
just workaround.

Part of #6184
Follows up #3633
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app bug Something isn't working good first issue Good for newcomers tarantoolctl
Projects
None yet
Development

No branches or pull requests

3 participants