-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Bitwarden does not react on SIGTERM (v1.13.1 sqlite) #905
Comments
Maybe this is due to rocket doesn't implement clean shutdown until now: |
Indeed the current version of the rocket web server we use doesn't support any kind of graceful shutdow. The new async version in development does have the capabilities to do it and I have a separate branch using it: https://github.com/dani-garcia/bitwarden_rs/tree/async |
Another solution could be to use tiny-init or dumb-init. these are meant as small init replacements and handle signals correctly. See https://github.com/Yelp/dumb-init for a complete explanation. |
@mfriedenhagen no, that would not help. An init "system" like you mentioned would help when a main process forks child process(es) and does not control the lifecycle of the childs. The problem is that the whole |
Hello, when reading https://github.com/Yelp/dumb-init's README it is my understanding that the Linux kernel does not send SIGTERM to PID 1 at all if no explicit handler is implemented. If dumb-init would start bitwarden_rs the standard SIGTERM handler would terminate bitwarden_rs as expected. |
Okay, we can simulate this without building a new container image with podman and docker when starting the container with the option [mkilchhofer@localhost ~]$ docker run --name bw_rs-init --init --rm docker.io/bitwardenrs/server:1.13.1
/--------------------------------------------------------------------\
| Starting Bitwarden_RS |
| Version 1.13.1 |
|--------------------------------------------------------------------|
| This is an *unofficial* Bitwarden implementation, DO NOT use the |
| official channels to report bugs/features, regardless of client. |
| Report URL: https://github.com/dani-garcia/bitwarden_rs/issues/new |
\--------------------------------------------------------------------/
[2021-01-02 11:02:04][bitwarden_rs][INFO] JWT keys don't exist, checking if OpenSSL is available...
OpenSSL 1.1.1d 10 Sep 2019
[2021-01-02 11:02:04][bitwarden_rs][INFO] OpenSSL detected, creating keys...
Generating RSA private key, 2048 bit long modulus (2 primes)
...........................................................................+++++
..............................+++++
e is 65537 (0x010001)
writing RSA key
writing RSA key
[2021-01-02 11:02:05][bitwarden_rs][INFO] Keys created correctly.
Running migration 20180114171611
(...)
Running migration 20191117011009
[2021-01-02 11:02:05][start][INFO] Rocket has launched from http://0.0.0.0:80
^C[mkilchhofer@localhost ~]$
[mkilchhofer@localhost ~]$ As you can see, when we Press CTRL+C the container stops. [mkilchhofer@localhost ~]$ docker exec -ti bw_rs-init bash
root@a05b79e3eeb0:/# apt update && apt install -y procps
root@a05b79e3eeb0:/# ps -ef
UID PID PPID C STIME TTY TIME CMD
root 1 0 0 11:10 ? 00:00:00 /sbin/docker-init -- /bitwarden_rs
root 6 1 0 11:10 ? 00:00:00 /bitwarden_rs
root 25 0 0 11:11 pts/0 00:00:00 bash
root 357 25 0 11:11 pts/0 00:00:00 ps -ef And without the option, we are unable to stop the container with CTRL+C: [mkilchhofer@localhost ~]$ docker run --name bw_rs --rm docker.io/bitwardenrs/server:1.13.1
/--------------------------------------------------------------------\
| Starting Bitwarden_RS |
| Version 1.13.1 |
|--------------------------------------------------------------------|
| This is an *unofficial* Bitwarden implementation, DO NOT use the |
| official channels to report bugs/features, regardless of client. |
| Report URL: https://github.com/dani-garcia/bitwarden_rs/issues/new |
\--------------------------------------------------------------------/
[2021-01-02 11:02:21][bitwarden_rs][INFO] JWT keys don't exist, checking if OpenSSL is available...
OpenSSL 1.1.1d 10 Sep 2019
[2021-01-02 11:02:21][bitwarden_rs][INFO] OpenSSL detected, creating keys...
Generating RSA private key, 2048 bit long modulus (2 primes)
..................+++++
............................................+++++
e is 65537 (0x010001)
writing RSA key
writing RSA key
[2021-01-02 11:02:21][bitwarden_rs][INFO] Keys created correctly.
Running migration 20180114171611
(...)
Running migration 20191117011009
[2021-01-02 11:02:22][start][INFO] Rocket has launched from http://0.0.0.0:80
^C^C^C^C What we do not know is what's going on inside the bitwarden_rs process. It seems that the framework rocket still does not support graceful shutdown in the mainline version. |
As stated in the comment from @dani-garcia, this is not yet in the master branch. It is only in the async version, which we will probably release shortly once rocket releases there new version as a stable release so that we know there will be no to minimal breaking changes made. Untill then, run bitwarden_rs using |
Also, i see that you are still using a very old version. |
There are some more things I observed, when I terminate a detached bitwarden conainer which [mkilchhofer@localhost ~]$ docker run --name bw_rs-init --init --detach docker.io/bitwardenrs/server:1.13.1
e750910edeb190bb9e5a672d4563b4812bc36fd8fdb7bdd4a5e18c61932b6878
[mkilchhofer@localhost ~]$ docker ps -a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
e750910edeb1 bitwardenrs/server:1.13.1 "/bitwarden_rs" 12 seconds ago Up 12 seconds (health: starting) 80/tcp, 3012/tcp bw_rs-init
[mkilchhofer@localhost ~]$ docker stop bw_rs-init
bw_rs-init
[mkilchhofer@localhost ~]$ docker ps -a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
e750910edeb1 bitwardenrs/server:1.13.1 "/bitwarden_rs" 24 seconds ago Exited (143) 1 second ago bw_rs-init We see here, that the return code is 143. From my java experience it looks like we also need to substract 128 and then we get the real 15 return code which is SIGTERM. @BlackDex yes I know I used an old version in the comment above. I would like to reproduce this with the same version as the issue was created. |
The enhancement would be that simple: diff --git a/docker/amd64/Dockerfile b/docker/amd64/Dockerfile
index 75ee4dd..7edb32b 100644
--- a/docker/amd64/Dockerfile
+++ b/docker/amd64/Dockerfile
@@ -78,6 +78,7 @@ RUN apt-get update && apt-get install -y \
openssl \
ca-certificates \
curl \
+ dumb-init \
sqlite3 \
libmariadb-dev-compat \
libpq5 \
@@ -101,5 +102,6 @@ HEALTHCHECK --interval=60s --timeout=10s CMD ["/healthcheck.sh"]
# Configures the startup!
WORKDIR /
+ENTRYPOINT ["/usr/bin/dumb-init", "--"]
CMD ["/start.sh"]
diff --git a/docker/amd64/Dockerfile.alpine b/docker/amd64/Dockerfile.alpine
index 3ede25b..675c28b 100644
--- a/docker/amd64/Dockerfile.alpine
+++ b/docker/amd64/Dockerfile.alpine
@@ -74,6 +74,7 @@ ENV SSL_CERT_DIR=/etc/ssl/certs
RUN apk add --no-cache \
openssl \
curl \
+ dumb-init \
sqlite \
postgresql-libs \
ca-certificates
@@ -96,5 +97,6 @@ HEALTHCHECK --interval=60s --timeout=10s CMD ["/healthcheck.sh"]
# Configures the startup!
WORKDIR /
+ENTRYPOINT ["/usr/bin/dumb-init", "--"]
CMD ["/start.sh"]
|
This issue is solved now for all images. |
Subject of the issue
At least the sqlite flavor of bitwarden_rs does not react on SIGTERM. I already wondered why evicting kubernetes nodes takes longer when bitwarden_rs runs on this node.
I now determined that the binary
/bitwarden_rs
inside the sqlite flavored docker image does not react on a SIGTERM signal.Your environment
Steps to reproduce / Actual behaviour
Terminate the docker Image takes long and the exist status it not 0:
Another attempt to check if the binary handles SIGTERM:
podman start bitwardenrs
)podman exec -ti bitwardenrs bash
)procps
packageroot@153e82e8be39:/# apt update && apt install -y procps
Nothing happens, I expect, that bitwarden starts to teardown.
Expected behaviour
Bitwarden_rs binary implements a signal handler and handles SIGTERM correctly
Relevant logs
The text was updated successfully, but these errors were encountered: