From 265994230c24465f2904ed96d9fd48b79699cfb5 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Tue, 17 Oct 2023 01:30:04 +0200 Subject: [PATCH 01/25] Feat: Redis support for keyshare server and myirmaserver --- README.md | 4 + go.mod | 1 - go.sum | 33 +- internal/sessiontest/redis_test.go | 39 +- irma/cmd/config.go | 36 +- irma/cmd/keyshare-myirma.go | 20 +- irma/cmd/keyshare-server.go | 24 +- irma/cmd/server.go | 33 +- server/conf.go | 100 ++++- server/irmaserver/api.go | 358 ++++++++++-------- server/irmaserver/handle.go | 150 ++++---- server/irmaserver/helpers.go | 190 ++++------ server/irmaserver/sessions.go | 377 +++++++++---------- server/irmaserver/sessions_test.go | 8 +- server/keyshare/keyshareserver/session.go | 3 +- server/keyshare/myirmaserver/server.go | 229 ++++++----- server/keyshare/myirmaserver/session.go | 136 +++++-- server/keyshare/myirmaserver/session_test.go | 47 ++- 18 files changed, 1007 insertions(+), 781 deletions(-) diff --git a/README.md b/README.md index 8b191c017..04e2f7176 100644 --- a/README.md +++ b/README.md @@ -111,6 +111,10 @@ You can then start `irma` with the store-type flag set to Redis and the [default irma server -vv --store-type redis --redis-addr "localhost:6379" --redis-allow-empty-password --redis-no-tls ``` +If you use Redis in Sentinel or Cluster mode for high availability, you need to consider whether you accept the risk of losing session state in case of a failover. Redis does not guarantee [strong consistency](https://redis.io/docs/management/scaling/#redis-cluster-consistency-guarantees) in these setups. We mitigated this by waiting for a write to have reached the master node and at least one replica. This means that at least two replicas should be configured for every master node to achieve high availability. Even then, there is a small chance of losing session state when a replica fails at the same time as the master node. For example, this might be problematic if you want to guarantee that a credential is not issued twice or if you need a session QR to have a long lifetime but you do want the session to be finished soon after the QR is scanned. If you require IRMA sessions to be highly consistent, you should use the default in-memory store or Redis in standalone mode. If you accept this risk, then you can enable Sentinel and Cluster mode support by setting the `--redis-accept-inconsistency-risk` flag. + +Besides the `irma server`, Redis can also be configured for the `irma keyshare server` and the `irma keyshare myirmaserver` in the same way as described above. Note that the `irma keyshare server` does not become stateless when using Redis, because it stores the keyshare commitments and authentication challenges in memory. These cannot be stored in Redis, because we require this data to be strongly consistent. Instead, you can use sticky sessions to make sure that the same user is always routed to the same keyshare server instance. The stored commitments and challenges are only relevant for a few seconds, so the risk of losing this data is low. The `irma keyshare myirmaserver` does become stateless when using Redis. + ## Performance tests This project only includes performance tests for the `irma keyshare server`. These tests can be run using the [k6 load testing tool](https://k6.io/docs/) and need a running keyshare server instance to test against. Instructions on how to run a keyshare server locally can be found [above](#running). diff --git a/go.mod b/go.mod index 11a44113e..e8a09aac0 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,6 @@ toolchain go1.21.1 require ( github.com/alexandrevicenzi/go-sse v1.6.0 github.com/alicebob/miniredis/v2 v2.17.0 - github.com/bsm/redislock v0.7.2 github.com/bwesterb/go-atum v1.1.5 github.com/eknkc/basex v1.0.1 github.com/fxamacker/cbor v1.5.1 diff --git a/go.sum b/go.sum index 149cb85b4..fab14489a 100644 --- a/go.sum +++ b/go.sum @@ -51,8 +51,6 @@ github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a/go.mod h1:SGn github.com/alicebob/miniredis/v2 v2.17.0 h1:EwLdrIS50uczw71Jc7iVSxZluTKj5nfSP8n7ARRnJy0= github.com/alicebob/miniredis/v2 v2.17.0/go.mod h1:gquAfGbzn92jvtrSC69+6zZnwSODVXVpYDRaGhWaL6I= github.com/alvaroloes/enumer v1.1.2/go.mod h1:FxrjvuXoDAx9isTJrv4c+T410zFi0DtXIT0m65DJ+Wo= -github.com/bsm/redislock v0.7.2 h1:jggqOio8JyX9FJBKIfjF3fTxAu/v7zC5mAID9LveqG4= -github.com/bsm/redislock v0.7.2/go.mod h1:kS2g0Yvlymc9Dz8V3iVYAtLAaSVruYbAFdYBDrmC5WU= github.com/bwesterb/byteswriter v1.0.0 h1:xY3MWW1N1jiJ2qlw6/U3YjqyuqNIYu3W7KOCiBbtZp8= github.com/bwesterb/byteswriter v1.0.0/go.mod h1:Gm9TBFNK7ypbrMrWZXBYqX2S1N8mc8DdoHW+Rl002Pc= github.com/bwesterb/go-atum v1.1.5 h1:rqP8fSxOBPh4wv+jfvU0xwbbmE+x2YAbGvt+BpNvqVM= @@ -99,8 +97,6 @@ github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w= github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= github.com/frankban/quicktest v1.14.3 h1:FJKSZTDHjyhriyC81FLQ0LY93eSai0ZyR/ZIkd3ZUKE= github.com/frankban/quicktest v1.14.3/go.mod h1:mgiwOwqx65TmIk1wJ6Q7wvnVMocbUorkibMOrVTHZps= -github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= -github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= github.com/fsnotify/fsnotify v1.5.4 h1:jRbGcIw6P2Meqdwuo0H1p6JVLbL5DHKAKlYndzMwVZI= github.com/fsnotify/fsnotify v1.5.4/go.mod h1:OVB6XrOHzAwXMpEM7uPOzcehqUV2UqJxmVXmkdnm1bU= github.com/fxamacker/cbor v1.5.1 h1:XjQWBgdmQyqimslUh5r4tUGmoqzHmBFQOImkWGi2awg= @@ -116,13 +112,11 @@ github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3Bop github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= -github.com/go-redis/redis/v8 v8.11.4/go.mod h1:2Z2wHZXdQpCDXEGzqMockDpNyYvi2l4Pxt6RJr792+w= github.com/go-redis/redis/v8 v8.11.5 h1:AcZZR7igkdvfVmQTPnu9WE37LRrO/YrBH5zWyjDC0oI= github.com/go-redis/redis/v8 v8.11.5/go.mod h1:gREzHqY1hg6oD9ngVRbLStwAWKhA0FEgq8Jd4h5lpwo= github.com/go-sql-driver/mysql v1.7.0/go.mod h1:OXbVy3sEdcQ2Doequ6Z5BW6fXNQTmx+9S1MCJN5yJMI= github.com/go-sql-driver/mysql v1.7.1 h1:lUIinVbN1DY0xBg0eMOzmmtGoHwWBbvnWubQUrtU8EI= github.com/go-sql-driver/mysql v1.7.1/go.mod h1:OXbVy3sEdcQ2Doequ6Z5BW6fXNQTmx+9S1MCJN5yJMI= -github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE= github.com/golang-jwt/jwt/v4 v4.4.3/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= @@ -155,8 +149,6 @@ github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvq github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QDs8UjoX8= github.com/golang/protobuf v1.4.2/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= github.com/golang/protobuf v1.4.3/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= -github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= -github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= @@ -168,8 +160,6 @@ github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= @@ -212,7 +202,6 @@ github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= -github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= @@ -275,17 +264,10 @@ github.com/multiformats/go-multihash v0.0.11 h1:yEyBxwoR/7vBM5NfLVXRnpQNVLrMhpS6 github.com/multiformats/go-multihash v0.0.11/go.mod h1:LXRDJcYYY+9BjlsFe6i5LV7uekf0OoEJdnRmitUshxk= github.com/nightlyone/lockfile v1.0.0 h1:RHep2cFKK4PonZJDdEl4GmkabuhbsRMgk/k3uAmxBiA= github.com/nightlyone/lockfile v1.0.0/go.mod h1:rywoIealpdNse2r832aiD9jRk8ErCatROs6LzC841CI= -github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= -github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= -github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk= -github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0= github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU= -github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= -github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= -github.com/onsi/gomega v1.16.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= github.com/onsi/gomega v1.18.1 h1:M1GfJqGRrBrrGGsbxzV5dqM2U2ApXefZCQpkukxYRLE= github.com/onsi/gomega v1.18.1/go.mod h1:0q+aL8jAiMXy9hbwj2mr5GziHiwhAIQpFmmtT5hitRs= github.com/pascaldekloe/name v0.0.0-20180628100202-0fd16699aae1/go.mod h1:eD5JxqMiuNYyFNmyY9rkJ/slN8y59oEu4Ei7F8OoKWQ= @@ -401,6 +383,7 @@ golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e h1:+WEEuIdZHnUeJJmEUjyYC2gfUMj69yZXw17EnHg/otA= +golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e/go.mod h1:Kr81I6Kryrl9sr8s2FK3vxD90NdsKWRuOIl2O4CvYbA= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= @@ -428,7 +411,6 @@ golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91 golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= -golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -449,7 +431,6 @@ golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e/go.mod h1:qpuaurCH72eLCgpAm/ golang.org/x/net v0.0.0-20200501053045-e0ff5e5a1de5/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/net v0.0.0-20200506145744-7e3656a0809f/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/net v0.0.0-20200513185701-a91f0712d120/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= -golang.org/x/net v0.0.0-20200520004742-59133d7f0dd7/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/net v0.0.0-20200520182314-0ba52f642ac2/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= @@ -459,7 +440,6 @@ golang.org/x/net v0.0.0-20201031054903-ff519b6c9102/go.mod h1:sp8m0HH+o8qH0wwXwY golang.org/x/net v0.0.0-20201209123823-ac852fbbde11/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= -golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT/9KSuxbyM7479/AVlXFRxuMCk= golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= @@ -489,7 +469,6 @@ golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190204203706-41f3e6584952/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -499,10 +478,7 @@ golang.org/x/sys v0.0.0-20190507160741-ecd444e8653b/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190606165138-5da285871e9c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190624142023-c5567b49c5d0/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190726091711-fc99dfbffb4e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191228213918-04cbcbbfeed8/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200113162924-86b910548bc1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -525,7 +501,6 @@ golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201201145000-ef89a241ccb3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210104204734-6f8348627aad/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210225134936-a50acf3fe073/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -613,7 +588,6 @@ golang.org/x/tools v0.0.0-20200904185747-39188db58858/go.mod h1:Cj7w3i3Rnn0Xh82u golang.org/x/tools v0.0.0-20201110124207-079ba7bd75cd/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20201201161351-ac6f37ff4c2a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20201208233053-a543418bbed2/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= -golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210105154028-b0ab187a4818/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210108195828-e2f9c7f1fc8e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= @@ -711,14 +685,11 @@ google.golang.org/protobuf v1.23.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2 google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGjtUeSXeh4= google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c= -google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= -google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= -gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/ini.v1 v1.66.6 h1:LATuAqN/shcYAOkv3wl2L4rkaKqkcgTBQjOyYDvcPKI= gopkg.in/ini.v1 v1.66.6/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce h1:+JknDZhAj8YMt7GC73Ei8pv4MzjDUNPHgQWJdtMAaDU= @@ -727,9 +698,7 @@ gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkep gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/sessiontest/redis_test.go b/internal/sessiontest/redis_test.go index b8de08bbd..518519327 100644 --- a/internal/sessiontest/redis_test.go +++ b/internal/sessiontest/redis_test.go @@ -198,44 +198,7 @@ func checkErrorInternal(t *testing.T, err error) { require.Equal(t, string(server.ErrorInternal.Type), serr.RemoteError.ErrorName) } -func TestRedisUpdates(t *testing.T) { - mr, cert := startRedis(t, true) - defer mr.Close() - - irmaServer := StartIrmaServer(t, redisConfigDecorator(mr, cert, "", IrmaServerConfiguration)()) - defer irmaServer.Stop() - qr, token, _, err := irmaServer.irma.StartSession(irma.NewDisclosureRequest( - irma.NewAttributeTypeIdentifier("irma-demo.RU.studentCard.studentID"), - ), nil) - require.NoError(t, err) - - var o interface{} - transport := irma.NewHTTPTransport(qr.URL, false) - transport.SetHeader(irma.MinVersionHeader, "2.8") - transport.SetHeader(irma.MaxVersionHeader, "2.8") - transport.SetHeader(irma.AuthorizationHeader, "testauthtoken") - clientToken, err := mr.Get("token:" + string(token)) - require.NoError(t, err) - - initialData, _ := mr.Get("session:" + clientToken) - require.NoError(t, transport.Get("", &o)) - updatedData, _ := mr.Get("session:" + clientToken) - require.NoError(t, transport.Get("", &o)) - latestData, _ := mr.Get("session:" + clientToken) - - // First Get should update the data stored in Redis - require.NotEqual(t, updatedData, initialData) - // Second Get should not update the data stored in Redis - require.Equal(t, updatedData, latestData) - - // lock session for token - require.NoError(t, mr.Set("lock:"+clientToken, "bla")) - defer mr.Del("lock:" + clientToken) - - // try to update locked session - err = transport.Get("", &o) - checkErrorInternal(t, err) -} +// TODO: What to do with TestRedisUpdates? func TestRedisRedundancy(t *testing.T) { mr, cert := startRedis(t, true) diff --git a/irma/cmd/config.go b/irma/cmd/config.go index 4c9e9b150..aa28c992c 100644 --- a/irma/cmd/config.go +++ b/irma/cmd/config.go @@ -2,15 +2,16 @@ package cmd import ( "crypto/tls" - irma "github.com/privacybydesign/irmago" - "github.com/privacybydesign/irmago/server" - "github.com/privacybydesign/irmago/server/keyshare" "net/smtp" "os" "path/filepath" "regexp" "strings" + irma "github.com/privacybydesign/irmago" + "github.com/privacybydesign/irmago/server" + "github.com/privacybydesign/irmago/server/keyshare" + "github.com/go-errors/errors" "github.com/mitchellh/mapstructure" "github.com/sirupsen/logrus" @@ -40,8 +41,8 @@ func configureEmail() keyshare.EmailConfiguration { } } -func configureIRMAServer() *server.Configuration { - return &server.Configuration{ +func configureIRMAServer() (*server.Configuration, error) { + conf := &server.Configuration{ SchemesPath: viper.GetString("schemes_path"), SchemesAssetsPath: viper.GetString("schemes_assets_path"), SchemesUpdateInterval: viper.GetInt("schemes_update"), @@ -68,6 +69,31 @@ func configureIRMAServer() *server.Configuration { AllowUnsignedCallbacks: viper.GetBool("allow_unsigned_callbacks"), AugmentClientReturnURL: viper.GetBool("augment_client_return_url"), } + + // Parse session store configuration + switch conf.StoreType { + case "redis": + conf.RedisSettings = &server.RedisSettings{} + conf.RedisSettings.Addr = viper.GetString("redis_addr") + conf.RedisSettings.SentinelAddrs = viper.GetStringSlice("redis_sentinel_addrs") + conf.RedisSettings.SentinelMasterName = viper.GetString("redis_sentinel_master_name") + conf.RedisSettings.AcceptInconsistencyRisk = viper.GetBool("redis_accept_inconsistency_risk") + + if conf.RedisSettings.Addr == "" && len(conf.RedisSettings.SentinelAddrs) == 0 || conf.RedisSettings.Addr != "" && len(conf.RedisSettings.SentinelAddrs) > 0 { + return nil, errors.New("When Redis is used as session data store, exactly one of --redis-addr, --redis-sentinel-addrs or --redis-cluster-addrs must be specified.") + } + + if conf.RedisSettings.Password = viper.GetString("redis_pw"); conf.RedisSettings.Password == "" && !viper.GetBool("redis_allow_empty_password") { + return nil, errors.New("When Redis is used as session data store, a non-empty Redis password must be specified with the --redis-pw flag. This restriction can be relaxed by setting the --redis-allow-empty-password flag to true.") + } + + conf.RedisSettings.DB = viper.GetInt("redis_db") + + conf.RedisSettings.TLSCertificate = viper.GetString("redis_tls_cert") + conf.RedisSettings.TLSCertificateFile = viper.GetString("redis_tls_cert_file") + conf.RedisSettings.DisableTLS = viper.GetBool("redis_no_tls") + } + return conf, nil } func configureTLS() *tls.Config { diff --git a/irma/cmd/keyshare-myirma.go b/irma/cmd/keyshare-myirma.go index 64da4ca51..9b60fd359 100644 --- a/irma/cmd/keyshare-myirma.go +++ b/irma/cmd/keyshare-myirma.go @@ -60,6 +60,19 @@ func init() { flags.Int("db-max-idle-time", 0, "Time in seconds after which idle database connections are closed (default unlimited)") flags.Int("db-max-open-time", 0, "Maximum lifetime in seconds of open database connections (default unlimited)") + headers["store-type"] = "Session store configuration" + flags.String("store-type", "", "specifies how session state will be saved on the server (default \"memory\")") + flags.String("redis-addr", "", "Redis address, to be specified as host:port") + flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") + flags.StringSlice("redis-cluster-addrs", nil, "Redis Cluster addresses, to be specified as host:port") + flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") + flags.String("redis-pw", "", "Redis server password") + flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") + flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") + flags.String("redis-tls-cert", "", "use Redis TLS with specific certificate or certificate authority") + flags.String("redis-tls-cert-file", "", "use Redis TLS path to specific certificate or certificate authority") + flags.Bool("redis-no-tls", false, "disable Redis TLS (by default, Redis TLS is enabled with the system certificate pool)") + headers["keyshare-attributes"] = "IRMA session configuration" flags.StringSlice("keyshare-attributes", nil, "Attributes allowed for login to myirma") flags.StringSlice("email-attributes", nil, "Attributes allowed for adding email addresses") @@ -98,9 +111,14 @@ func init() { func configureMyirmaServer(cmd *cobra.Command) (*myirmaserver.Configuration, error) { readConfig(cmd, "myirmaserver", "myirmaserver", []string{".", "/etc/myirmaserver/"}, nil) + irmaServerConf, err := configureIRMAServer() + if err != nil { + return nil, err + } + // And build the configuration conf := &myirmaserver.Configuration{ - Configuration: configureIRMAServer(), + Configuration: irmaServerConf, EmailConfiguration: configureEmail(), CORSAllowedOrigins: viper.GetStringSlice("cors_allowed_origins"), diff --git a/irma/cmd/keyshare-server.go b/irma/cmd/keyshare-server.go index 0bf27b204..9f768f230 100644 --- a/irma/cmd/keyshare-server.go +++ b/irma/cmd/keyshare-server.go @@ -57,14 +57,27 @@ func init() { flags.Int("db-max-idle-time", 0, "Time in seconds after which idle database connections are closed (default unlimited)") flags.Int("db-max-open-time", 0, "Maximum lifetime in seconds of open database connections (default unlimited)") + headers["store-type"] = "Session store configuration" + flags.String("store-type", "", "specifies how session state will be saved on the server (default \"memory\")") + flags.String("redis-addr", "", "Redis address, to be specified as host:port") + flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") + flags.StringSlice("redis-cluster-addrs", nil, "Redis Cluster addresses, to be specified as host:port") + flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") + flags.String("redis-pw", "", "Redis server password") + flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") + flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") + flags.String("redis-tls-cert", "", "use Redis TLS with specific certificate or certificate authority") + flags.String("redis-tls-cert-file", "", "use Redis TLS path to specific certificate or certificate authority") + flags.Bool("redis-no-tls", false, "disable Redis TLS (by default, Redis TLS is enabled with the system certificate pool)") + headers["jwt-privkey"] = "Cryptographic keys" flags.String("jwt-privkey", "", "Private jwt key of keyshare server") flags.String("jwt-privkey-file", "", "Path to file containing private jwt key of keyshare server") flags.Int("jwt-privkey-id", 0, "Key identifier of keyshare server public key matching used private key") flags.String("jwt-issuer", keysharecore.JWTIssuerDefault, "JWT issuer used in \"iss\" field") flags.Int("jwt-pin-expiry", keysharecore.JWTPinExpiryDefault, "Expiry of PIN JWT in seconds") - flags.String("storage-primary-keyfile", "", "Primary key used for encrypting and decrypting secure containers") - flags.StringSlice("storage-fallback-keyfile", nil, "Fallback key(s) used to decrypt older secure containers") + flags.String("storage-primary-key-file", "", "Primary key used for encrypting and decrypting secure containers") + flags.StringSlice("storage-fallback-key-file", nil, "Fallback key(s) used to decrypt older secure containers") headers["keyshare-attribute"] = "Keyshare server attribute issued during registration" flags.String("keyshare-attribute", "", "Attribute identifier that contains username") @@ -98,9 +111,14 @@ func init() { func configureKeyshareServer(cmd *cobra.Command) (*keyshareserver.Configuration, error) { readConfig(cmd, "keyshareserver", "keyshareserver", []string{".", "/etc/keyshareserver"}, nil) + irmaServerConf, err := configureIRMAServer() + if err != nil { + return nil, err + } + // And build the configuration conf := &keyshareserver.Configuration{ - Configuration: configureIRMAServer(), + Configuration: irmaServerConf, EmailConfiguration: configureEmail(), DBType: keyshareserver.DBType(viper.GetString("db_type")), diff --git a/irma/cmd/server.go b/irma/cmd/server.go index 3ab724977..b9bf3336e 100644 --- a/irma/cmd/server.go +++ b/irma/cmd/server.go @@ -124,6 +124,9 @@ func setFlags(cmd *cobra.Command, production bool) error { headers["store-type"] = "Session store configuration" flags.String("store-type", "", "specifies how session state will be saved on the server (default \"memory\")") flags.String("redis-addr", "", "Redis address, to be specified as host:port") + flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") + flags.StringSlice("redis-cluster-addrs", nil, "Redis Cluster addresses, to be specified as host:port") + flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") @@ -176,9 +179,14 @@ func configureServer(cmd *cobra.Command) (*requestorserver.Configuration, error) }, ) + irmaServerConf, err := configureIRMAServer() + if err != nil { + return nil, err + } + // Read configuration from flags and/or environmental variables conf := &requestorserver.Configuration{ - Configuration: configureIRMAServer(), + Configuration: irmaServerConf, Permissions: requestorserver.Permissions{ Disclosing: handlePermission("disclose_perms"), Signing: handlePermission("sign_perms"), @@ -217,11 +225,10 @@ func configureServer(cmd *cobra.Command) (*requestorserver.Configuration, error) } // Handle requestors - var err error - if err = handleMapOrString("requestors", &conf.Requestors); err != nil { + if err := handleMapOrString("requestors", &conf.Requestors); err != nil { return nil, err } - if err = handleMapOrString("static_sessions", &conf.StaticSessions); err != nil { + if err := handleMapOrString("static_sessions", &conf.StaticSessions); err != nil { return nil, err } var m map[string]*irma.RevocationSetting @@ -232,24 +239,6 @@ func configureServer(cmd *cobra.Command) (*requestorserver.Configuration, error) conf.RevocationSettings[irma.NewCredentialTypeIdentifier(i)] = s } - // Parse Redis store configuration - if conf.StoreType == "redis" { - conf.RedisSettings = &server.RedisSettings{} - if conf.RedisSettings.Addr = viper.GetString("redis_addr"); conf.RedisSettings.Addr == "" { - return nil, errors.New("When Redis is used as session data store, a Redis URL must be specified with the --redis-addr flag.") - } - - if conf.RedisSettings.Password = viper.GetString("redis_pw"); conf.RedisSettings.Password == "" && !viper.GetBool("redis_allow_empty_password") { - return nil, errors.New("When Redis is used as session data store, a non-empty Redis password must be specified with the --redis-pw flag. This restriction can be relaxed by setting the --redis-allow-empty-password flag to true.") - } - - conf.RedisSettings.DB = viper.GetInt("redis_db") - - conf.RedisSettings.TLSCertificate = viper.GetString("redis_tls_cert") - conf.RedisSettings.TLSCertificateFile = viper.GetString("redis_tls_cert_file") - conf.RedisSettings.DisableTLS = viper.GetBool("redis_no_tls") - } - logger.Debug("Done configuring") return conf, nil diff --git a/server/conf.go b/server/conf.go index 2717ed338..8c2a6a7ee 100644 --- a/server/conf.go +++ b/server/conf.go @@ -1,16 +1,20 @@ package server import ( + "context" "crypto/rsa" "crypto/tls" + "crypto/x509" "encoding/json" "fmt" "regexp" "strconv" "strings" + "time" "github.com/go-co-op/gocron" "github.com/go-errors/errors" + "github.com/go-redis/redis/v8" "github.com/golang-jwt/jwt/v4" "github.com/privacybydesign/gabi/gabikeys" irma "github.com/privacybydesign/irmago" @@ -51,6 +55,8 @@ type Configuration struct { StoreType string `json:"store_type" mapstructure:"store_type"` // RedisSettings that need to be specified when Redis is used as session data store. RedisSettings *RedisSettings `json:"redis_settings" mapstructure:"redis_settings"` + // redisClient that is already initialized using the above RedisSettings. + redisClient *RedisClient `json:"-"` // Static session requests that can be created by POST /session/{name} StaticSessions map[string]interface{} `json:"static_sessions"` @@ -96,10 +102,20 @@ type Configuration struct { Production bool `json:"production" mapstructure:"production"` } +type RedisClient struct { + *redis.Client + FailoverMode bool +} + type RedisSettings struct { - Addr string `json:"address,omitempty" mapstructure:"address"` + Addr string `json:"address,omitempty" mapstructure:"address"` + SentinelAddrs []string `json:"sentinel_addresses,omitempty" mapstructure:"sentinel_addresses"` + SentinelMasterName string `json:"sentinel_master_name,omitempty" mapstructure:"sentinel_master_name"` + AcceptInconsistencyRisk bool `json:"accept_inconsistency_risk,omitempty" mapstructure:"accept_inconsistency_risk"` + Password string `json:"password,omitempty" mapstructure:"password"` DB int `json:"db,omitempty" mapstructure:"db"` + // TODO: ACL prefix? TLSCertificate string `json:"tls_cert,omitempty" mapstructure:"tls_cert"` TLSCertificateFile string `json:"tls_cert_file,omitempty" mapstructure:"tls_cert_file"` @@ -144,7 +160,7 @@ func (conf *Configuration) Check() error { } if conf.EnableSSE && conf.StoreType == "redis" { - return errors.New("Currently server-sent events (SSE) cannot be used simultaneously with the Redis session store.") + return errors.New("Currently server-sent events (SSE) cannot be used simultaneously with the Redis/etcd session store.") } return nil @@ -415,6 +431,86 @@ func (conf *Configuration) verifyJwtPrivateKey() error { return err } +// RedisClient returns the Redis client using the settings from the configuration. +func (conf *Configuration) RedisClient() (*RedisClient, error) { + if conf.redisClient != nil { + return conf.redisClient, nil + } + + // Configure Redis TLS. If Redis TLS is disabled, tlsConfig becomes nil and the redis client will not use TLS. + tlsConfig, err := conf.redisTLSConfig() + if err != nil { + return nil, err + } + + // setup client + var cl *redis.Client + if len(conf.RedisSettings.SentinelAddrs) > 0 { + cl = redis.NewFailoverClient(&redis.FailoverOptions{ + MasterName: conf.RedisSettings.SentinelMasterName, + SentinelAddrs: conf.RedisSettings.SentinelAddrs, + Password: conf.RedisSettings.Password, + DB: conf.RedisSettings.DB, + TLSConfig: tlsConfig, + }) + } else { + cl = redis.NewClient(&redis.Options{ + Addr: conf.RedisSettings.Addr, + Password: conf.RedisSettings.Password, + DB: conf.RedisSettings.DB, + TLSConfig: tlsConfig, + }) + } + if err := cl.Ping(context.Background()).Err(); err != nil { + return nil, errors.WrapPrefix(err, "failed to connect to Redis", 0) + } + + // Check whether Redis is in failover mode (either Redis Sentinel or Redis Cluster) + failoverMode := len(conf.RedisSettings.SentinelAddrs) > 0 || cl.ClusterInfo(context.Background()).Err() == nil + if failoverMode { + if !conf.RedisSettings.AcceptInconsistencyRisk { + return nil, errors.New("inconsistency risk not accepted for using Redis Sentinel/Cluster (see --accept-inconsistency-risk in irma server -h)") + } + if replicasReached, _ := cl.Wait(context.Background(), 2, 2*time.Second).Result(); replicasReached < 2 { + conf.Logger.Warn("Redis replication factor is less than 2, this may cause availability issues") + } + } + conf.redisClient = &RedisClient{Client: cl, FailoverMode: failoverMode} + return conf.redisClient, nil +} + +func (conf *Configuration) redisTLSConfig() (*tls.Config, error) { + if conf.RedisSettings.DisableTLS { + if conf.RedisSettings.TLSCertificate != "" || conf.RedisSettings.TLSCertificateFile != "" { + err := errors.New("Redis TLS cannot be disabled when a Redis TLS certificate is specified.") + return nil, errors.WrapPrefix(err, "Redis TLS config failed", 0) + } + return nil, nil + } + + if conf.RedisSettings.TLSCertificate != "" || conf.RedisSettings.TLSCertificateFile != "" { + cert, err := common.ReadKey(conf.RedisSettings.TLSCertificate, conf.RedisSettings.TLSCertificateFile) + if err != nil { + return nil, errors.WrapPrefix(err, "Redis TLS config failed", 0) + } + tlsConfig := &tls.Config{ + RootCAs: x509.NewCertPool(), + } + tlsConfig.RootCAs.AppendCertsFromPEM(cert) + return tlsConfig, nil + } + + // By default, the certificate pool of the system is used + systemCerts, err := x509.SystemCertPool() + if err != nil { + return nil, errors.WrapPrefix(err, "Redis TLS config failed", 0) + } + tlsConfig := &tls.Config{ + RootCAs: systemCerts, + } + return tlsConfig, nil +} + // ReplacePortString is a helper that returns a copy of the specified url of the form // "http(s)://...:port" with "port" replaced by the specified port. func ReplacePortString(url string, port int) string { diff --git a/server/irmaserver/api.go b/server/irmaserver/api.go index 535bb7fe5..475b83a75 100644 --- a/server/irmaserver/api.go +++ b/server/irmaserver/api.go @@ -6,16 +6,15 @@ package irmaserver import ( "context" - "crypto/tls" - "crypto/x509" + "encoding/json" + "fmt" "net/http" "net/url" + "sync" "time" "github.com/go-co-op/gocron" - "github.com/bsm/redislock" - "github.com/go-redis/redis/v8" "github.com/privacybydesign/irmago/internal/common" "github.com/alexandrevicenzi/go-sse" @@ -30,11 +29,13 @@ import ( var AllowIssuingExpiredCredentials = false type Server struct { - conf *server.Configuration - router *chi.Mux - sessions sessionStore - scheduler *gocron.Scheduler - serverSentEvents *sse.Server + conf *server.Configuration + router *chi.Mux + sessions sessionStore + scheduler *gocron.Scheduler + serverSentEvents *sse.Server + activeSSEHandlers map[irma.RequestorToken]bool + activeSSEHandlersMutex sync.Mutex } // Default server instance @@ -57,9 +58,10 @@ func New(conf *server.Configuration) (*Server, error) { conf.IrmaConfiguration.Revocation.ServerSentEvents = e s := &Server{ - conf: conf, - scheduler: gocron.NewScheduler(time.UTC), - serverSentEvents: e, + conf: conf, + scheduler: gocron.NewScheduler(time.UTC), + serverSentEvents: e, + activeSSEHandlers: make(map[irma.RequestorToken]bool), } switch conf.StoreType { @@ -67,9 +69,10 @@ func New(conf *server.Configuration) (*Server, error) { fallthrough // no specification defaults to the memory session store case "memory": s.sessions = &memorySessionStore{ - requestor: make(map[irma.RequestorToken]*session), - client: make(map[irma.ClientToken]*session), - conf: conf, + conf: conf, + requestor: make(map[irma.RequestorToken]*memorySessionData), + client: make(map[irma.ClientToken]*memorySessionData), + statusChannels: make(map[irma.RequestorToken][]chan statusChange), } if _, err := s.scheduler.Every(10).Seconds().Do(func() { @@ -78,27 +81,13 @@ func New(conf *server.Configuration) (*Server, error) { return nil, err } case "redis": - // Configure Redis TLS. If Redis TLS is disabled, tlsConfig becomes nil and the redis client will not use TLS. - tlsConfig, err := redisTLSConfig(conf) + cl, err := conf.RedisClient() if err != nil { return nil, err } - - // setup client - cl := redis.NewClient(&redis.Options{ - Addr: conf.RedisSettings.Addr, - Password: conf.RedisSettings.Password, - DB: conf.RedisSettings.DB, - TLSConfig: tlsConfig, - }) - if err := cl.Ping(context.Background()).Err(); err != nil { - return nil, errors.WrapPrefix(err, "failed to connect to Redis", 0) - } - s.sessions = &redisSessionStore{ client: cl, conf: conf, - locker: redislock.New(cl), } default: return nil, errors.New("storeType not known") @@ -124,38 +113,6 @@ func New(conf *server.Configuration) (*Server, error) { return s, nil } -func redisTLSConfig(conf *server.Configuration) (*tls.Config, error) { - if conf.RedisSettings.DisableTLS { - if conf.RedisSettings.TLSCertificate != "" || conf.RedisSettings.TLSCertificateFile != "" { - err := errors.New("Redis TLS cannot be disabled when a Redis TLS certificate is specified.") - return nil, errors.WrapPrefix(err, "Redis TLS config failed", 0) - } - return nil, nil - } - - if conf.RedisSettings.TLSCertificate != "" || conf.RedisSettings.TLSCertificateFile != "" { - cert, err := common.ReadKey(conf.RedisSettings.TLSCertificate, conf.RedisSettings.TLSCertificateFile) - if err != nil { - return nil, errors.WrapPrefix(err, "Redis TLS config failed", 0) - } - tlsConfig := &tls.Config{ - RootCAs: x509.NewCertPool(), - } - tlsConfig.RootCAs.AppendCertsFromPEM(cert) - return tlsConfig, nil - } - - // By default, the certificate pool of the system is used - systemCerts, err := x509.SystemCertPool() - if err != nil { - return nil, errors.WrapPrefix(err, "Redis TLS config failed", 0) - } - tlsConfig := &tls.Config{ - RootCAs: systemCerts, - } - return tlsConfig, nil -} - // HandlerFunc returns a http.HandlerFunc that handles the IRMA protocol // with IRMA apps. // @@ -301,27 +258,45 @@ func (s *Server) startNextSession( } request.Base().DevelopmentMode = !s.conf.Production - session, err := s.newSession(action, rrequest, disclosed, FrontendAuth) + ses, err := s.newSession(action, rrequest, disclosed, FrontendAuth) if err != nil { return nil, "", nil, err } - s.conf.Logger.WithFields(logrus.Fields{"action": action, "session": session.RequestorToken}).Infof("Session started") + s.conf.Logger.WithFields(logrus.Fields{"action": action, "session": ses.RequestorToken}).Infof("Session started") if s.conf.Logger.IsLevelEnabled(logrus.DebugLevel) { s.conf.Logger. - WithFields(logrus.Fields{"session": session.RequestorToken, "clienttoken": session.ClientToken}). + WithFields(logrus.Fields{"session": ses.RequestorToken, "clienttoken": ses.ClientToken}). Info("Session request: ", server.ToJson(rrequest)) } else { s.conf.Logger. - WithFields(logrus.Fields{"session": session.RequestorToken}). + WithFields(logrus.Fields{"session": ses.RequestorToken}). Info("Session request (purged of attribute values): ", server.ToJson(purgeRequest(rrequest))) } - session.handler = handler + + if handler != nil { + go func() { + statusChangeChan, err := s.sessions.subscribeStatusChanges(ses.RequestorToken) + if err != nil { + s.conf.Logger.WithError(err).Error("Failed to subscribe to session status changes for handler") + return + } + for change := range statusChangeChan { + if change.Status.Finished() { + s.sessions.transaction(ses.RequestorToken, func(ses *sessionData) error { + handler(ses.Result) + return nil + }) + return + } + } + }() + } url, err := url.Parse(s.conf.URL) if err != nil { return nil, "", nil, err } - url = url.JoinPath("session", string(session.ClientToken)) + url = url.JoinPath("session", string(ses.ClientToken)) if request.Base().Host != "" { url.Host = request.Base().Host } @@ -330,9 +305,9 @@ func (s *Server) startNextSession( Type: action, URL: url.String(), }, - session.RequestorToken, + ses.RequestorToken, &irma.FrontendSessionRequest{ - Authorization: session.FrontendAuth, + Authorization: ses.FrontendAuth, PairingRecommended: pairingRecommended, MinProtocolVersion: minFrontendProtocolVersion, MaxProtocolVersion: maxFrontendProtocolVersion, @@ -345,13 +320,10 @@ func GetSessionResult(requestorToken irma.RequestorToken) (*server.SessionResult return s.GetSessionResult(requestorToken) } func (s *Server) GetSessionResult(requestorToken irma.RequestorToken) (res *server.SessionResult, err error) { - session, err := s.sessions.get(requestorToken) - defer func() { err = updateAndUnlock(session, err) }() - if err != nil { - return - } - - res = session.Result + err = s.sessions.transaction(requestorToken, func(session *sessionData) error { + res = session.Result + return nil + }) return } @@ -360,13 +332,10 @@ func GetRequest(requestorToken irma.RequestorToken) (irma.RequestorRequest, erro return s.GetRequest(requestorToken) } func (s *Server) GetRequest(requestorToken irma.RequestorToken) (req irma.RequestorRequest, err error) { - session, err := s.sessions.get(requestorToken) - defer func() { err = updateAndUnlock(session, err) }() - if err != nil { - return - } - - req = session.Rrequest + err = s.sessions.transaction(requestorToken, func(session *sessionData) error { + req = session.Rrequest + return nil + }) return } @@ -375,13 +344,10 @@ func CancelSession(requestorToken irma.RequestorToken) error { return s.CancelSession(requestorToken) } func (s *Server) CancelSession(requestorToken irma.RequestorToken) (err error) { - session, err := s.sessions.get(requestorToken) - defer func() { err = updateAndUnlock(session, err) }() - if err != nil { - return - } - - session.handleDelete() + err = s.sessions.transaction(requestorToken, func(session *sessionData) error { + session.handleDelete(s.conf) + return nil + }) return } @@ -393,13 +359,10 @@ func SetFrontendOptions(requestorToken irma.RequestorToken, request *irma.Fronte return s.SetFrontendOptions(requestorToken, request) } func (s *Server) SetFrontendOptions(requestorToken irma.RequestorToken, request *irma.FrontendOptionsRequest) (o *irma.SessionOptions, err error) { - session, err := s.sessions.get(requestorToken) - defer func() { err = updateAndUnlock(session, err) }() - if err != nil { - return - } - o, err = session.updateFrontendOptions(request) - + err = s.sessions.transaction(requestorToken, func(session *sessionData) error { + o, err = session.updateFrontendOptions(request) + return err + }) return } @@ -408,15 +371,10 @@ func (s *Server) SetFrontendOptions(requestorToken irma.RequestorToken, request func PairingCompleted(requestorToken irma.RequestorToken) error { return s.PairingCompleted(requestorToken) } -func (s *Server) PairingCompleted(requestorToken irma.RequestorToken) (err error) { - session, err := s.sessions.get(requestorToken) - defer func() { err = updateAndUnlock(session, err) }() - if err != nil { - return - } - - err = session.pairingCompleted() - return +func (s *Server) PairingCompleted(requestorToken irma.RequestorToken) error { + return s.sessions.transaction(requestorToken, func(session *sessionData) error { + return session.pairingCompleted(s.conf) + }) } // Revoke revokes the earlier issued credential specified by key. (Can only be used if this server @@ -431,56 +389,52 @@ func (s *Server) Revoke(credid irma.CredentialTypeIdentifier, key string, issued // SubscribeServerSentEvents subscribes the HTTP client to server sent events on status updates // of the specified IRMA session. -func (s *Server) SubscribeServerSentEvents(w http.ResponseWriter, r *http.Request, token irma.RequestorToken) (err error) { +func (s *Server) SubscribeServerSentEvents(w http.ResponseWriter, r *http.Request, token irma.RequestorToken) error { + r = r.WithContext(context.WithValue(r.Context(), "sse", common.SSECtx{ + Component: server.ComponentSession, + Arg: string(token), + })) + return s.sessions.transaction(token, func(session *sessionData) error { + return s.subscribeServerSentEvents(w, r, session, true) + }) +} + +func (s *Server) subscribeServerSentEvents(w http.ResponseWriter, r *http.Request, session *sessionData, requestor bool) error { if !s.conf.EnableSSE { server.WriteResponse(w, nil, &irma.RemoteError{ - Status: 500, + Status: http.StatusInternalServerError, Description: "Server sent events disabled", ErrorName: "SSE_DISABLED", }) s.conf.Logger.Info("GET /statusevents: endpoint disabled (see --sse in irma server -h)") return nil } - session, err := s.sessions.get(token) - err = updateAndUnlock(session, err) - if err != nil { - if _, ok := err.(*RedisError); ok { - // In no flow, you should end up with an storeError. If you do, be alarmed! - // Only the Redis session store implementation actively uses these errors. As the Redis session store - // currently cannot be used in combination with SSE, there should be no storeError here. - // Furthermore, the specific storeError is already logged in `session.go` and does not have - // to be logged again. - err = server.LogError(errors.Errorf("unexpectedly triggered error when trying to receive session %s", token)) - return - } else { - return - } - } - - err = s.subscribeServerSentEvents(w, r, session, true) - return -} -func (s *Server) subscribeServerSentEvents(w http.ResponseWriter, r *http.Request, session *session, requestor bool) error { - if !s.conf.EnableSSE { - server.WriteResponse(w, nil, &irma.RemoteError{ - Status: 500, - Description: "Server sent events disabled", - ErrorName: "SSE_DISABLED", - }) - s.conf.Logger.Info("GET /statusevents: endpoint disabled (see --sse in irma server -h)") + if session.Status.Finished() { + server.WriteError(w, server.ErrorUnexpectedRequest, "Can't subscribe to server sent events of finished session") return nil } - var token string - if requestor { - token = string(session.RequestorToken) + activeHandler := false + s.activeSSEHandlersMutex.Lock() + if active := s.activeSSEHandlers[session.RequestorToken]; active { + activeHandler = true } else { - token = string(session.ClientToken) + s.activeSSEHandlers[session.RequestorToken] = true } + s.activeSSEHandlersMutex.Unlock() - if session.Status.Finished() { - return server.LogError(errors.Errorf("can't subscribe to server sent events of finished session %s", token)) + if !activeHandler { + statusChangeChan, err := s.sessions.subscribeStatusChanges(session.RequestorToken) + if err != nil { + return err + } + go func() { + s.serverSentEventsHandler(session, statusChangeChan) + s.activeSSEHandlersMutex.Lock() + defer s.activeSSEHandlersMutex.Unlock() + delete(s.activeSSEHandlers, session.RequestorToken) + }() } // The EventSource.onopen Javascript callback is not consistently called across browsers (Chrome yes, Firefox+Safari no). @@ -491,47 +445,121 @@ func (s *Server) subscribeServerSentEvents(w http.ResponseWriter, r *http.Reques // event to just the webclient currently listening. (Thus the handler of this "open" event must be idempotent.) go func() { time.Sleep(200 * time.Millisecond) + token := string(session.ClientToken) + if requestor { + token = string(session.RequestorToken) + } s.serverSentEvents.SendMessage("session/"+token, sse.NewMessage("", "", "open")) s.serverSentEvents.SendMessage("frontendsession/"+token, sse.NewMessage("", "", "open")) }() + s.serverSentEvents.ServeHTTP(w, r) return nil } +func (s *Server) serverSentEventsHandler(session *sessionData, statusChangeChan chan statusChange) { + timeoutTime := time.Now().Add(session.timeout(s.conf)) + + // Close the channels when this function returns. + defer func() { + s.serverSentEvents.CloseChannel("session/" + string(session.RequestorToken)) + s.serverSentEvents.CloseChannel("session/" + string(session.ClientToken)) + s.serverSentEvents.CloseChannel("frontendsession/" + string(session.ClientToken)) + }() + + for { + select { + case change, ok := <-statusChangeChan: + if !ok { + return + } + + frontendStatusBytes, err := json.Marshal(change.FrontendSessionStatus) + if err != nil { + s.conf.Logger.Error(err) + return + } + + s.serverSentEvents.SendMessage("session/"+string(session.RequestorToken), + sse.SimpleMessage(fmt.Sprintf(`"%s"`, change.Status)), + ) + s.serverSentEvents.SendMessage("session/"+string(session.ClientToken), + sse.SimpleMessage(fmt.Sprintf(`"%s"`, change.Status)), + ) + s.serverSentEvents.SendMessage("frontendsession/"+string(session.ClientToken), + sse.SimpleMessage(string(frontendStatusBytes)), + ) + if change.Status.Finished() { + return + } + timeoutTime = time.Now().Add(change.Timeout) + case <-time.After(time.Until(timeoutTime)): + frontendStatus := irma.FrontendSessionStatus{ + Status: irma.ServerStatusTimeout, + } + frontendStatusBytes, err := json.Marshal(frontendStatus) + if err != nil { + s.conf.Logger.Error(err) + return + } + + s.serverSentEvents.SendMessage("session/"+string(session.RequestorToken), + sse.SimpleMessage(fmt.Sprintf(`"%s"`, frontendStatus.Status)), + ) + s.serverSentEvents.SendMessage("session/"+string(session.ClientToken), + sse.SimpleMessage(fmt.Sprintf(`"%s"`, frontendStatus.Status)), + ) + s.serverSentEvents.SendMessage("frontendsession/"+string(session.ClientToken), + sse.SimpleMessage(string(frontendStatusBytes)), + ) + return + } + } +} + // SessionStatus retrieves a channel on which the current session status of the specified // IRMA session can be retrieved. func SessionStatus(requestorToken irma.RequestorToken) (chan irma.ServerStatus, error) { return s.SessionStatus(requestorToken) } func (s *Server) SessionStatus(requestorToken irma.RequestorToken) (statusChan chan irma.ServerStatus, err error) { - if s.conf.StoreType == "redis" { - return nil, errors.New("SessionStatus cannot be used in combination with Redis.") + if s.conf.StoreType != "memory" { + return nil, errors.New("SessionStatus cannot be used in combination with Redis/etcd.") } - session, err := s.sessions.get(requestorToken) - err = updateAndUnlock(session, err) + statusChangeChan, err := s.sessions.subscribeStatusChanges(requestorToken) if err != nil { - return + return nil, err } + var timeoutTime time.Time + s.sessions.transaction(requestorToken, func(session *sessionData) error { + timeoutTime = time.Now().Add(session.timeout(s.conf)) + return nil + }) statusChan = make(chan irma.ServerStatus, 4) - statusChan <- session.Status - session.statusChannels = append(session.statusChannels, statusChan) - return -} + go func() { + for { + select { + case change, ok := <-statusChangeChan: + if !ok { + close(statusChan) + return + } + statusChan <- change.Status + + if change.Status.Finished() { + close(statusChan) + return + } + timeoutTime = time.Now().Add(change.Timeout) + case <-time.After(time.Until(timeoutTime)): + statusChan <- irma.ServerStatusTimeout + close(statusChan) + return + } + } + }() -// updateAndUnlock is a helper function that is mainly used in defer functions to make sure a session -// is updated and unlocked eventually. Each session gets locked automatically in the session store's -// `get` and `getClient` methods. -// If the passed error is not nil it is always returned, as this first error is more important for -// the eventual response. Otherwise, the return value of ses.updateAndUnlock() is returned. -func updateAndUnlock(ses *session, err error) error { - if ses == nil { - return err - } - e := ses.updateAndUnlock() - if err != nil { - return err - } - return e + return statusChan, nil } diff --git a/server/irmaserver/handle.go b/server/irmaserver/handle.go index 04f35a265..54d5aeaaa 100644 --- a/server/irmaserver/handle.go +++ b/server/irmaserver/handle.go @@ -24,17 +24,17 @@ import ( // Maintaining the session state is done here, as well as checking whether the session is in the // appropriate status before handling the request. -func (session *session) handleDelete() { +func (session *sessionData) handleDelete(conf *server.Configuration) { if session.Status.Finished() { return } session.markAlive() session.Result = &server.SessionResult{Token: session.RequestorToken, Status: irma.ServerStatusCancelled, Type: session.Action} - session.setStatus(irma.ServerStatusCancelled) + session.setStatus(irma.ServerStatusCancelled, conf) } -func (session *session) handleGetClientRequest(min, max *irma.ProtocolVersion, clientAuth irma.ClientAuthorization) ( +func (session *sessionData) handleGetClientRequest(min, max *irma.ProtocolVersion, clientAuth irma.ClientAuthorization, conf *server.Configuration) ( interface{}, *irma.RemoteError) { if session.Status != irma.ServerStatusInitialized { @@ -42,41 +42,42 @@ func (session *session) handleGetClientRequest(min, max *irma.ProtocolVersion, c } session.markAlive() - logger := session.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}) + logger := conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}) var err error if session.Version, err = session.chooseProtocolVersion(min, max); err != nil { - return nil, session.fail(server.ErrorProtocolVersion, "") + return nil, session.fail(server.ErrorProtocolVersion, "", conf) } // Protocol versions below 2.8 don't include an authorization header. Therefore skip the authorization // header presence check if a lower version is used. if clientAuth == "" && session.Version.Above(2, 7) { - return nil, session.fail(server.ErrorIrmaUnauthorized, "No authorization header provided") + return nil, session.fail(server.ErrorIrmaUnauthorized, "No authorization header provided", conf) } session.ClientAuth = clientAuth // we include the latest revocation updates for the client here, as opposed to when the session // was started, so that the client always gets the very latest revocation records - if err = session.conf.IrmaConfiguration.Revocation.SetRevocationUpdates(session.request.Base()); err != nil { - return nil, session.fail(server.ErrorRevocation, err.Error()) + sessionRequest := session.Rrequest.SessionRequest() + if err = conf.IrmaConfiguration.Revocation.SetRevocationUpdates(sessionRequest.Base()); err != nil { + return nil, session.fail(server.ErrorRevocation, err.Error(), conf) } // Handle legacy clients that do not support condiscon, by attempting to convert the condiscon // session request to the legacy session request format - legacy, legacyErr := session.request.Legacy() + legacy, legacyErr := sessionRequest.Legacy() session.LegacyCompatible = legacyErr == nil if legacyErr != nil { logger.Info("Using condiscon: backwards compatibility with legacy IRMA apps is disabled") } logger.WithFields(logrus.Fields{"version": session.Version.String()}).Debugf("Protocol version negotiated") - session.request.Base().ProtocolVersion = session.Version + sessionRequest.Base().ProtocolVersion = session.Version if session.Options.PairingMethod != irma.PairingMethodNone && session.Version.Above(2, 7) { - session.setStatus(irma.ServerStatusPairing) + session.setStatus(irma.ServerStatusPairing, conf) } else { - session.setStatus(irma.ServerStatusConnected) + session.setStatus(irma.ServerStatusConnected, conf) } if session.Version.Below(2, 5) { @@ -89,22 +90,22 @@ func (session *session) handleGetClientRequest(min, max *irma.ProtocolVersion, c // These versions do not support the ClientSessionRequest format, so send the SessionRequest. request, err := session.getRequest() if err != nil { - return nil, session.fail(server.ErrorRevocation, err.Error()) + return nil, session.fail(server.ErrorRevocation, err.Error(), conf) } return request, nil } info, err := session.getClientRequest() if err != nil { - return nil, session.fail(server.ErrorRevocation, err.Error()) + return nil, session.fail(server.ErrorRevocation, err.Error(), conf) } return info, nil } -func (session *session) handleGetStatus() (irma.ServerStatus, *irma.RemoteError) { +func (session *sessionData) handleGetStatus() (irma.ServerStatus, *irma.RemoteError) { return session.Status, nil } -func (session *session) handlePostSignature(signature *irma.SignedMessage) (*irma.ServerSessionResponse, *irma.RemoteError) { +func (session *sessionData) handlePostSignature(signature *irma.SignedMessage, conf *server.Configuration) (*irma.ServerSessionResponse, *irma.RemoteError) { session.markAlive() var err error @@ -112,14 +113,15 @@ func (session *session) handlePostSignature(signature *irma.SignedMessage) (*irm session.Result.Signature = signature // In case of chained sessions, we also expect attributes from previous sessions to be disclosed again. - request := session.request.(*irma.SignatureRequest) + sessionRequest := session.Rrequest.SessionRequest() + request := sessionRequest.(*irma.SignatureRequest) request.Disclose = append(request.Disclose, session.ImplicitDisclosure...) - session.Result.Disclosed, session.Result.ProofStatus, err = signature.Verify(session.conf.IrmaConfiguration, request) + session.Result.Disclosed, session.Result.ProofStatus, err = signature.Verify(conf.IrmaConfiguration, request) if err != nil && err == irma.ErrMissingPublicKey { - rerr = session.fail(server.ErrorUnknownPublicKey, err.Error()) + rerr = session.fail(server.ErrorUnknownPublicKey, err.Error(), conf) } else if err != nil { - rerr = session.fail(server.ErrorUnknown, err.Error()) + rerr = session.fail(server.ErrorUnknown, err.Error(), conf) } return &irma.ServerSessionResponse{ @@ -129,21 +131,21 @@ func (session *session) handlePostSignature(signature *irma.SignedMessage) (*irm }, rerr } -func (session *session) handlePostDisclosure(disclosure *irma.Disclosure) (*irma.ServerSessionResponse, *irma.RemoteError) { +func (session *sessionData) handlePostDisclosure(disclosure *irma.Disclosure, conf *server.Configuration) (*irma.ServerSessionResponse, *irma.RemoteError) { session.markAlive() var err error var rerr *irma.RemoteError // In case of chained sessions, we also expect attributes from previous sessions to be disclosed again. - request := session.request.(*irma.DisclosureRequest) + request := session.Rrequest.SessionRequest().(*irma.DisclosureRequest) request.Disclose = append(request.Disclose, session.ImplicitDisclosure...) - session.Result.Disclosed, session.Result.ProofStatus, err = disclosure.Verify(session.conf.IrmaConfiguration, request) + session.Result.Disclosed, session.Result.ProofStatus, err = disclosure.Verify(conf.IrmaConfiguration, request) if err != nil && err == irma.ErrMissingPublicKey { - rerr = session.fail(server.ErrorUnknownPublicKey, err.Error()) + rerr = session.fail(server.ErrorUnknownPublicKey, err.Error(), conf) } else if err != nil { - rerr = session.fail(server.ErrorUnknown, err.Error()) + rerr = session.fail(server.ErrorUnknown, err.Error(), conf) } return &irma.ServerSessionResponse{ @@ -153,24 +155,24 @@ func (session *session) handlePostDisclosure(disclosure *irma.Disclosure) (*irma }, rerr } -func (session *session) handlePostCommitments(commitments *irma.IssueCommitmentMessage) (*irma.ServerSessionResponse, *irma.RemoteError) { +func (session *sessionData) handlePostCommitments(commitments *irma.IssueCommitmentMessage, conf *server.Configuration) (*irma.ServerSessionResponse, *irma.RemoteError) { session.markAlive() - request := session.request.(*irma.IssuanceRequest) + request := session.Rrequest.SessionRequest().(*irma.IssuanceRequest) discloseCount := len(commitments.Proofs) - len(request.Credentials) if discloseCount < 0 { - return nil, session.fail(server.ErrorMalformedInput, "Received insufficient proofs") + return nil, session.fail(server.ErrorMalformedInput, "Received insufficient proofs", conf) } // Compute list of public keys against which to verify the received proofs disclosureproofs := irma.ProofList(commitments.Proofs[:discloseCount]) - pubkeys, err := disclosureproofs.ExtractPublicKeys(session.conf.IrmaConfiguration) + pubkeys, err := disclosureproofs.ExtractPublicKeys(conf.IrmaConfiguration) if err != nil { - return nil, session.fail(server.ErrorMalformedInput, err.Error()) + return nil, session.fail(server.ErrorMalformedInput, err.Error(), conf) } for _, cred := range request.Credentials { iss := cred.CredentialTypeID.IssuerIdentifier() - pubkey, _ := session.conf.IrmaConfiguration.PublicKey(iss, cred.KeyCounter) // No error, already checked earlier + pubkey, _ := conf.IrmaConfiguration.PublicKey(iss, cred.KeyCounter) // No error, already checked earlier pubkeys = append(pubkeys, pubkey) } @@ -178,10 +180,10 @@ func (session *session) handlePostCommitments(commitments *irma.IssueCommitmentM for i, proof := range commitments.Proofs { pubkey := pubkeys[i] schemeid := irma.NewIssuerIdentifier(pubkey.Issuer).SchemeManagerIdentifier() - if session.conf.IrmaConfiguration.SchemeManagers[schemeid].Distributed() { - proofP, err := session.getProofP(commitments, schemeid) + if conf.IrmaConfiguration.SchemeManagers[schemeid].Distributed() { + proofP, err := session.getProofP(commitments, schemeid, conf) if err != nil { - return nil, session.fail(server.ErrorKeyshareProofMissing, err.Error()) + return nil, session.fail(server.ErrorKeyshareProofMissing, err.Error(), conf) } proof.MergeProofP(proofP, pubkey) } @@ -191,41 +193,41 @@ func (session *session) handlePostCommitments(commitments *irma.IssueCommitmentM now := time.Now() request.Disclose = append(request.Disclose, session.ImplicitDisclosure...) session.Result.Disclosed, session.Result.ProofStatus, err = commitments.Disclosure().VerifyAgainstRequest( - session.conf.IrmaConfiguration, request, request.GetContext(), request.GetNonce(nil), pubkeys, &now, false, + conf.IrmaConfiguration, request, request.GetContext(), request.GetNonce(nil), pubkeys, &now, false, ) if err != nil { if err == irma.ErrMissingPublicKey { - return nil, session.fail(server.ErrorUnknownPublicKey, "") + return nil, session.fail(server.ErrorUnknownPublicKey, "", conf) } else { - return nil, session.fail(server.ErrorUnknown, "") + return nil, session.fail(server.ErrorUnknown, "", conf) } } if session.Result.ProofStatus == irma.ProofStatusExpired { - return nil, session.fail(server.ErrorAttributesExpired, "") + return nil, session.fail(server.ErrorAttributesExpired, "", conf) } if session.Result.ProofStatus != irma.ProofStatusValid { - return nil, session.fail(server.ErrorInvalidProofs, "") + return nil, session.fail(server.ErrorInvalidProofs, "", conf) } // Compute CL signatures var sigs []*gabi.IssueSignatureMessage for i, cred := range request.Credentials { id := cred.CredentialTypeID.IssuerIdentifier() - pk, _ := session.conf.IrmaConfiguration.PublicKey(id, cred.KeyCounter) - sk, _ := session.conf.IrmaConfiguration.PrivateKeys.Latest(id) + pk, _ := conf.IrmaConfiguration.PublicKey(id, cred.KeyCounter) + sk, _ := conf.IrmaConfiguration.PrivateKeys.Latest(id) issuer := gabi.NewIssuer(sk, pk, one) proof, ok := commitments.Proofs[i+discloseCount].(*gabi.ProofU) if !ok { - return nil, session.fail(server.ErrorMalformedInput, "Received invalid issuance commitment") + return nil, session.fail(server.ErrorMalformedInput, "Received invalid issuance commitment", conf) } - attrs, witness, err := session.computeAttributes(sk, cred) + attrs, witness, err := session.computeAttributes(sk, cred, conf) if err != nil { - return nil, session.fail(server.ErrorIssuanceFailed, err.Error()) + return nil, session.fail(server.ErrorIssuanceFailed, err.Error(), conf) } - rb := session.conf.IrmaConfiguration.CredentialTypes[cred.CredentialTypeID].RandomBlindAttributeIndices() + rb := conf.IrmaConfiguration.CredentialTypes[cred.CredentialTypeID].RandomBlindAttributeIndices() sig, err := issuer.IssueSignature(proof.U, attrs, witness, commitments.Nonce2, rb) if err != nil { - return nil, session.fail(server.ErrorIssuanceFailed, err.Error()) + return nil, session.fail(server.ErrorIssuanceFailed, err.Error(), conf) } sigs = append(sigs, sig) } @@ -238,7 +240,7 @@ func (session *session) handlePostCommitments(commitments *irma.IssueCommitmentM }, nil } -func (session *session) nextSession() (irma.RequestorRequest, irma.AttributeConDisCon, error) { +func (session *sessionData) nextSession(conf *server.Configuration) (irma.RequestorRequest, irma.AttributeConDisCon, error) { base := session.Rrequest.Base() if base.NextSession == nil { return nil, nil, nil @@ -255,12 +257,12 @@ func (session *session) nextSession() (irma.RequestorRequest, irma.AttributeConD var res interface{} var err error - if session.conf.JwtRSAPrivateKey != nil { + if conf.JwtRSAPrivateKey != nil { res, err = server.ResultJwt( session.Result, - session.conf.JwtIssuer, + conf.JwtIssuer, base.ResultJwtValidity, - session.conf.JwtRSAPrivateKey, + conf.JwtRSAPrivateKey, ) if err != nil { return nil, nil, err @@ -300,8 +302,8 @@ func (session *session) nextSession() (irma.RequestorRequest, irma.AttributeConD return req, disclosed, nil } -func (s *Server) startNext(session *session, res *irma.ServerSessionResponse) error { - next, disclosed, err := session.nextSession() +func (s *Server) startNext(session *sessionData, res *irma.ServerSessionResponse) error { + next, disclosed, err := session.nextSession(s.conf) if err != nil { return err } @@ -335,8 +337,8 @@ func (s *Server) handleSessionCommitments(w http.ResponseWriter, r *http.Request server.WriteError(w, server.ErrorMalformedInput, err.Error()) return } - session := r.Context().Value("session").(*session) - res, rerr := session.handlePostCommitments(commitments) + session := r.Context().Value("session").(*sessionData) + res, rerr := session.handlePostCommitments(commitments, s.conf) if rerr != nil { server.WriteResponse(w, nil, rerr) return @@ -345,7 +347,7 @@ func (s *Server) handleSessionCommitments(w http.ResponseWriter, r *http.Request server.WriteError(w, server.ErrorNextSession, err.Error()) return } - session.setStatus(irma.ServerStatusDone) + session.setStatus(irma.ServerStatusDone, s.conf) server.WriteResponse(w, res, nil) } @@ -356,7 +358,7 @@ func (s *Server) handleSessionProofs(w http.ResponseWriter, r *http.Request) { server.WriteError(w, server.ErrorMalformedInput, err.Error()) return } - session := r.Context().Value("session").(*session) + session := r.Context().Value("session").(*sessionData) var res *irma.ServerSessionResponse var rerr *irma.RemoteError switch session.Action { @@ -366,14 +368,14 @@ func (s *Server) handleSessionProofs(w http.ResponseWriter, r *http.Request) { server.WriteError(w, server.ErrorMalformedInput, err.Error()) return } - res, rerr = session.handlePostDisclosure(disclosure) + res, rerr = session.handlePostDisclosure(disclosure, s.conf) case irma.ActionSigning: signature := &irma.SignedMessage{} if err := irma.UnmarshalValidate(bts, signature); err != nil { server.WriteError(w, server.ErrorMalformedInput, err.Error()) return } - res, rerr = session.handlePostSignature(signature) + res, rerr = session.handlePostSignature(signature, s.conf) default: rerr = server.RemoteError(server.ErrorInvalidRequest, "") } @@ -385,19 +387,17 @@ func (s *Server) handleSessionProofs(w http.ResponseWriter, r *http.Request) { server.WriteError(w, server.ErrorNextSession, err.Error()) return } - session.setStatus(irma.ServerStatusDone) + session.setStatus(irma.ServerStatusDone, s.conf) server.WriteResponse(w, res, nil) } func (s *Server) handleSessionStatus(w http.ResponseWriter, r *http.Request) { - res, err := r.Context().Value("session").(*session).handleGetStatus() + res, err := r.Context().Value("session").(*sessionData).handleGetStatus() server.WriteResponse(w, res, err) } func (s *Server) handleSessionStatusEvents(w http.ResponseWriter, r *http.Request) { - session := r.Context().Value("session").(*session) - // Unlock session, so SSE will not block the session. - session.sessions.unlock(session) + session := r.Context().Value("session").(*sessionData) r = r.WithContext(context.WithValue(r.Context(), "sse", common.SSECtx{ Component: server.ComponentSession, @@ -410,8 +410,8 @@ func (s *Server) handleSessionStatusEvents(w http.ResponseWriter, r *http.Reques } func (s *Server) handleSessionDelete(w http.ResponseWriter, r *http.Request) { - session := r.Context().Value("session").(*session) - session.handleDelete() + session := r.Context().Value("session").(*sessionData) + session.handleDelete(s.conf) w.WriteHeader(200) } @@ -425,14 +425,14 @@ func (s *Server) handleSessionGet(w http.ResponseWriter, r *http.Request) { server.WriteError(w, server.ErrorMalformedInput, err.Error()) return } - session := r.Context().Value("session").(*session) + session := r.Context().Value("session").(*sessionData) clientAuth := irma.ClientAuthorization(r.Header.Get(irma.AuthorizationHeader)) - res, err := session.handleGetClientRequest(&min, &max, clientAuth) + res, err := session.handleGetClientRequest(&min, &max, clientAuth, s.conf) server.WriteResponse(w, res, err) } func (s *Server) handleSessionGetRequest(w http.ResponseWriter, r *http.Request) { - session := r.Context().Value("session").(*session) + session := r.Context().Value("session").(*sessionData) if session.Version.Below(2, 8) { server.WriteError(w, server.ErrorUnexpectedRequest, "Endpoint is not support in used protocol version") return @@ -440,21 +440,19 @@ func (s *Server) handleSessionGetRequest(w http.ResponseWriter, r *http.Request) var rerr *irma.RemoteError request, err := session.getRequest() if err != nil { - rerr = session.fail(server.ErrorRevocation, err.Error()) + rerr = session.fail(server.ErrorRevocation, err.Error(), s.conf) } server.WriteResponse(w, request, rerr) } func (s *Server) handleFrontendStatus(w http.ResponseWriter, r *http.Request) { - session := r.Context().Value("session").(*session) + session := r.Context().Value("session").(*sessionData) status := irma.FrontendSessionStatus{Status: session.Status, NextSession: session.Next} server.WriteResponse(w, status, nil) } func (s *Server) handleFrontendStatusEvents(w http.ResponseWriter, r *http.Request) { - session := r.Context().Value("session").(*session) - // Unlock session, so frontend status events will not block the session. - session.sessions.unlock(session) + session := r.Context().Value("session").(*sessionData) r = r.WithContext(context.WithValue(r.Context(), "sse", common.SSECtx{ Component: server.ComponentFrontendSession, @@ -480,7 +478,7 @@ func (s *Server) handleFrontendOptionsPost(w http.ResponseWriter, r *http.Reques return } - session := r.Context().Value("session").(*session) + session := r.Context().Value("session").(*sessionData) res, err := session.updateFrontendOptions(optionsRequest) if err != nil { server.WriteError(w, server.ErrorUnexpectedRequest, err.Error()) @@ -490,8 +488,8 @@ func (s *Server) handleFrontendOptionsPost(w http.ResponseWriter, r *http.Reques } func (s *Server) handleFrontendPairingCompleted(w http.ResponseWriter, r *http.Request) { - session := r.Context().Value("session").(*session) - if err := session.pairingCompleted(); err != nil { + session := r.Context().Value("session").(*sessionData) + if err := session.pairingCompleted(s.conf); err != nil { server.WriteError(w, server.ErrorUnexpectedRequest, err.Error()) return } diff --git a/server/irmaserver/helpers.go b/server/irmaserver/helpers.go index edf11ce86..7b44c17ee 100644 --- a/server/irmaserver/helpers.go +++ b/server/irmaserver/helpers.go @@ -5,7 +5,6 @@ import ( "context" "crypto/sha256" "encoding/json" - "fmt" "io" "log" "net/http" @@ -29,73 +28,42 @@ import ( // Session helpers -func (session *session) markAlive() { +func (session *sessionData) markAlive() { + // TODO: use from conf? session.LastActive = time.Now() - session.conf.Logger. + server.Logger. WithFields(logrus.Fields{"session": session.RequestorToken}). Debug("Session marked active, deletion delayed") } -func (session *session) setStatus(status irma.ServerStatus) { - session.conf.Logger. +func (session *sessionData) setStatus(status irma.ServerStatus, conf *server.Configuration) { + conf.Logger. WithFields(logrus.Fields{"session": session.RequestorToken, "status": status}). Info("Session status updated") session.Status = status session.Result.Status = status - session.onStatusChange() -} - -func (session *session) onStatusChange() { - // Send status update to all listener channels - for _, statusChan := range session.statusChannels { - statusChan <- session.Status - if session.Status.Finished() { - close(statusChan) - } - } // Execute callback and handler if status is Finished if session.Status.Finished() { - session.doResultCallback() - - if session.handler != nil { - handler := session.handler - session.handler = nil - go handler(session.Result) - } + session.doResultCallback(conf) } - - // Send updates in case SSE is used - if session.sse == nil { - return - } - session.sse.SendMessage("session/"+string(session.ClientToken), - sse.SimpleMessage(fmt.Sprintf(`"%s"`, session.Status)), - ) - session.sse.SendMessage("session/"+string(session.RequestorToken), - sse.SimpleMessage(fmt.Sprintf(`"%s"`, session.Status)), - ) - frontendstatus, _ := json.Marshal(irma.FrontendSessionStatus{Status: session.Status, NextSession: session.Next}) - session.sse.SendMessage("frontendsession/"+string(session.ClientToken), - sse.SimpleMessage(string(frontendstatus)), - ) } -func (session *session) doResultCallback() { +func (session *sessionData) doResultCallback(conf *server.Configuration) { url := session.Rrequest.Base().CallbackURL if url == "" { return } server.DoResultCallback(url, session.Result, - session.conf.JwtIssuer, + conf.JwtIssuer, session.Rrequest.Base().ResultJwtValidity, - session.conf.JwtRSAPrivateKey, + conf.JwtRSAPrivateKey, ) } // Checks whether requested options are valid in the current session context. -func (session *session) updateFrontendOptions(request *irma.FrontendOptionsRequest) (*irma.SessionOptions, error) { +func (session *sessionData) updateFrontendOptions(request *irma.FrontendOptionsRequest) (*irma.SessionOptions, error) { if session.Status != irma.ServerStatusInitialized { return nil, errors.New("Frontend options can only be updated when session is in initialized state") } @@ -113,22 +81,22 @@ func (session *session) updateFrontendOptions(request *irma.FrontendOptionsReque } // Complete the pairing process of frontend and irma client -func (session *session) pairingCompleted() error { +func (session *sessionData) pairingCompleted(conf *server.Configuration) error { if session.Status == irma.ServerStatusPairing { - session.setStatus(irma.ServerStatusConnected) + session.setStatus(irma.ServerStatusConnected, conf) return nil } return errors.New("Pairing was not enabled") } -func (session *session) fail(err server.Error, message string) *irma.RemoteError { +func (session *sessionData) fail(err server.Error, message string, conf *server.Configuration) *irma.RemoteError { rerr := server.RemoteError(err, message) session.Result = &server.SessionResult{Err: rerr, Token: session.RequestorToken, Status: irma.ServerStatusCancelled, Type: session.Action} - session.setStatus(irma.ServerStatusCancelled) + session.setStatus(irma.ServerStatusCancelled, conf) return rerr } -func (session *session) chooseProtocolVersion(minClient, maxClient *irma.ProtocolVersion) (*irma.ProtocolVersion, error) { +func (session *sessionData) chooseProtocolVersion(minClient, maxClient *irma.ProtocolVersion) (*irma.ProtocolVersion, error) { minSessionProtocolVersion := minSecureProtocolVersion if AcceptInsecureProtocolVersions { // Set minimum supported version to 2.5 if condiscon compatibility is required @@ -137,7 +105,7 @@ func (session *session) chooseProtocolVersion(minClient, maxClient *irma.Protoco minSessionProtocolVersion = &irma.ProtocolVersion{Major: 2, Minor: 5} } // Set minimum to 2.6 if nonrevocation is required - if len(session.request.Base().Revocation) > 0 { + if len(session.Rrequest.SessionRequest().Base().Revocation) > 0 { minSessionProtocolVersion = &irma.ProtocolVersion{Major: 2, Minor: 6} } // Set minimum to 2.7 if chained session are used @@ -166,7 +134,7 @@ const retryTimeLimit = 10 * time.Second // - the body is not empty // - last time was not more than 10 seconds ago (retryablehttp client gives up before this) // - the session status is what it is expected to be when receiving the request for a second time. -func (session *session) checkCache(endpoint string, message []byte) (int, []byte) { +func (session *sessionData) checkCache(endpoint string, message []byte) (int, []byte) { if session.ResponseCache.Endpoint != endpoint || len(session.ResponseCache.Response) == 0 || session.ResponseCache.SessionStatus != session.Status || @@ -180,15 +148,15 @@ func (session *session) checkCache(endpoint string, message []byte) (int, []byte // Issuance helpers -func (session *session) computeWitness(sk *gabikeys.PrivateKey, cred *irma.CredentialRequest) (*revocation.Witness, error) { +func (session *sessionData) computeWitness(sk *gabikeys.PrivateKey, cred *irma.CredentialRequest, conf *server.Configuration) (*revocation.Witness, error) { id := cred.CredentialTypeID - credtyp := session.conf.IrmaConfiguration.CredentialTypes[id] - if !credtyp.RevocationSupported() || !session.request.Base().RevocationSupported() { + credtyp := conf.IrmaConfiguration.CredentialTypes[id] + if !credtyp.RevocationSupported() || !session.Rrequest.SessionRequest().Base().RevocationSupported() { return nil, nil } // ensure the client always gets an up to date nonrevocation witness - rs := session.conf.IrmaConfiguration.Revocation + rs := conf.IrmaConfiguration.Revocation if err := rs.SyncDB(id); err != nil { return nil, err } @@ -222,11 +190,11 @@ func (session *session) computeWitness(sk *gabikeys.PrivateKey, cred *irma.Crede return witness, nil } -func (session *session) computeAttributes( - sk *gabikeys.PrivateKey, cred *irma.CredentialRequest, +func (session *sessionData) computeAttributes( + sk *gabikeys.PrivateKey, cred *irma.CredentialRequest, conf *server.Configuration, ) ([]*big.Int, *revocation.Witness, error) { id := cred.CredentialTypeID - witness, err := session.computeWitness(sk, cred) + witness, err := session.computeWitness(sk, cred, conf) if err != nil { return nil, nil, err } @@ -236,7 +204,7 @@ func (session *session) computeAttributes( } issuedAt := time.Now() - attributes, err := cred.AttributeList(session.conf.IrmaConfiguration, 0x03, nonrevAttr, issuedAt) + attributes, err := cred.AttributeList(conf.IrmaConfiguration, 0x03, nonrevAttr, issuedAt) if err != nil { return nil, nil, err } @@ -250,7 +218,7 @@ func (session *session) computeAttributes( Issued: issuedAt.UnixNano(), ValidUntil: attributes.Expiry().UnixNano(), } - err = session.conf.IrmaConfiguration.Revocation.SaveIssuanceRecord(id, issrecord, sk) + err = conf.IrmaConfiguration.Revocation.SaveIssuanceRecord(id, issrecord, sk) if err != nil { return nil, nil, err } @@ -311,7 +279,7 @@ func (s *Server) validateIssuanceRequest(request *irma.IssuanceRequest) error { return nil } -func (session *session) getProofP(commitments *irma.IssueCommitmentMessage, scheme irma.SchemeManagerIdentifier) (*gabi.ProofP, error) { +func (session *sessionData) getProofP(commitments *irma.IssueCommitmentMessage, scheme irma.SchemeManagerIdentifier, conf *server.Configuration) (*gabi.ProofP, error) { if session.KssProofs == nil { session.KssProofs = make(map[irma.SchemeManagerIdentifier]*gabi.ProofP) } @@ -321,12 +289,12 @@ func (session *session) getProofP(commitments *irma.IssueCommitmentMessage, sche if !contains { return nil, errors.Errorf("no keyshare proof included for scheme %s", scheme.Name()) } - session.conf.Logger.Debug("Parsing keyshare ProofP JWT: ", str) + conf.Logger.Debug("Parsing keyshare ProofP JWT: ", str) claims := &struct { jwt.StandardClaims ProofP *gabi.ProofP }{} - token, err := jwt.ParseWithClaims(str, claims, session.conf.IrmaConfiguration.KeyshareServerKeyFunc(scheme)) + token, err := jwt.ParseWithClaims(str, claims, conf.IrmaConfiguration.KeyshareServerKeyFunc(scheme)) if err != nil { return nil, err } @@ -339,7 +307,7 @@ func (session *session) getProofP(commitments *irma.IssueCommitmentMessage, sche return session.KssProofs[scheme], nil } -func (session *session) getClientRequest() (*irma.ClientSessionRequest, error) { +func (session *sessionData) getClientRequest() (*irma.ClientSessionRequest, error) { info := irma.ClientSessionRequest{ LDContext: irma.LDContextClientSessionRequest, ProtocolVersion: session.Version, @@ -356,11 +324,12 @@ func (session *session) getClientRequest() (*irma.ClientSessionRequest, error) { return &info, nil } -func (session *session) getRequest() (irma.SessionRequest, error) { +func (session *sessionData) getRequest() (irma.SessionRequest, error) { + req := session.Rrequest.SessionRequest() // In case of issuance requests, strip revocation keys from []CredentialRequest - isreq, issuing := session.request.(*irma.IssuanceRequest) + isreq, issuing := req.(*irma.IssuanceRequest) if !issuing { - return session.request, nil + return req, nil } cpy, err := copyObject(isreq) if err != nil { @@ -373,33 +342,37 @@ func (session *session) getRequest() (irma.SessionRequest, error) { return cpy.(*irma.IssuanceRequest), nil } -func (session *session) updateAndUnlock() error { - err := session.sessions.update(session) +func (session *sessionData) hash() [32]byte { + // Note: This marshalling does not consider the order of the `map[irma.SchemeManagerIdentifier]*gabi.ProofP` items. + sessionJSON, err := json.Marshal(session) if err != nil { - return err + panic(err) } - session.sessions.unlock(session) - return nil + return sha256.Sum256(sessionJSON) } -func (s *sessionData) hash() [32]byte { - // Note: This marshalling does not consider the order of the `map[irma.SchemeManagerIdentifier]*gabi.ProofP` items. - sessionJSON, err := json.Marshal(s) - if err != nil { - panic(err) +func (session *sessionData) timeout(conf *server.Configuration) time.Duration { + maxSessionDuration := time.Duration(conf.MaxSessionLifetime) * time.Minute + if session.Status == irma.ServerStatusInitialized && session.Rrequest.Base().ClientTimeout != 0 { + maxSessionDuration = time.Duration(session.Rrequest.Base().ClientTimeout) * time.Second + } else if session.Status.Finished() { + maxSessionDuration = 0 } + return maxSessionDuration - time.Since(session.LastActive) +} - return sha256.Sum256(sessionJSON) +func (session *sessionData) ttl(conf *server.Configuration) time.Duration { + return session.timeout(conf) + time.Duration(conf.SessionResultLifetime)*time.Minute } // UnmarshalJSON unmarshals sessionData. -func (s *sessionData) UnmarshalJSON(data []byte) error { - type rawSessionData sessionData +func (session *sessionData) UnmarshalJSON(data []byte) error { + type rawSession sessionData var temp struct { Rrequest json.RawMessage `json:",omitempty"` - rawSessionData + rawSession } if err := json.Unmarshal(data, &temp); err != nil { @@ -410,19 +383,19 @@ func (s *sessionData) UnmarshalJSON(data []byte) error { return errors.Errorf("temp.Rrequest == nil: %d \n", temp.Rrequest) } - *s = sessionData(temp.rawSessionData) + *session = sessionData(temp.rawSession) // unmarshal Rrequest - switch s.Action { + switch session.Action { case "issuing": - s.Rrequest = &irma.IdentityProviderRequest{} + session.Rrequest = &irma.IdentityProviderRequest{} case "disclosing": - s.Rrequest = &irma.ServiceProviderRequest{} + session.Rrequest = &irma.ServiceProviderRequest{} case "signing": - s.Rrequest = &irma.SignatureRequestorRequest{} + session.Rrequest = &irma.SignatureRequestorRequest{} } - return json.Unmarshal(temp.Rrequest, s.Rrequest) + return json.Unmarshal(temp.Rrequest, session.Rrequest) } // Other @@ -524,7 +497,7 @@ func errorWriter(err *irma.RemoteError, writer func(w http.ResponseWriter, objec func (s *Server) frontendMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - session := r.Context().Value("session").(*session) + session := r.Context().Value("session").(*sessionData) frontendAuth := irma.FrontendAuthorization(r.Header.Get(irma.AuthorizationHeader)) if frontendAuth != session.FrontendAuth { @@ -537,7 +510,7 @@ func (s *Server) frontendMiddleware(next http.Handler) http.Handler { func (s *Server) cacheMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - session := r.Context().Value("session").(*session) + session := r.Context().Value("session").(*sessionData) // Read r.Body, and then replace with a fresh ReadCloser for the next handler message, err := io.ReadAll(r.Body) @@ -579,44 +552,37 @@ func (s *Server) sessionMiddleware(next http.Handler) http.Handler { return } - session, err := s.sessions.clientGet(token) - if err != nil { - if _, ok := err.(*UnknownSessionError); ok { - server.WriteError(w, server.ErrorSessionUnknown, "") - } else { - server.WriteError(w, server.ErrorInternal, "") + if err := s.sessions.clientTransaction(token, func(session *sessionData) error { + expectedHost := session.Rrequest.SessionRequest().Base().Host + if expectedHost != "" && expectedHost != r.Host { + server.WriteError(w, server.ErrorUnauthorized, "Host mismatch") + return nil } - return - } - defer func() { - err := session.updateAndUnlock() - if err != nil { - // Error already logged in update method. - server.WriteError(w, server.ErrorInternal, "") - return - } + next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), "session", session))) + // Write session result to context for irmac.go functions. result := session.Result - r := r.Context().Value("sessionresult") - if r != nil { - *r.(*server.SessionResult) = *result + resultValue := r.Context().Value("sessionresult") + if resultValue != nil { + *resultValue.(*server.SessionResult) = *result } - }() - expectedHost := session.request.Base().Host - if expectedHost != "" && expectedHost != r.Host { - server.WriteError(w, server.ErrorUnauthorized, "Host mismatch") + return nil + }); err != nil { + if _, ok := err.(*UnknownSessionError); ok { + server.WriteError(w, server.ErrorSessionUnknown, "") + } else { + server.WriteError(w, server.ErrorInternal, "") + } return } - - next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), "session", session))) }) } func (s *Server) pairingMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - session := r.Context().Value("session").(*session) + session := r.Context().Value("session").(*sessionData) if session.Status == irma.ServerStatusPairing { server.WriteError(w, server.ErrorPairingRequired, "") diff --git a/server/irmaserver/sessions.go b/server/irmaserver/sessions.go index 4462e4352..1afc39073 100644 --- a/server/irmaserver/sessions.go +++ b/server/irmaserver/sessions.go @@ -10,9 +10,6 @@ import ( "github.com/go-errors/errors" - "github.com/bsm/redislock" - - "github.com/alexandrevicenzi/go-sse" "github.com/go-redis/redis/v8" "github.com/privacybydesign/gabi" "github.com/privacybydesign/gabi/big" @@ -23,21 +20,6 @@ import ( "github.com/sirupsen/logrus" ) -type session struct { - sync.Mutex - sse *sse.Server - locked bool - lock *redislock.Lock - hashBefore *[32]byte - sessions sessionStore - conf *server.Configuration - request irma.SessionRequest - statusChannels []chan irma.ServerStatus - handler server.SessionHandler - - sessionData -} - type sessionData struct { Action irma.Action RequestorToken irma.RequestorToken @@ -66,25 +48,33 @@ type responseCache struct { } type sessionStore interface { - get(token irma.RequestorToken) (*session, error) - clientGet(token irma.ClientToken) (*session, error) - add(session *session) error - update(session *session) error - unlock(session *session) + add(*sessionData) error + transaction(irma.RequestorToken, func(*sessionData) error) error + clientTransaction(irma.ClientToken, func(*sessionData) error) error + subscribeStatusChanges(irma.RequestorToken) (chan statusChange, error) stop() } +type statusChange struct { + irma.FrontendSessionStatus + Timeout time.Duration +} + type memorySessionStore struct { sync.RWMutex - conf *server.Configuration + conf *server.Configuration + requestor map[irma.RequestorToken]*memorySessionData + client map[irma.ClientToken]*memorySessionData + statusChannels map[irma.RequestorToken][]chan statusChange +} - requestor map[irma.RequestorToken]*session - client map[irma.ClientToken]*session +type memorySessionData struct { + sync.Mutex + *sessionData } type redisSessionStore struct { - client *redis.Client - locker *redislock.Client + client *server.RedisClient conf *server.Configuration } @@ -115,7 +105,6 @@ const ( maxLockRetryTime = 2 * time.Second requestorTokenLookupPrefix = "token:" clientTokenLookupPrefix = "session:" - lockPrefix = "lock:" ) var ( @@ -129,67 +118,111 @@ var ( minFrontendProtocolVersion = irma.NewVersion(1, 0) maxFrontendProtocolVersion = irma.NewVersion(1, 1) - - lockingRetryOptions = &redislock.Options{RetryStrategy: redislock.ExponentialBackoff(minLockRetryTime, maxLockRetryTime)} ) -func (s *memorySessionStore) get(t irma.RequestorToken) (*session, error) { +func (s *memorySessionStore) add(session *sessionData) error { + s.Lock() + defer s.Unlock() + memSes := &memorySessionData{sessionData: session} + s.requestor[session.RequestorToken] = memSes + s.client[session.ClientToken] = memSes + return nil +} + +func (s *memorySessionStore) transaction(t irma.RequestorToken, handler func(session *sessionData) error) error { s.RLock() ses := s.requestor[t] s.RUnlock() - if ses != nil { - ses.Lock() - ses.locked = true + if ses == nil { + return server.LogError(&UnknownSessionError{t, ""}) + } - return ses, nil - } else { - return nil, server.LogError(&UnknownSessionError{t, ""}) + ses.Lock() + copy, err := copyObject(ses.sessionData) + ses.Unlock() + if err != nil { + return err } + sesCopy := copy.(*sessionData) + + if err := s.handleTransaction(sesCopy, handler); err != nil { + return err + } + + s.Lock() + defer s.Unlock() + s.requestor[t].sessionData = sesCopy + return nil } -func (s *memorySessionStore) clientGet(t irma.ClientToken) (*session, error) { +func (s *memorySessionStore) clientTransaction(t irma.ClientToken, handler func(session *sessionData) error) error { s.RLock() ses := s.client[t] s.RUnlock() - if ses != nil { - ses.Lock() - ses.locked = true + if ses == nil { + return server.LogError(&UnknownSessionError{"", t}) + } - return ses, nil - } else { - return nil, server.LogError(&UnknownSessionError{"", t}) + ses.Lock() + copy, err := copyObject(ses.sessionData) + ses.Unlock() + if err != nil { + return err + } + sesCopy := copy.(*sessionData) + + if err := s.handleTransaction(sesCopy, handler); err != nil { + return err } -} -func (s *memorySessionStore) add(session *session) error { s.Lock() defer s.Unlock() - s.requestor[session.RequestorToken] = session - s.client[session.ClientToken] = session + s.client[t].sessionData = sesCopy return nil } -func (s *memorySessionStore) update(_ *session) error { +func (s *memorySessionStore) handleTransaction(ses *sessionData, handler func(session *sessionData) error) error { + prevStatus := ses.Status + + if !ses.Status.Finished() && ses.timeout(s.conf) <= 0 { + s.conf.Logger.WithFields(logrus.Fields{"session": ses.RequestorToken}).Info("Session expired") + ses.setStatus(irma.ServerStatusTimeout, s.conf) + } + + if err := handler(ses); err != nil { + return err + } + + if prevStatus != ses.Status { + for _, channel := range s.statusChannels[ses.RequestorToken] { + channel <- statusChange{ + FrontendSessionStatus: irma.FrontendSessionStatus{ + Status: ses.Status, + NextSession: ses.Next, + }, + Timeout: ses.timeout(s.conf), + } + } + } return nil } -func (s *memorySessionStore) unlock(session *session) { - if session.locked { - session.locked = false - session.Unlock() - } +func (s *memorySessionStore) subscribeStatusChanges(token irma.RequestorToken) (chan statusChange, error) { + statusChan := make(chan statusChange, 4) + s.Lock() + defer s.Unlock() + s.statusChannels[token] = append(s.statusChannels[token], statusChan) + return statusChan, nil } func (s *memorySessionStore) stop() { s.Lock() defer s.Unlock() for _, session := range s.requestor { - if session.sse != nil { - session.sse.CloseChannel("session/" + string(session.RequestorToken)) - session.sse.CloseChannel("session/" + string(session.ClientToken)) - session.sse.CloseChannel("frontendsession/" + string(session.ClientToken)) + for _, channel := range s.statusChannels[session.RequestorToken] { + close(channel) } } } @@ -198,180 +231,137 @@ func (s *memorySessionStore) deleteExpired() { // First check which sessions have expired // We don't need a write lock for this yet, so postpone that for actual deleting s.RLock() - toCheck := make(map[irma.RequestorToken]*session, len(s.requestor)) - for token, session := range s.requestor { - toCheck[token] = session + var toCheck []irma.RequestorToken + for token := range s.requestor { + toCheck = append(toCheck, token) } s.RUnlock() expired := make([]irma.RequestorToken, 0, len(toCheck)) - for token, session := range toCheck { - session.Lock() - - timeout := time.Duration(s.conf.MaxSessionLifetime) * time.Minute - if session.Status == irma.ServerStatusInitialized && session.Rrequest.Base().ClientTimeout != 0 { - timeout = time.Duration(session.Rrequest.Base().ClientTimeout) * time.Second - } else if session.Status.Finished() { - timeout = time.Duration(s.conf.SessionResultLifetime) * time.Minute - } - - if session.LastActive.Add(timeout).Before(time.Now()) { - if !session.Status.Finished() { - s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Session expired") - session.markAlive() - session.setStatus(irma.ServerStatusTimeout) - } else { + for _, token := range toCheck { + s.transaction(token, func(session *sessionData) error { + if session.ttl(s.conf) <= 0 { s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Deleting session") expired = append(expired, token) } - } - session.Unlock() + return nil + }) } // Using a write lock, delete the expired sessions s.Lock() for _, token := range expired { session := s.requestor[token] - if session.sse != nil { - session.sse.CloseChannel("session/" + string(session.RequestorToken)) - session.sse.CloseChannel("session/" + string(session.ClientToken)) - session.sse.CloseChannel("frontendsession/" + string(session.ClientToken)) - } delete(s.client, session.ClientToken) delete(s.requestor, token) } s.Unlock() } -func (s *redisSessionStore) get(t irma.RequestorToken) (*session, error) { +func (s *redisSessionStore) add(session *sessionData) error { + sessionJSON, err := json.Marshal(session) + if err != nil { + return server.LogError(err) + } + + ttl := session.ttl(s.conf) + conn := s.client.Conn(context.Background()) + if err := conn.Set(context.Background(), requestorTokenLookupPrefix+string(session.RequestorToken), string(session.ClientToken), ttl).Err(); err != nil { + return logAsRedisError(err) + } + if err := conn.Set(context.Background(), clientTokenLookupPrefix+string(session.ClientToken), sessionJSON, ttl).Err(); err != nil { + return logAsRedisError(err) + } + + if s.client.FailoverMode { + if err := s.client.Wait(context.Background(), 1, time.Second).Err(); err != nil { + return logAsRedisError(err) + } + } + + s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("session added or updated in Redis datastore") + return nil +} + +func (s *redisSessionStore) transaction(t irma.RequestorToken, handler func(session *sessionData) error) error { val, err := s.client.Get(context.Background(), requestorTokenLookupPrefix+string(t)).Result() if err == redis.Nil { - return nil, server.LogError(&UnknownSessionError{t, ""}) + return server.LogError(&UnknownSessionError{t, ""}) } else if err != nil { - return nil, logAsRedisError(err) + return logAsRedisError(err) } clientToken, err := irma.ParseClientToken(val) if err != nil { - return nil, logAsRedisError(err) + return logAsRedisError(err) } s.conf.Logger.WithFields(logrus.Fields{"session": t, "clientToken": clientToken}).Debug("clientToken found in Redis datastore") - return s.clientGet(clientToken) + return s.clientTransaction(clientToken, handler) } -func (s *redisSessionStore) clientGet(t irma.ClientToken) (*session, error) { - session := &session{ - sessions: s, - conf: s.conf, - } +func (s *redisSessionStore) clientTransaction(t irma.ClientToken, handler func(session *sessionData) error) error { + if err := s.client.Watch(context.Background(), func(tx *redis.Tx) error { + getResult := tx.Get(context.Background(), clientTokenLookupPrefix+string(t)) + if getResult.Err() == redis.Nil { + // Both session and error need to be returned. The session will already be locked and needs to + // be passed along, so it can be unlocked later. + return server.LogError(&UnknownSessionError{"", t}) + } else if getResult.Err() != nil { + return logAsRedisError(getResult.Err()) + } - // lock via clientToken since requestorToken first fetches clientToken en then comes here, this is fine - lock, err := s.locker.Obtain(context.Background(), lockPrefix+string(t), maxLockLifetime, lockingRetryOptions) - if err != nil { - // It is possible that the session is already locked. However, it should not happen often. - // If you get the redislock.ErrNotObtained error often, you should investigate why. - return nil, logAsRedisError(err) - } - session.locked = true - session.lock = lock - s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("session locked successfully") + session := &sessionData{} + if err := json.Unmarshal([]byte(getResult.Val()), &session); err != nil { + return logAsRedisError(err) + } - // get the session data - val, err := s.client.Get(context.Background(), clientTokenLookupPrefix+string(t)).Result() - if err == redis.Nil { - // Both session and error need to be returned. The session will already be locked and needs to - // be passed along, so it can be unlocked later. - return session, server.LogError(&UnknownSessionError{"", t}) - } else if err != nil { - return session, logAsRedisError(err) - } + s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("Session received from Redis datastore") - if err := json.Unmarshal([]byte(val), &session.sessionData); err != nil { - return session, logAsRedisError(err) - } - session.request = session.Rrequest.SessionRequest() - s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("Session received from Redis datastore") - - // hashing the current session data needs to take place before the timeout check to detect all changes! - hash := session.sessionData.hash() - session.hashBefore = &hash - - // timeout check - lifetime := time.Duration(s.conf.MaxSessionLifetime) * time.Minute - if session.LastActive.Add(lifetime).Before(time.Now()) && !session.Status.Finished() { - s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Session expired") - session.markAlive() - session.setStatus(irma.ServerStatusTimeout) - } + // Hashing the current session data needs to take place before the timeout check to detect all changes! + hashBefore := session.hash() - return session, nil -} + // Timeout check + if !session.Status.Finished() && session.timeout(s.conf) <= 0 { + s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Session expired") + session.setStatus(irma.ServerStatusTimeout, s.conf) + } -func (s *redisSessionStore) add(session *session) error { - sessionLifetime := time.Duration(s.conf.MaxSessionLifetime) * time.Minute - resultLifetime := time.Duration(s.conf.SessionResultLifetime) * time.Minute - // After the timeout, the session will automatically be removed. Therefore, the timeout needs to - // already include the session result lifetime. In this way, when the session expires, the session - // will be preserved until session result lifetime ends. - timeout := sessionLifetime + resultLifetime - if session.Status == irma.ServerStatusInitialized && session.Rrequest.Base().ClientTimeout != 0 { - timeout = time.Duration(session.Rrequest.Base().ClientTimeout) * time.Second - } else if session.Status.Finished() { - timeout = resultLifetime - } + if err := handler(session); err != nil { + return err + } - sessionJSON, err := json.Marshal(session.sessionData) - if err != nil { - return server.LogError(err) - } + // Check if the session has changed + hashAfter := session.hash() + if hashBefore == hashAfter { + return nil + } - err = s.client.Set(context.Background(), requestorTokenLookupPrefix+string(session.sessionData.RequestorToken), string(session.sessionData.ClientToken), timeout).Err() - if err != nil { - return logAsRedisError(err) - } - err = s.client.Set(context.Background(), clientTokenLookupPrefix+string(session.sessionData.ClientToken), sessionJSON, timeout).Err() - if err != nil { - return logAsRedisError(err) - } + // If the session has changed, update it in Redis + sessionJSON, err := json.Marshal(session) + if err != nil { + return server.LogError(err) + } - s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("session added or updated in Redis datastore") - return nil -} + err = tx.Set(context.Background(), clientTokenLookupPrefix+string(t), sessionJSON, 0).Err() + if err != nil { + return logAsRedisError(err) + } -func (s *redisSessionStore) update(session *session) error { - hash := session.hash() - if session.hashBefore == nil || *session.hashBefore == hash { - // if nothing changed, updating is not necessary + if s.client.FailoverMode { + if err := tx.Wait(context.Background(), 1, time.Second).Err(); err != nil { + return logAsRedisError(err) + } + } return nil - } - - // Time passes between acquiring the lock and writing to Redis. Check before write action that lock is still valid. - if session.lock == nil { - return logAsRedisError(errors.Errorf("lock is not set for session with requestorToken %s", session.RequestorToken)) - } else if ttl, err := session.lock.TTL(context.Background()); err != nil { + }); err != nil { return logAsRedisError(err) - } else if ttl == 0 { - return logAsRedisError(errors.Errorf("no session lock available for session with requestorToken %s", session.RequestorToken)) } - return s.add(session) + return nil } -func (s *redisSessionStore) unlock(session *session) { - if !session.locked { - return - } - err := session.lock.Release(context.Background()) - if err == redislock.ErrLockNotHeld { - s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Redis lock could not be released as the lock was not held") - } else if err != nil { - // The Redis lock will be set free eventually after the `maxLockLifetime`. So it is safe to - // ignore this error. - _ = logAsRedisError(err) - return - } - session.locked = false - s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("session unlocked successfully") +func (s *redisSessionStore) subscribeStatusChanges(token irma.RequestorToken) (chan statusChange, error) { + return nil, errors.New("not implemented") } func (s *redisSessionStore) stop() { @@ -384,7 +374,7 @@ func (s *redisSessionStore) stop() { var one *big.Int = big.NewInt(1) -func (s *Server) newSession(action irma.Action, request irma.RequestorRequest, disclosed irma.AttributeConDisCon, FrontendAuth irma.FrontendAuthorization) (*session, error) { +func (s *Server) newSession(action irma.Action, request irma.RequestorRequest, disclosed irma.AttributeConDisCon, FrontendAuth irma.FrontendAuthorization) (*sessionData, error) { clientToken := irma.ClientToken(common.NewSessionToken()) requestorToken := irma.RequestorToken(common.NewSessionToken()) if len(FrontendAuth) == 0 { @@ -400,7 +390,7 @@ func (s *Server) newSession(action irma.Action, request irma.RequestorRequest, d } } - sd := sessionData{ + ses := &sessionData{ Action: action, Rrequest: request, LastActive: time.Now(), @@ -420,13 +410,6 @@ func (s *Server) newSession(action irma.Action, request irma.RequestorRequest, d FrontendAuth: FrontendAuth, ImplicitDisclosure: disclosed, } - ses := &session{ - sessionData: sd, - sessions: s.sessions, - sse: s.serverSentEvents, - conf: s.conf, - request: request.SessionRequest(), - } s.conf.Logger.WithFields(logrus.Fields{"session": ses.RequestorToken}).Debug("New session started") nonce, _ := gabi.GenerateNonce() diff --git a/server/irmaserver/sessions_test.go b/server/irmaserver/sessions_test.go index 2b7a7af0f..a3bbce4af 100644 --- a/server/irmaserver/sessions_test.go +++ b/server/irmaserver/sessions_test.go @@ -79,12 +79,16 @@ func TestMemoryStoreNoDeadlock(t *testing.T) { session, err := s.newSession(irma.ActionDisclosing, req, nil, "") require.NoError(t, err) - session.Lock() + memSessions, ok := s.sessions.(*memorySessionStore) + require.True(t, ok) + memSession := memSessions.requestor[session.RequestorToken] + + memSession.Lock() deletingCompleted := false addingCompleted := false // Make sure the deleting continues on completion such that the test itself will not hang. defer func() { - session.Unlock() + memSession.Unlock() time.Sleep(100 * time.Millisecond) require.True(t, deletingCompleted) }() diff --git a/server/keyshare/keyshareserver/session.go b/server/keyshare/keyshareserver/session.go index 027514249..72c48f934 100644 --- a/server/keyshare/keyshareserver/session.go +++ b/server/keyshare/keyshareserver/session.go @@ -1,10 +1,11 @@ package keyshareserver import ( - "github.com/privacybydesign/gabi" "sync" "time" + "github.com/privacybydesign/gabi" + irma "github.com/privacybydesign/irmago" ) diff --git a/server/keyshare/myirmaserver/server.go b/server/keyshare/myirmaserver/server.go index c80c40a96..fd8d61553 100644 --- a/server/keyshare/myirmaserver/server.go +++ b/server/keyshare/myirmaserver/server.go @@ -34,6 +34,22 @@ type Server struct { var errUnknownEmail = errors.New("Email not associated with account") func New(conf *Configuration) (*Server, error) { + var store sessionStore + switch conf.Configuration.StoreType { + case "": + fallthrough // no specification defaults to the memory session store + case "memory": + store = newMemorySessionStore() + case "redis": + cl, err := conf.Configuration.RedisClient() + if err != nil { + return nil, err + } + store = &redisSessionStore{client: cl} + default: + return nil, errors.New("unsupported session store type") + } + irmaserv, err := irmaserver.New(conf.Configuration) if err != nil { return nil, err @@ -46,7 +62,7 @@ func New(conf *Configuration) (*Server, error) { s := &Server{ conf: conf, irmaserv: irmaserv, - store: newMemorySessionStore(time.Duration(conf.SessionLifetime) * time.Second), + store: store, db: conf.DB, scheduler: gocron.NewScheduler(time.UTC), } @@ -128,35 +144,37 @@ func (s *Server) Handler() http.Handler { } func (s *Server) handleCheckSession(w http.ResponseWriter, r *http.Request) { - session := s.sessionFromCookie(r) - if session == nil { - server.WriteString(w, "expired") - return - } - - session.Lock() - defer session.Unlock() + var msg string + err := s.sessionFromCookie(r, func(session *session) error { + if session.LoginSessionToken != "" { + if err := s.processLoginIrmaSessionResult(r.Context(), session); err != nil { + return err + } + } - var ( - err server.Error - msg string - ) - if session.loginSessionToken != "" { - err, msg = s.processLoginIrmaSessionResult(r.Context(), session) - } + if session.UserID == nil { + msg = "expired" + } else { + msg = "ok" + } + return nil - if err != (server.Error{}) { - server.WriteError(w, err, msg) - } else if session.userID == nil { - // Errors matter more than expired status if we have them - server.WriteString(w, "expired") - } else { - server.WriteString(w, "ok") + }) + // If the session is unknown, we return "expired" to the frontend. This is not an error. + if err == errUnknownSession { + msg = "expired" + } else if rerr, ok := err.(*irma.RemoteError); ok { + server.WriteResponse(w, nil, rerr) + return + } else if err != nil { + server.WriteError(w, server.ErrorInternal, err.Error()) + return } + server.WriteString(w, msg) } func (s *Server) sendDeleteEmails(ctx context.Context, session *session) error { - user, err := s.db.user(ctx, *session.userID) + user, err := s.db.user(ctx, *session.UserID) if err != nil { s.conf.Logger.WithField("error", err).Error("Could not fetch user information") return err @@ -186,7 +204,7 @@ func (s *Server) sendDeleteEmails(ctx context.Context, session *session) error { } if revalidation { - if err := s.db.scheduleEmailRevalidation(ctx, *session.userID, email.Email, delay); err != nil { + if err := s.db.scheduleEmailRevalidation(ctx, *session.UserID, email.Email, delay); err != nil { s.conf.Logger.WithField("error", err).Error("Could not schedule email revalidation") return err } @@ -196,7 +214,7 @@ func (s *Server) sendDeleteEmails(ctx context.Context, session *session) error { // When not all email addresses are valid and revalidation is enabled the invalid email addresses are // already scheduled for revalidation we block the user account for 5 days if len(addrs) < len(user.Emails) && revalidation { - if err := s.db.setPinBlockDate(ctx, *session.userID, delay); err != nil { + if err := s.db.setPinBlockDate(ctx, *session.UserID, delay); err != nil { s.conf.Logger.WithField("error", err).Error("Could not update pin block date") return err } @@ -242,14 +260,14 @@ func (s *Server) handleDeleteUser(w http.ResponseWriter, r *http.Request) { } // Then remove user - err := s.db.scheduleUserRemoval(r.Context(), *session.userID, 24*time.Hour*time.Duration(s.conf.DeleteDelay)) + err := s.db.scheduleUserRemoval(r.Context(), *session.UserID, 24*time.Hour*time.Duration(s.conf.DeleteDelay)) if err != nil { s.conf.Logger.WithField("error", err).Error("Problem removing user") keyshare.WriteError(w, err) return } - s.logoutUser(w, r) + s.logoutUser(w, session) w.WriteHeader(http.StatusNoContent) } @@ -359,8 +377,15 @@ func (s *Server) processTokenLogin(ctx context.Context, request tokenLoginReques return "", err } - session := s.store.create() - session.userID = &id + session := session{ + Token: common.NewSessionToken(), + UserID: &id, + Expiry: time.Now().Add(time.Duration(s.conf.SessionLifetime) * time.Second), + } + + if err := s.store.add(ctx, session); err != nil { + return "", err + } err = s.db.setSeen(ctx, id) if err != nil { @@ -368,7 +393,7 @@ func (s *Server) processTokenLogin(ctx context.Context, request tokenLoginReques // not relevant for frontend, so ignore beyond log. } - return session.token, nil + return session.Token, nil } func (s *Server) handleTokenLogin(w http.ResponseWriter, r *http.Request) { @@ -394,51 +419,48 @@ func (s *Server) handleTokenLogin(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } -func (s *Server) processLoginIrmaSessionResult(ctx context.Context, session *session) (server.Error, string) { - result, err := s.irmaserv.GetSessionResult(session.loginSessionToken) +func (s *Server) processLoginIrmaSessionResult(ctx context.Context, session *session) error { + result, err := s.irmaserv.GetSessionResult(session.LoginSessionToken) if err != nil { - return server.ErrorInternal, "" + return server.RemoteError(server.ErrorInternal, "") } if result == nil { - session.loginSessionToken = "" - return server.ErrorInternal, "unknown login session" + session.LoginSessionToken = "" + return server.RemoteError(server.ErrorInternal, "unknown login session") } if result.Status != irma.ServerStatusDone { // Ignore incomplete attempts, frontend handles these. - return server.Error{}, "" + return nil } - session.loginSessionToken = "" + session.LoginSessionToken = "" if result.ProofStatus != irma.ProofStatusValid { s.conf.Logger.Info("received invalid login attribute") - return server.ErrorInvalidProofs, "" + return server.RemoteError(server.ErrorInvalidProofs, "") } username := *result.Disclosed[0][0].RawValue id, err := s.db.userIDByUsername(ctx, username) if err == keyshare.ErrUserNotFound { - return server.ErrorUserNotRegistered, "" + return server.RemoteError(server.ErrorUserNotRegistered, "") } else if err != nil { s.conf.Logger.WithField("error", err).Error("Error during processing of login IRMA session result") - return server.ErrorInternal, err.Error() + return server.RemoteError(server.ErrorInternal, err.Error()) } - session.userID = &id + session.UserID = &id err = s.db.setSeen(ctx, id) if err != nil { s.conf.Logger.WithField("error", err).Error("Could not update users last seen time/date") // not relevant for frontend, so ignore beyond log. } - return server.Error{}, "" + return nil } -func (s *Server) handleIrmaLogin(w http.ResponseWriter, _ *http.Request) { - session := s.store.create() - sessiontoken := session.token - +func (s *Server) handleIrmaLogin(w http.ResponseWriter, r *http.Request) { qr, loginToken, frontendRequest, err := s.irmaserv.StartSession( newIrmaDisclosureRequest(s.conf.KeyshareAttributes), nil, @@ -449,8 +471,17 @@ func (s *Server) handleIrmaLogin(w http.ResponseWriter, _ *http.Request) { return } - session.loginSessionToken = loginToken - s.setCookie(w, sessiontoken, s.conf.SessionLifetime) + session := session{ + Token: common.NewSessionToken(), + LoginSessionToken: loginToken, + Expiry: time.Now().Add(time.Duration(s.conf.SessionLifetime) * time.Second), + } + if err := s.store.add(r.Context(), session); err != nil { + keyshare.WriteError(w, err) + return + } + + s.setCookie(w, session.Token, s.conf.SessionLifetime) server.WriteJson(w, server.SessionPackage{ SessionPtr: qr, FrontendRequest: frontendRequest, @@ -475,8 +506,15 @@ func (s *Server) handleVerifyEmail(w http.ResponseWriter, r *http.Request) { return } - session := s.store.create() - session.userID = &id + session := session{ + Token: common.NewSessionToken(), + UserID: &id, + Expiry: time.Now().Add(time.Duration(s.conf.SessionLifetime) * time.Second), + } + if err := s.store.add(r.Context(), session); err != nil { + keyshare.WriteError(w, err) + return + } err = s.db.setSeen(r.Context(), id) if err != nil { @@ -484,20 +522,23 @@ func (s *Server) handleVerifyEmail(w http.ResponseWriter, r *http.Request) { // not relevant for frontend, so ignore beyond log. } - s.setCookie(w, session.token, s.conf.SessionLifetime) + s.setCookie(w, session.Token, s.conf.SessionLifetime) w.WriteHeader(http.StatusNoContent) } -func (s *Server) logoutUser(w http.ResponseWriter, r *http.Request) { - session := s.sessionFromCookie(r) - if session != nil { - session.userID = nil // expire session - } +func (s *Server) logoutUser(w http.ResponseWriter, session *session) { + session.UserID = nil s.setCookie(w, "", -1) } func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) { - s.logoutUser(w, r) + if err := s.sessionFromCookie(r, func(session *session) error { + s.logoutUser(w, session) + return nil + }); err != nil { + // Ignore error and just delete the cookie. + s.setCookie(w, "", -1) + } w.WriteHeader(http.StatusNoContent) } @@ -505,7 +546,7 @@ func (s *Server) handleUserInfo(w http.ResponseWriter, r *http.Request) { session := r.Context().Value("session").(*session) // Handle finished IRMA session used for adding email address, if any - if session.emailSessionToken != "" { + if session.EmailSessionToken != "" { e, msg := s.processAddEmailIrmaSessionResult(r.Context(), session) if e != (server.Error{}) { server.WriteError(w, e, msg) @@ -513,15 +554,15 @@ func (s *Server) handleUserInfo(w http.ResponseWriter, r *http.Request) { } } - user, err := s.db.user(r.Context(), *session.userID) + user, err := s.db.user(r.Context(), *session.UserID) if err != nil { s.conf.Logger.WithField("error", err).Error("Problem fetching user information from database") keyshare.WriteError(w, err) return } - session.expiry = time.Now().Add(time.Duration(s.conf.SessionLifetime) * time.Second) - s.setCookie(w, session.token, s.conf.SessionLifetime) + session.Expiry = time.Now().Add(time.Duration(s.conf.SessionLifetime) * time.Second) + s.setCookie(w, session.Token, s.conf.SessionLifetime) if user.Emails == nil { user.Emails = []userEmail{} @@ -539,15 +580,15 @@ func (s *Server) handleGetLogs(w http.ResponseWriter, r *http.Request) { } session := r.Context().Value("session").(*session) - entries, err := s.db.logs(r.Context(), *session.userID, offset, 11) + entries, err := s.db.logs(r.Context(), *session.UserID, offset, 11) if err != nil { s.conf.Logger.WithField("error", err).Error("Could not load log entries") keyshare.WriteError(w, err) return } - session.expiry = time.Now().Add(time.Duration(s.conf.SessionLifetime) * time.Second) - s.setCookie(w, session.token, s.conf.SessionLifetime) + session.Expiry = time.Now().Add(time.Duration(s.conf.SessionLifetime) * time.Second) + s.setCookie(w, session.Token, s.conf.SessionLifetime) if entries == nil { entries = []logEntry{} @@ -556,7 +597,7 @@ func (s *Server) handleGetLogs(w http.ResponseWriter, r *http.Request) { } func (s *Server) processRemoveEmail(ctx context.Context, session *session, email string) error { - user, err := s.db.user(ctx, *session.userID) + user, err := s.db.user(ctx, *session.UserID) if err != nil { s.conf.Logger.WithField("error", err).Error("Error checking whether email address can be removed") return err @@ -574,7 +615,7 @@ func (s *Server) processRemoveEmail(ctx context.Context, session *session, email if err := keyshare.VerifyMXRecord(email); err != nil { if s.db.hasEmailRevalidation(ctx) { - if err := s.db.scheduleEmailRevalidation(ctx, *session.userID, email, 5*24*time.Hour); err != nil { + if err := s.db.scheduleEmailRevalidation(ctx, *session.UserID, email, 5*24*time.Hour); err != nil { s.conf.Logger.WithField("error", err).Error("Could not schedule email address for revalidation") return err } @@ -595,7 +636,7 @@ func (s *Server) processRemoveEmail(ctx context.Context, session *session, email } } - err = s.db.scheduleEmailRemoval(ctx, *session.userID, email, 24*time.Hour*time.Duration(s.conf.DeleteDelay)) + err = s.db.scheduleEmailRemoval(ctx, *session.UserID, email, 24*time.Hour*time.Duration(s.conf.DeleteDelay)) if err != nil { s.conf.Logger.WithField("error", err).Error("Error removing user email address") return err @@ -629,19 +670,19 @@ func (s *Server) handleRemoveEmail(w http.ResponseWriter, r *http.Request) { return } - session.expiry = time.Now().Add(time.Duration(s.conf.SessionLifetime) * time.Second) - s.setCookie(w, session.token, s.conf.SessionLifetime) + session.Expiry = time.Now().Add(time.Duration(s.conf.SessionLifetime) * time.Second) + s.setCookie(w, session.Token, s.conf.SessionLifetime) w.WriteHeader(http.StatusNoContent) } func (s *Server) processAddEmailIrmaSessionResult(ctx context.Context, session *session) (server.Error, string) { - result, err := s.irmaserv.GetSessionResult(session.emailSessionToken) + result, err := s.irmaserv.GetSessionResult(session.EmailSessionToken) if err != nil { return server.ErrorInternal, "" } if result == nil { - session.emailSessionToken = "" + session.EmailSessionToken = "" return server.ErrorInternal, "unknown login session" } @@ -650,7 +691,7 @@ func (s *Server) processAddEmailIrmaSessionResult(ctx context.Context, session * return server.Error{}, "" } - session.emailSessionToken = "" + session.EmailSessionToken = "" if result.ProofStatus != irma.ProofStatusValid { s.conf.Logger.Info("received invalid email attribute") @@ -658,7 +699,7 @@ func (s *Server) processAddEmailIrmaSessionResult(ctx context.Context, session * } email := *result.Disclosed[0][0].RawValue - err = s.db.addEmail(ctx, *session.userID, email) + err = s.db.addEmail(ctx, *session.UserID, email) if err != nil { s.conf.Logger.WithField("error", err).Error("Could not add email address to user") return server.ErrorInternal, err.Error() @@ -680,9 +721,9 @@ func (s *Server) handleAddEmail(w http.ResponseWriter, r *http.Request) { return } - session.emailSessionToken = emailToken - session.expiry = time.Now().Add(time.Duration(s.conf.SessionLifetime) * time.Second) - s.setCookie(w, session.token, s.conf.SessionLifetime) + session.EmailSessionToken = emailToken + session.Expiry = time.Now().Add(time.Duration(s.conf.SessionLifetime) * time.Second) + s.setCookie(w, session.Token, s.conf.SessionLifetime) server.WriteJson(w, server.SessionPackage{ SessionPtr: qr, @@ -692,16 +733,26 @@ func (s *Server) handleAddEmail(w http.ResponseWriter, r *http.Request) { func (s *Server) sessionMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - session := s.sessionFromCookie(r) - if session == nil || session.userID == nil { - s.conf.Logger.Info("Malformed request: user not logged in") - server.WriteError(w, server.ErrorInvalidRequest, "not logged in") - return - } + if err := s.sessionFromCookie(r, func(session *session) error { + if session.UserID == nil { + s.conf.Logger.Info("Malformed request: user not logged in") + server.WriteError(w, server.ErrorInvalidRequest, "not logged in") + return nil + } + + next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), "session", session))) - session.Lock() - defer session.Unlock() - next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), "session", session))) + return nil + }); err != nil { + if err == errUnknownSession { + server.WriteError(w, server.ErrorInvalidRequest, "not logged in") + return + } + + // TODO: this might lead to a duplicate write to w. Check if this is a problem. + // This also happens in userMiddleware in irmaserver. + keyshare.WriteError(w, err) + } }) } @@ -709,12 +760,12 @@ func (s *Server) staticFilesHandler() http.Handler { return http.StripPrefix(s.conf.StaticPrefix, http.FileServer(http.Dir(s.conf.StaticPath))) } -func (s *Server) sessionFromCookie(r *http.Request) *session { +func (s *Server) sessionFromCookie(r *http.Request, handler func(ses *session) error) error { token, err := r.Cookie("session") if err != nil { // only happens if cookie is not present - return nil + return errUnknownSession } - return s.store.get(token.Value) + return s.store.update(r.Context(), token.Value, handler) } // newIrmaDisclosureRequest composes an irma.DisclosureRequest with one attribute disjunction diff --git a/server/keyshare/myirmaserver/session.go b/server/keyshare/myirmaserver/session.go index 68b168785..f4ef34f96 100644 --- a/server/keyshare/myirmaserver/session.go +++ b/server/keyshare/myirmaserver/session.go @@ -1,60 +1,76 @@ package myirmaserver import ( + "context" + "encoding/json" "sync" "time" + "github.com/go-errors/errors" + "github.com/go-redis/redis/v8" irma "github.com/privacybydesign/irmago" - "github.com/privacybydesign/irmago/internal/common" + "github.com/privacybydesign/irmago/server" + "github.com/sirupsen/logrus" ) -type session struct { - sync.Mutex +const sessionLookupPrefix = "myirmaserver/session/" - token string - userID *int64 +var ( + errRedis = errors.New("redis error") + errUnknownSession = errors.New("unknown session") +) + +type session struct { + Token string `json:"token"` + UserID *int64 `json:"user_id,omitempty"` - loginSessionToken irma.RequestorToken - emailSessionToken irma.RequestorToken + LoginSessionToken irma.RequestorToken `json:"login_session_token,omitempty"` + EmailSessionToken irma.RequestorToken `json:"email_session_token,omitempty"` - expiry time.Time + Expiry time.Time `json:"expiry"` } type sessionStore interface { - create() *session - get(token string) *session + add(ctx context.Context, ses session) error + update(ctx context.Context, token string, handler func(ses *session) error) error flush() } type memorySessionStore struct { sync.Mutex + data map[string]session +} - data map[string]*session - sessionLifetime time.Duration +type redisSessionStore struct { + client *server.RedisClient + logger *logrus.Logger } -func newMemorySessionStore(sessionLifetime time.Duration) sessionStore { +func newMemorySessionStore() sessionStore { return &memorySessionStore{ - sessionLifetime: sessionLifetime, - data: map[string]*session{}, + data: map[string]session{}, } } -func (s *memorySessionStore) create() *session { +func (s *memorySessionStore) add(_ context.Context, ses session) error { s.Lock() defer s.Unlock() - token := common.NewSessionToken() - s.data[token] = &session{ - token: token, - expiry: time.Now().Add(s.sessionLifetime), - } - return s.data[token] + s.data[ses.Token] = ses + return nil } -func (s *memorySessionStore) get(token string) *session { +func (s *memorySessionStore) update(_ context.Context, token string, handler func(ses *session) error) error { s.Lock() defer s.Unlock() - return s.data[token] + ses, ok := s.data[token] + if !ok { + return errUnknownSession + } + if err := handler(&ses); err != nil { + return err + } + s.data[token] = ses + return nil } func (s *memorySessionStore) flush() { @@ -62,8 +78,78 @@ func (s *memorySessionStore) flush() { s.Lock() defer s.Unlock() for k, v := range s.data { - if now.After(v.expiry) { + if now.After(v.Expiry) { delete(s.data, k) } } } + +func (s *redisSessionStore) add(ctx context.Context, ses session) error { + bytes, err := json.Marshal(ses) + if err != nil { + return err + } + + ttl := time.Until(ses.Expiry).Seconds() + conn := s.client.Conn(ctx) + if err := conn.Set(ctx, sessionLookupPrefix+ses.Token, string(bytes), time.Duration(ttl)*time.Second).Err(); err != nil { + s.logger.WithError(err).Error("failed to store session") + return errRedis + } + if s.client.FailoverMode { + if err := conn.Wait(ctx, 1, time.Second).Err(); err != nil { + s.logger.WithError(err).Error("failed to persist session") + return errRedis + } + } + return nil +} + +func (s *redisSessionStore) update(ctx context.Context, token string, handler func(ses *session) error) error { + key := sessionLookupPrefix + token + + err := s.client.Watch(ctx, func(tx *redis.Tx) error { + bytes, err := tx.Get(ctx, key).Bytes() + if err == redis.Nil { + return errUnknownSession + } else if err != nil { + return err + } + + session := &session{} + if err := json.Unmarshal(bytes, session); err != nil { + return err + } + + if err := handler(session); err != nil { + return err + } + + updatedBytes, err := json.Marshal(session) + if err != nil { + return err + } + + if err := tx.Set(ctx, key, string(updatedBytes), time.Until(session.Expiry)).Err(); err != nil { + return err + } + + if s.client.FailoverMode { + if err := tx.Wait(ctx, 1, time.Second).Err(); err != nil { + return err + } + } + return nil + }) + if err == errUnknownSession { + return err + } else if err != nil { + s.logger.WithError(err).Error("failed to update session") + return errRedis + } + return nil +} + +func (s *redisSessionStore) flush() { + // Redis keys expire automatically. +} diff --git a/server/keyshare/myirmaserver/session_test.go b/server/keyshare/myirmaserver/session_test.go index 57d0a2465..f75348108 100644 --- a/server/keyshare/myirmaserver/session_test.go +++ b/server/keyshare/myirmaserver/session_test.go @@ -1,33 +1,60 @@ package myirmaserver import ( + "context" "testing" "time" + irma "github.com/privacybydesign/irmago" "github.com/stretchr/testify/assert" ) func TestSessions(t *testing.T) { - store := newMemorySessionStore(1 * time.Second) + store := newMemorySessionStore() - s := store.create() - assert.NotEqual(t, (*session)(nil), s) + s := session{ + Token: "token", + Expiry: time.Now().Add(1 * time.Second), + } + err := store.add(context.Background(), s) + assert.NoError(t, err) - session2 := store.get(s.token) + session2, err := getSession(store, s.Token) + assert.NoError(t, err) assert.Equal(t, s, session2) - session3 := store.get("DOESNOTEXIST") - assert.Equal(t, (*session)(nil), session3) + emailSessionToken := irma.RequestorToken("emailtoken") + err = store.update(context.Background(), s.Token, func(ses *session) error { + ses.EmailSessionToken = emailSessionToken + return nil + }) + assert.NoError(t, err) + + session3, err := getSession(store, s.Token) + assert.NoError(t, err) + assert.Equal(t, session3.EmailSessionToken, emailSessionToken) + + _, err = getSession(store, "DOESNOTEXIST") + assert.ErrorIs(t, err, errUnknownSession) store.flush() - session4 := store.get(s.token) - assert.Equal(t, s, session4) + session4, err := getSession(store, s.Token) + assert.NoError(t, err) + assert.Equal(t, session4.Token, s.Token) time.Sleep(2 * time.Second) store.flush() - session5 := store.get(s.token) - assert.Equal(t, (*session)(nil), session5) + _, err = getSession(store, s.Token) + assert.ErrorIs(t, err, errUnknownSession) +} + +func getSession(store sessionStore, token string) (session, error) { + var ses session + return ses, store.update(context.Background(), token, func(s *session) error { + ses = *s + return nil + }) } From 46307df88f5a7d6bf6416e73b136001b7992f199 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Wed, 18 Oct 2023 12:09:06 +0200 Subject: [PATCH 02/25] Refactor: improve code structure of Redis logic --- README.md | 2 +- internal/sessiontest/redis_test.go | 2 -- irma/cmd/config.go | 2 +- irma/cmd/keyshare-myirma.go | 2 +- irma/cmd/keyshare-server.go | 2 +- irma/cmd/server.go | 2 +- server/irmaserver/api.go | 11 +++++++---- server/irmaserver/handle.go | 10 +++++----- server/irmaserver/helpers.go | 5 ++--- server/irmaserver/sessions.go | 6 +++--- 10 files changed, 22 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 04e2f7176..5a87431b2 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,7 @@ You can then start `irma` with the store-type flag set to Redis and the [default irma server -vv --store-type redis --redis-addr "localhost:6379" --redis-allow-empty-password --redis-no-tls ``` -If you use Redis in Sentinel or Cluster mode for high availability, you need to consider whether you accept the risk of losing session state in case of a failover. Redis does not guarantee [strong consistency](https://redis.io/docs/management/scaling/#redis-cluster-consistency-guarantees) in these setups. We mitigated this by waiting for a write to have reached the master node and at least one replica. This means that at least two replicas should be configured for every master node to achieve high availability. Even then, there is a small chance of losing session state when a replica fails at the same time as the master node. For example, this might be problematic if you want to guarantee that a credential is not issued twice or if you need a session QR to have a long lifetime but you do want the session to be finished soon after the QR is scanned. If you require IRMA sessions to be highly consistent, you should use the default in-memory store or Redis in standalone mode. If you accept this risk, then you can enable Sentinel and Cluster mode support by setting the `--redis-accept-inconsistency-risk` flag. +If you use Redis in Sentinel mode for high availability, you need to consider whether you accept the risk of losing session state in case of a failover. Redis does not guarantee [strong consistency](https://redis.io/docs/management/scaling/#redis-cluster-consistency-guarantees) in these setups. We mitigated this by waiting for a write to have reached the master node and at least one replica. This means that at least two replicas should be configured for every master node to achieve high availability. Even then, there is a small chance of losing session state when a replica fails at the same time as the master node. For example, this might be problematic if you want to guarantee that a credential is not issued twice or if you need a session QR to have a long lifetime but you do want the session to be finished soon after the QR is scanned. If you require IRMA sessions to be highly consistent, you should use the default in-memory store or Redis in standalone mode. If you accept this risk, then you can enable Sentinel mode support by setting the `--redis-accept-inconsistency-risk` flag. Besides the `irma server`, Redis can also be configured for the `irma keyshare server` and the `irma keyshare myirmaserver` in the same way as described above. Note that the `irma keyshare server` does not become stateless when using Redis, because it stores the keyshare commitments and authentication challenges in memory. These cannot be stored in Redis, because we require this data to be strongly consistent. Instead, you can use sticky sessions to make sure that the same user is always routed to the same keyshare server instance. The stored commitments and challenges are only relevant for a few seconds, so the risk of losing this data is low. The `irma keyshare myirmaserver` does become stateless when using Redis. diff --git a/internal/sessiontest/redis_test.go b/internal/sessiontest/redis_test.go index 518519327..8c98d514f 100644 --- a/internal/sessiontest/redis_test.go +++ b/internal/sessiontest/redis_test.go @@ -198,8 +198,6 @@ func checkErrorInternal(t *testing.T, err error) { require.Equal(t, string(server.ErrorInternal.Type), serr.RemoteError.ErrorName) } -// TODO: What to do with TestRedisUpdates? - func TestRedisRedundancy(t *testing.T) { mr, cert := startRedis(t, true) defer mr.Close() diff --git a/irma/cmd/config.go b/irma/cmd/config.go index aa28c992c..4fed7e350 100644 --- a/irma/cmd/config.go +++ b/irma/cmd/config.go @@ -80,7 +80,7 @@ func configureIRMAServer() (*server.Configuration, error) { conf.RedisSettings.AcceptInconsistencyRisk = viper.GetBool("redis_accept_inconsistency_risk") if conf.RedisSettings.Addr == "" && len(conf.RedisSettings.SentinelAddrs) == 0 || conf.RedisSettings.Addr != "" && len(conf.RedisSettings.SentinelAddrs) > 0 { - return nil, errors.New("When Redis is used as session data store, exactly one of --redis-addr, --redis-sentinel-addrs or --redis-cluster-addrs must be specified.") + return nil, errors.New("When Redis is used as session data store, either --redis-addr or --redis-sentinel-addrs must be specified.") } if conf.RedisSettings.Password = viper.GetString("redis_pw"); conf.RedisSettings.Password == "" && !viper.GetBool("redis_allow_empty_password") { diff --git a/irma/cmd/keyshare-myirma.go b/irma/cmd/keyshare-myirma.go index 9b60fd359..3afb5a1fd 100644 --- a/irma/cmd/keyshare-myirma.go +++ b/irma/cmd/keyshare-myirma.go @@ -64,7 +64,7 @@ func init() { flags.String("store-type", "", "specifies how session state will be saved on the server (default \"memory\")") flags.String("redis-addr", "", "Redis address, to be specified as host:port") flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") - flags.StringSlice("redis-cluster-addrs", nil, "Redis Cluster addresses, to be specified as host:port") + flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") diff --git a/irma/cmd/keyshare-server.go b/irma/cmd/keyshare-server.go index 9f768f230..5b7ba871e 100644 --- a/irma/cmd/keyshare-server.go +++ b/irma/cmd/keyshare-server.go @@ -61,7 +61,7 @@ func init() { flags.String("store-type", "", "specifies how session state will be saved on the server (default \"memory\")") flags.String("redis-addr", "", "Redis address, to be specified as host:port") flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") - flags.StringSlice("redis-cluster-addrs", nil, "Redis Cluster addresses, to be specified as host:port") + flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") diff --git a/irma/cmd/server.go b/irma/cmd/server.go index b9bf3336e..23479c3c6 100644 --- a/irma/cmd/server.go +++ b/irma/cmd/server.go @@ -125,7 +125,7 @@ func setFlags(cmd *cobra.Command, production bool) error { flags.String("store-type", "", "specifies how session state will be saved on the server (default \"memory\")") flags.String("redis-addr", "", "Redis address, to be specified as host:port") flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") - flags.StringSlice("redis-cluster-addrs", nil, "Redis Cluster addresses, to be specified as host:port") + flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") diff --git a/server/irmaserver/api.go b/server/irmaserver/api.go index 475b83a75..3c05ff765 100644 --- a/server/irmaserver/api.go +++ b/server/irmaserver/api.go @@ -448,9 +448,10 @@ func (s *Server) subscribeServerSentEvents(w http.ResponseWriter, r *http.Reques token := string(session.ClientToken) if requestor { token = string(session.RequestorToken) + } else { + s.serverSentEvents.SendMessage("frontendsession/"+token, sse.NewMessage("", "", "open")) } s.serverSentEvents.SendMessage("session/"+token, sse.NewMessage("", "", "open")) - s.serverSentEvents.SendMessage("frontendsession/"+token, sse.NewMessage("", "", "open")) }() s.serverSentEvents.ServeHTTP(w, r) @@ -523,7 +524,7 @@ func SessionStatus(requestorToken irma.RequestorToken) (chan irma.ServerStatus, return s.SessionStatus(requestorToken) } func (s *Server) SessionStatus(requestorToken irma.RequestorToken) (statusChan chan irma.ServerStatus, err error) { - if s.conf.StoreType != "memory" { + if s.conf.StoreType == "redis" { return nil, errors.New("SessionStatus cannot be used in combination with Redis/etcd.") } @@ -532,10 +533,12 @@ func (s *Server) SessionStatus(requestorToken irma.RequestorToken) (statusChan c return nil, err } var timeoutTime time.Time - s.sessions.transaction(requestorToken, func(session *sessionData) error { + if err := s.sessions.transaction(requestorToken, func(session *sessionData) error { timeoutTime = time.Now().Add(session.timeout(s.conf)) return nil - }) + }); err != nil { + return nil, err + } statusChan = make(chan irma.ServerStatus, 4) go func() { diff --git a/server/irmaserver/handle.go b/server/irmaserver/handle.go index 54d5aeaaa..fb476c6b8 100644 --- a/server/irmaserver/handle.go +++ b/server/irmaserver/handle.go @@ -28,7 +28,7 @@ func (session *sessionData) handleDelete(conf *server.Configuration) { if session.Status.Finished() { return } - session.markAlive() + session.markAlive(conf) session.Result = &server.SessionResult{Token: session.RequestorToken, Status: irma.ServerStatusCancelled, Type: session.Action} session.setStatus(irma.ServerStatusCancelled, conf) @@ -41,7 +41,7 @@ func (session *sessionData) handleGetClientRequest(min, max *irma.ProtocolVersio return nil, server.RemoteError(server.ErrorUnexpectedRequest, "Session already started") } - session.markAlive() + session.markAlive(conf) logger := conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}) var err error @@ -106,7 +106,7 @@ func (session *sessionData) handleGetStatus() (irma.ServerStatus, *irma.RemoteEr } func (session *sessionData) handlePostSignature(signature *irma.SignedMessage, conf *server.Configuration) (*irma.ServerSessionResponse, *irma.RemoteError) { - session.markAlive() + session.markAlive(conf) var err error var rerr *irma.RemoteError @@ -132,7 +132,7 @@ func (session *sessionData) handlePostSignature(signature *irma.SignedMessage, c } func (session *sessionData) handlePostDisclosure(disclosure *irma.Disclosure, conf *server.Configuration) (*irma.ServerSessionResponse, *irma.RemoteError) { - session.markAlive() + session.markAlive(conf) var err error var rerr *irma.RemoteError @@ -156,7 +156,7 @@ func (session *sessionData) handlePostDisclosure(disclosure *irma.Disclosure, co } func (session *sessionData) handlePostCommitments(commitments *irma.IssueCommitmentMessage, conf *server.Configuration) (*irma.ServerSessionResponse, *irma.RemoteError) { - session.markAlive() + session.markAlive(conf) request := session.Rrequest.SessionRequest().(*irma.IssuanceRequest) discloseCount := len(commitments.Proofs) - len(request.Credentials) diff --git a/server/irmaserver/helpers.go b/server/irmaserver/helpers.go index 7b44c17ee..971a46aad 100644 --- a/server/irmaserver/helpers.go +++ b/server/irmaserver/helpers.go @@ -28,10 +28,9 @@ import ( // Session helpers -func (session *sessionData) markAlive() { - // TODO: use from conf? +func (session *sessionData) markAlive(conf *server.Configuration) { session.LastActive = time.Now() - server.Logger. + conf.Logger. WithFields(logrus.Fields{"session": session.RequestorToken}). Debug("Session marked active, deletion delayed") } diff --git a/server/irmaserver/sessions.go b/server/irmaserver/sessions.go index 1afc39073..40685c2a9 100644 --- a/server/irmaserver/sessions.go +++ b/server/irmaserver/sessions.go @@ -231,14 +231,14 @@ func (s *memorySessionStore) deleteExpired() { // First check which sessions have expired // We don't need a write lock for this yet, so postpone that for actual deleting s.RLock() - var toCheck []irma.RequestorToken + toCheck := make(map[irma.RequestorToken]struct{}, len(s.requestor)) for token := range s.requestor { - toCheck = append(toCheck, token) + toCheck[token] = struct{}{} } s.RUnlock() expired := make([]irma.RequestorToken, 0, len(toCheck)) - for _, token := range toCheck { + for token := range toCheck { s.transaction(token, func(session *sessionData) error { if session.ttl(s.conf) <= 0 { s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Deleting session") From 173bd02945a415f06c2de6b9b95b1b3bac000aa5 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Wed, 18 Oct 2023 12:06:41 +0200 Subject: [PATCH 03/25] Refactor: improve subscribe status updates code structure --- server/irmaserver/api.go | 73 +++++++++++++--------- server/irmaserver/handle.go | 3 +- server/irmaserver/helpers.go | 36 +++++++---- server/irmaserver/sessions.go | 113 ++++++++++++++++------------------ 4 files changed, 121 insertions(+), 104 deletions(-) diff --git a/server/irmaserver/api.go b/server/irmaserver/api.go index 3c05ff765..1d0f86605 100644 --- a/server/irmaserver/api.go +++ b/server/irmaserver/api.go @@ -72,7 +72,7 @@ func New(conf *server.Configuration) (*Server, error) { conf: conf, requestor: make(map[irma.RequestorToken]*memorySessionData), client: make(map[irma.ClientToken]*memorySessionData), - statusChannels: make(map[irma.RequestorToken][]chan statusChange), + updateChannels: make(map[irma.RequestorToken][]chan *sessionData), } if _, err := s.scheduler.Every(10).Seconds().Do(func() { @@ -275,17 +275,19 @@ func (s *Server) startNextSession( if handler != nil { go func() { - statusChangeChan, err := s.sessions.subscribeStatusChanges(ses.RequestorToken) + updateChan, err := s.sessions.subscribeUpdates(ses.RequestorToken) if err != nil { s.conf.Logger.WithError(err).Error("Failed to subscribe to session status changes for handler") return } - for change := range statusChangeChan { - if change.Status.Finished() { - s.sessions.transaction(ses.RequestorToken, func(ses *sessionData) error { + for update := range updateChan { + if update.Status.Finished() { + if err := s.sessions.transaction(ses.RequestorToken, func(ses *sessionData) error { handler(ses.Result) return nil - }) + }); err != nil { + s.conf.Logger.WithError(err).Error("Failed to execute session handler") + } return } } @@ -425,12 +427,12 @@ func (s *Server) subscribeServerSentEvents(w http.ResponseWriter, r *http.Reques s.activeSSEHandlersMutex.Unlock() if !activeHandler { - statusChangeChan, err := s.sessions.subscribeStatusChanges(session.RequestorToken) + updateChan, err := s.sessions.subscribeUpdates(session.RequestorToken) if err != nil { return err } go func() { - s.serverSentEventsHandler(session, statusChangeChan) + s.serverSentEventsHandler(session, updateChan) s.activeSSEHandlersMutex.Lock() defer s.activeSSEHandlersMutex.Unlock() delete(s.activeSSEHandlers, session.RequestorToken) @@ -458,42 +460,47 @@ func (s *Server) subscribeServerSentEvents(w http.ResponseWriter, r *http.Reques return nil } -func (s *Server) serverSentEventsHandler(session *sessionData, statusChangeChan chan statusChange) { - timeoutTime := time.Now().Add(session.timeout(s.conf)) +func (s *Server) serverSentEventsHandler(initialSession *sessionData, updateChan chan *sessionData) { + timeoutTime := time.Now().Add(initialSession.timeout(s.conf)) // Close the channels when this function returns. defer func() { - s.serverSentEvents.CloseChannel("session/" + string(session.RequestorToken)) - s.serverSentEvents.CloseChannel("session/" + string(session.ClientToken)) - s.serverSentEvents.CloseChannel("frontendsession/" + string(session.ClientToken)) + s.serverSentEvents.CloseChannel("session/" + string(initialSession.RequestorToken)) + s.serverSentEvents.CloseChannel("session/" + string(initialSession.ClientToken)) + s.serverSentEvents.CloseChannel("frontendsession/" + string(initialSession.ClientToken)) }() + currStatus := initialSession.Status for { select { - case change, ok := <-statusChangeChan: + case update, ok := <-updateChan: if !ok { return } + if currStatus == update.Status { + continue + } + currStatus = update.Status - frontendStatusBytes, err := json.Marshal(change.FrontendSessionStatus) + frontendStatusBytes, err := json.Marshal(update.frontendSessionStatus()) if err != nil { s.conf.Logger.Error(err) return } - s.serverSentEvents.SendMessage("session/"+string(session.RequestorToken), - sse.SimpleMessage(fmt.Sprintf(`"%s"`, change.Status)), + s.serverSentEvents.SendMessage("session/"+string(update.RequestorToken), + sse.SimpleMessage(fmt.Sprintf(`"%s"`, currStatus)), ) - s.serverSentEvents.SendMessage("session/"+string(session.ClientToken), - sse.SimpleMessage(fmt.Sprintf(`"%s"`, change.Status)), + s.serverSentEvents.SendMessage("session/"+string(update.ClientToken), + sse.SimpleMessage(fmt.Sprintf(`"%s"`, currStatus)), ) - s.serverSentEvents.SendMessage("frontendsession/"+string(session.ClientToken), + s.serverSentEvents.SendMessage("frontendsession/"+string(update.ClientToken), sse.SimpleMessage(string(frontendStatusBytes)), ) - if change.Status.Finished() { + if currStatus.Finished() { return } - timeoutTime = time.Now().Add(change.Timeout) + timeoutTime = time.Now().Add(update.timeout(s.conf)) case <-time.After(time.Until(timeoutTime)): frontendStatus := irma.FrontendSessionStatus{ Status: irma.ServerStatusTimeout, @@ -504,13 +511,13 @@ func (s *Server) serverSentEventsHandler(session *sessionData, statusChangeChan return } - s.serverSentEvents.SendMessage("session/"+string(session.RequestorToken), + s.serverSentEvents.SendMessage("session/"+string(initialSession.RequestorToken), sse.SimpleMessage(fmt.Sprintf(`"%s"`, frontendStatus.Status)), ) - s.serverSentEvents.SendMessage("session/"+string(session.ClientToken), + s.serverSentEvents.SendMessage("session/"+string(initialSession.ClientToken), sse.SimpleMessage(fmt.Sprintf(`"%s"`, frontendStatus.Status)), ) - s.serverSentEvents.SendMessage("frontendsession/"+string(session.ClientToken), + s.serverSentEvents.SendMessage("frontendsession/"+string(initialSession.ClientToken), sse.SimpleMessage(string(frontendStatusBytes)), ) return @@ -528,7 +535,7 @@ func (s *Server) SessionStatus(requestorToken irma.RequestorToken) (statusChan c return nil, errors.New("SessionStatus cannot be used in combination with Redis/etcd.") } - statusChangeChan, err := s.sessions.subscribeStatusChanges(requestorToken) + updateChan, err := s.sessions.subscribeUpdates(requestorToken) if err != nil { return nil, err } @@ -542,20 +549,26 @@ func (s *Server) SessionStatus(requestorToken irma.RequestorToken) (statusChan c statusChan = make(chan irma.ServerStatus, 4) go func() { + var currStatus irma.ServerStatus for { select { - case change, ok := <-statusChangeChan: + case update, ok := <-updateChan: if !ok { close(statusChan) return } - statusChan <- change.Status + if currStatus == update.Status { + continue + } + currStatus = update.Status + + statusChan <- currStatus - if change.Status.Finished() { + if currStatus.Finished() { close(statusChan) return } - timeoutTime = time.Now().Add(change.Timeout) + timeoutTime = time.Now().Add(update.timeout(s.conf)) case <-time.After(time.Until(timeoutTime)): statusChan <- irma.ServerStatusTimeout close(statusChan) diff --git a/server/irmaserver/handle.go b/server/irmaserver/handle.go index fb476c6b8..d4f0ba3c3 100644 --- a/server/irmaserver/handle.go +++ b/server/irmaserver/handle.go @@ -447,8 +447,7 @@ func (s *Server) handleSessionGetRequest(w http.ResponseWriter, r *http.Request) func (s *Server) handleFrontendStatus(w http.ResponseWriter, r *http.Request) { session := r.Context().Value("session").(*sessionData) - status := irma.FrontendSessionStatus{Status: session.Status, NextSession: session.Next} - server.WriteResponse(w, status, nil) + server.WriteResponse(w, session.frontendSessionStatus(), nil) } func (s *Server) handleFrontendStatusEvents(w http.ResponseWriter, r *http.Request) { diff --git a/server/irmaserver/helpers.go b/server/irmaserver/helpers.go index 971a46aad..d72694467 100644 --- a/server/irmaserver/helpers.go +++ b/server/irmaserver/helpers.go @@ -330,15 +330,15 @@ func (session *sessionData) getRequest() (irma.SessionRequest, error) { if !issuing { return req, nil } - cpy, err := copyObject(isreq) - if err != nil { + copy := &irma.IssuanceRequest{} + if err := copyObject(isreq, copy); err != nil { return nil, err } - for _, cred := range cpy.(*irma.IssuanceRequest).Credentials { + for _, cred := range copy.Credentials { cred.RevocationSupported = cred.RevocationKey != "" cred.RevocationKey = "" } - return cpy.(*irma.IssuanceRequest), nil + return copy, nil } func (session *sessionData) hash() [32]byte { @@ -365,6 +365,13 @@ func (session *sessionData) ttl(conf *server.Configuration) time.Duration { return session.timeout(conf) + time.Duration(conf.SessionResultLifetime)*time.Minute } +func (session *sessionData) frontendSessionStatus() irma.FrontendSessionStatus { + return irma.FrontendSessionStatus{ + Status: session.Status, + NextSession: session.Next, + } +} + // UnmarshalJSON unmarshals sessionData. func (session *sessionData) UnmarshalJSON(data []byte) error { type rawSession sessionData @@ -418,16 +425,23 @@ func (s *Server) validateRequest(request irma.SessionRequest) error { return request.Disclosure().Disclose.Validate(s.conf.IrmaConfiguration) } -func copyObject(i interface{}) (interface{}, error) { - cpy := reflect.New(reflect.TypeOf(i).Elem()).Interface() - bts, err := json.Marshal(i) +func copyObject[T any](object T, copy T) error { + bts, err := json.Marshal(object) if err != nil { - return nil, err + return err } - if err = json.Unmarshal(bts, cpy); err != nil { + if err = json.Unmarshal(bts, copy); err != nil { + return err + } + return nil +} + +func copyInterface(i interface{}) (interface{}, error) { + copy := reflect.New(reflect.TypeOf(i).Elem()).Interface() + if err := copyObject(i, copy); err != nil { return nil, err } - return cpy, nil + return copy, nil } // purgeRequest logs the request excluding any attribute values. @@ -438,7 +452,7 @@ func purgeRequest(request irma.RequestorRequest) irma.RequestorRequest { // Ugly hack alert: the easiest way to do this seems to be to convert it to JSON and then back. // As we do not know the precise type of request, we use reflection to create a new instance // of the same type as request, into which we then unmarshal our copy. - cpy, err := copyObject(request) + cpy, err := copyInterface(request) if err != nil { panic(err) } diff --git a/server/irmaserver/sessions.go b/server/irmaserver/sessions.go index 40685c2a9..11ff34a3e 100644 --- a/server/irmaserver/sessions.go +++ b/server/irmaserver/sessions.go @@ -51,21 +51,16 @@ type sessionStore interface { add(*sessionData) error transaction(irma.RequestorToken, func(*sessionData) error) error clientTransaction(irma.ClientToken, func(*sessionData) error) error - subscribeStatusChanges(irma.RequestorToken) (chan statusChange, error) + subscribeUpdates(irma.RequestorToken) (chan *sessionData, error) stop() } -type statusChange struct { - irma.FrontendSessionStatus - Timeout time.Duration -} - type memorySessionStore struct { sync.RWMutex conf *server.Configuration requestor map[irma.RequestorToken]*memorySessionData client map[irma.ClientToken]*memorySessionData - statusChannels map[irma.RequestorToken][]chan statusChange + updateChannels map[irma.RequestorToken][]chan *sessionData } type memorySessionData struct { @@ -131,60 +126,38 @@ func (s *memorySessionStore) add(session *sessionData) error { func (s *memorySessionStore) transaction(t irma.RequestorToken, handler func(session *sessionData) error) error { s.RLock() - ses := s.requestor[t] + memSes := s.requestor[t] s.RUnlock() - if ses == nil { + if memSes == nil { return server.LogError(&UnknownSessionError{t, ""}) } - - ses.Lock() - copy, err := copyObject(ses.sessionData) - ses.Unlock() - if err != nil { - return err - } - sesCopy := copy.(*sessionData) - - if err := s.handleTransaction(sesCopy, handler); err != nil { - return err - } - - s.Lock() - defer s.Unlock() - s.requestor[t].sessionData = sesCopy - return nil + return s.handleTransaction(memSes, handler) } func (s *memorySessionStore) clientTransaction(t irma.ClientToken, handler func(session *sessionData) error) error { s.RLock() - ses := s.client[t] + memSes := s.client[t] s.RUnlock() - if ses == nil { + if memSes == nil { return server.LogError(&UnknownSessionError{"", t}) } + return s.handleTransaction(memSes, handler) +} - ses.Lock() - copy, err := copyObject(ses.sessionData) - ses.Unlock() +func (s *memorySessionStore) handleTransaction(memSes *memorySessionData, handler func(session *sessionData) error) error { + // The session struct contains pointers to other structs, so we need to give the handler a deep copy to prevent side effects. + ses := &sessionData{} + memSes.Lock() + err := copyObject(memSes.sessionData, ses) + memSes.Unlock() if err != nil { return err } - sesCopy := copy.(*sessionData) - if err := s.handleTransaction(sesCopy, handler); err != nil { - return err - } - - s.Lock() - defer s.Unlock() - s.client[t].sessionData = sesCopy - return nil -} - -func (s *memorySessionStore) handleTransaction(ses *sessionData, handler func(session *sessionData) error) error { - prevStatus := ses.Status + // Hashing the current session data needs to take place before the timeout check to detect all changes. + hashBefore := ses.hash() if !ses.Status.Finished() && ses.timeout(s.conf) <= 0 { s.conf.Logger.WithFields(logrus.Fields{"session": ses.RequestorToken}).Info("Session expired") @@ -195,25 +168,39 @@ func (s *memorySessionStore) handleTransaction(ses *sessionData, handler func(se return err } - if prevStatus != ses.Status { - for _, channel := range s.statusChannels[ses.RequestorToken] { - channel <- statusChange{ - FrontendSessionStatus: irma.FrontendSessionStatus{ - Status: ses.Status, - NextSession: ses.Next, - }, - Timeout: ses.timeout(s.conf), - } - } + // Check if the session has changed. + hashAfter := ses.hash() + if hashBefore == hashAfter { + return nil } + + // Make a deep copy of the session data, so we can update it in memory without side effects. + sesCopy := &sessionData{} + if err := copyObject(ses, sesCopy); err != nil { + return err + } + + // Check if the session has changed by another routine, and if not, update it in memory. + memSes.Lock() + defer memSes.Unlock() + if memSes.hash() != hashBefore { + return errors.New("session changed by another routine") + } + memSes.sessionData = sesCopy + + go func() { + for _, channel := range s.updateChannels[ses.RequestorToken] { + channel <- ses + } + }() return nil } -func (s *memorySessionStore) subscribeStatusChanges(token irma.RequestorToken) (chan statusChange, error) { - statusChan := make(chan statusChange, 4) +func (s *memorySessionStore) subscribeUpdates(token irma.RequestorToken) (chan *sessionData, error) { + statusChan := make(chan *sessionData) s.Lock() defer s.Unlock() - s.statusChannels[token] = append(s.statusChannels[token], statusChan) + s.updateChannels[token] = append(s.updateChannels[token], statusChan) return statusChan, nil } @@ -221,7 +208,7 @@ func (s *memorySessionStore) stop() { s.Lock() defer s.Unlock() for _, session := range s.requestor { - for _, channel := range s.statusChannels[session.RequestorToken] { + for _, channel := range s.updateChannels[session.RequestorToken] { close(channel) } } @@ -250,12 +237,16 @@ func (s *memorySessionStore) deleteExpired() { // Using a write lock, delete the expired sessions s.Lock() + defer s.Unlock() for _, token := range expired { session := s.requestor[token] delete(s.client, session.ClientToken) delete(s.requestor, token) + for _, channel := range s.updateChannels[token] { + close(channel) + } + delete(s.updateChannels, token) } - s.Unlock() } func (s *redisSessionStore) add(session *sessionData) error { @@ -318,7 +309,7 @@ func (s *redisSessionStore) clientTransaction(t irma.ClientToken, handler func(s s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("Session received from Redis datastore") - // Hashing the current session data needs to take place before the timeout check to detect all changes! + // Hashing the current session data needs to take place before the timeout check to detect all changes. hashBefore := session.hash() // Timeout check @@ -360,7 +351,7 @@ func (s *redisSessionStore) clientTransaction(t irma.ClientToken, handler func(s return nil } -func (s *redisSessionStore) subscribeStatusChanges(token irma.RequestorToken) (chan statusChange, error) { +func (s *redisSessionStore) subscribeUpdates(token irma.RequestorToken) (chan *sessionData, error) { return nil, errors.New("not implemented") } From b57f836e8cadf01dbc4b0e1576409ba8cd90d619 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Wed, 18 Oct 2023 12:38:37 +0200 Subject: [PATCH 04/25] Improvement: log if deleting expired sessions fails --- server/irmaserver/sessions.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server/irmaserver/sessions.go b/server/irmaserver/sessions.go index 11ff34a3e..033f8bae3 100644 --- a/server/irmaserver/sessions.go +++ b/server/irmaserver/sessions.go @@ -226,13 +226,15 @@ func (s *memorySessionStore) deleteExpired() { expired := make([]irma.RequestorToken, 0, len(toCheck)) for token := range toCheck { - s.transaction(token, func(session *sessionData) error { + if err := s.transaction(token, func(session *sessionData) error { if session.ttl(s.conf) <= 0 { - s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Deleting session") + s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Deleting expired session") expired = append(expired, token) } return nil - }) + }); err != nil { + s.conf.Logger.WithFields(logrus.Fields{"session": token}).WithError(err).Error("Error while deleting expired session") + } } // Using a write lock, delete the expired sessions From 9b6c9aca2c46b20e34e2deced3c86c33ec4a78d3 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Wed, 18 Oct 2023 18:08:56 +0200 Subject: [PATCH 05/25] Fix: sessionMiddleware writing wrong status code on error --- server/api.go | 52 ++++++++++++++ server/irmaserver/api.go | 99 +++++--------------------- server/irmaserver/helpers.go | 94 ++++++++++++++++++++++-- server/irmaserver/sessions.go | 51 +++++-------- server/keyshare/myirmaserver/server.go | 18 ++--- 5 files changed, 183 insertions(+), 131 deletions(-) diff --git a/server/api.go b/server/api.go index 7b8e47a21..1df7ed306 100644 --- a/server/api.go +++ b/server/api.go @@ -606,3 +606,55 @@ func FilterStopError(err error) error { } return err } + +type HTTPResponseRecorder struct { + wrapped http.ResponseWriter + Flushed bool + + statusCode int + header http.Header + body []byte +} + +func NewHTTPResponseRecorder(w http.ResponseWriter) *HTTPResponseRecorder { + return &HTTPResponseRecorder{ + wrapped: w, + header: w.Header().Clone(), + } +} + +// Header implements http.ResponseWriter +func (r *HTTPResponseRecorder) Header() http.Header { + return r.header +} + +// Write implements http.ResponseWriter +func (r *HTTPResponseRecorder) Write(b []byte) (int, error) { + r.body = append(r.body, b...) + return len(b), nil +} + +// WriteHeader implements http.ResponseWriter +func (r *HTTPResponseRecorder) WriteHeader(statusCode int) { + r.statusCode = statusCode +} + +// Flush implements http.Flusher. +func (r *HTTPResponseRecorder) Flush() { + if !r.Flushed { + for k, v := range r.Header() { + r.wrapped.Header()[k] = v + } + if r.statusCode > 0 { + r.wrapped.WriteHeader(r.statusCode) + } + r.Flushed = true + } + if len(r.body) > 0 { + r.wrapped.Write(r.body) + if flusher, ok := r.wrapped.(http.Flusher); ok { + flusher.Flush() + } + r.body = nil + } +} diff --git a/server/irmaserver/api.go b/server/irmaserver/api.go index 1d0f86605..12b4c4718 100644 --- a/server/irmaserver/api.go +++ b/server/irmaserver/api.go @@ -6,8 +6,6 @@ package irmaserver import ( "context" - "encoding/json" - "fmt" "net/http" "net/url" "sync" @@ -282,9 +280,9 @@ func (s *Server) startNextSession( } for update := range updateChan { if update.Status.Finished() { - if err := s.sessions.transaction(ses.RequestorToken, func(ses *sessionData) error { + if err := s.sessions.transaction(ses.RequestorToken, func(ses *sessionData) (bool, error) { handler(ses.Result) - return nil + return false, nil }); err != nil { s.conf.Logger.WithError(err).Error("Failed to execute session handler") } @@ -322,9 +320,9 @@ func GetSessionResult(requestorToken irma.RequestorToken) (*server.SessionResult return s.GetSessionResult(requestorToken) } func (s *Server) GetSessionResult(requestorToken irma.RequestorToken) (res *server.SessionResult, err error) { - err = s.sessions.transaction(requestorToken, func(session *sessionData) error { + err = s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { res = session.Result - return nil + return false, nil }) return } @@ -334,9 +332,9 @@ func GetRequest(requestorToken irma.RequestorToken) (irma.RequestorRequest, erro return s.GetRequest(requestorToken) } func (s *Server) GetRequest(requestorToken irma.RequestorToken) (req irma.RequestorRequest, err error) { - err = s.sessions.transaction(requestorToken, func(session *sessionData) error { + err = s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { req = session.Rrequest - return nil + return false, nil }) return } @@ -346,9 +344,9 @@ func CancelSession(requestorToken irma.RequestorToken) error { return s.CancelSession(requestorToken) } func (s *Server) CancelSession(requestorToken irma.RequestorToken) (err error) { - err = s.sessions.transaction(requestorToken, func(session *sessionData) error { + err = s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { session.handleDelete(s.conf) - return nil + return true, nil }) return } @@ -361,9 +359,9 @@ func SetFrontendOptions(requestorToken irma.RequestorToken, request *irma.Fronte return s.SetFrontendOptions(requestorToken, request) } func (s *Server) SetFrontendOptions(requestorToken irma.RequestorToken, request *irma.FrontendOptionsRequest) (o *irma.SessionOptions, err error) { - err = s.sessions.transaction(requestorToken, func(session *sessionData) error { + err = s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { o, err = session.updateFrontendOptions(request) - return err + return true, err }) return } @@ -374,8 +372,8 @@ func PairingCompleted(requestorToken irma.RequestorToken) error { return s.PairingCompleted(requestorToken) } func (s *Server) PairingCompleted(requestorToken irma.RequestorToken) error { - return s.sessions.transaction(requestorToken, func(session *sessionData) error { - return session.pairingCompleted(s.conf) + return s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { + return true, session.pairingCompleted(s.conf) }) } @@ -396,8 +394,8 @@ func (s *Server) SubscribeServerSentEvents(w http.ResponseWriter, r *http.Reques Component: server.ComponentSession, Arg: string(token), })) - return s.sessions.transaction(token, func(session *sessionData) error { - return s.subscribeServerSentEvents(w, r, session, true) + return s.sessions.transaction(token, func(session *sessionData) (bool, error) { + return false, s.subscribeServerSentEvents(w, r, session, true) }) } @@ -460,71 +458,6 @@ func (s *Server) subscribeServerSentEvents(w http.ResponseWriter, r *http.Reques return nil } -func (s *Server) serverSentEventsHandler(initialSession *sessionData, updateChan chan *sessionData) { - timeoutTime := time.Now().Add(initialSession.timeout(s.conf)) - - // Close the channels when this function returns. - defer func() { - s.serverSentEvents.CloseChannel("session/" + string(initialSession.RequestorToken)) - s.serverSentEvents.CloseChannel("session/" + string(initialSession.ClientToken)) - s.serverSentEvents.CloseChannel("frontendsession/" + string(initialSession.ClientToken)) - }() - - currStatus := initialSession.Status - for { - select { - case update, ok := <-updateChan: - if !ok { - return - } - if currStatus == update.Status { - continue - } - currStatus = update.Status - - frontendStatusBytes, err := json.Marshal(update.frontendSessionStatus()) - if err != nil { - s.conf.Logger.Error(err) - return - } - - s.serverSentEvents.SendMessage("session/"+string(update.RequestorToken), - sse.SimpleMessage(fmt.Sprintf(`"%s"`, currStatus)), - ) - s.serverSentEvents.SendMessage("session/"+string(update.ClientToken), - sse.SimpleMessage(fmt.Sprintf(`"%s"`, currStatus)), - ) - s.serverSentEvents.SendMessage("frontendsession/"+string(update.ClientToken), - sse.SimpleMessage(string(frontendStatusBytes)), - ) - if currStatus.Finished() { - return - } - timeoutTime = time.Now().Add(update.timeout(s.conf)) - case <-time.After(time.Until(timeoutTime)): - frontendStatus := irma.FrontendSessionStatus{ - Status: irma.ServerStatusTimeout, - } - frontendStatusBytes, err := json.Marshal(frontendStatus) - if err != nil { - s.conf.Logger.Error(err) - return - } - - s.serverSentEvents.SendMessage("session/"+string(initialSession.RequestorToken), - sse.SimpleMessage(fmt.Sprintf(`"%s"`, frontendStatus.Status)), - ) - s.serverSentEvents.SendMessage("session/"+string(initialSession.ClientToken), - sse.SimpleMessage(fmt.Sprintf(`"%s"`, frontendStatus.Status)), - ) - s.serverSentEvents.SendMessage("frontendsession/"+string(initialSession.ClientToken), - sse.SimpleMessage(string(frontendStatusBytes)), - ) - return - } - } -} - // SessionStatus retrieves a channel on which the current session status of the specified // IRMA session can be retrieved. func SessionStatus(requestorToken irma.RequestorToken) (chan irma.ServerStatus, error) { @@ -540,9 +473,9 @@ func (s *Server) SessionStatus(requestorToken irma.RequestorToken) (statusChan c return nil, err } var timeoutTime time.Time - if err := s.sessions.transaction(requestorToken, func(session *sessionData) error { + if err := s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { timeoutTime = time.Now().Add(session.timeout(s.conf)) - return nil + return false, nil }); err != nil { return nil, err } diff --git a/server/irmaserver/helpers.go b/server/irmaserver/helpers.go index d72694467..a4c1fe067 100644 --- a/server/irmaserver/helpers.go +++ b/server/irmaserver/helpers.go @@ -5,6 +5,7 @@ import ( "context" "crypto/sha256" "encoding/json" + "fmt" "io" "log" "net/http" @@ -565,14 +566,27 @@ func (s *Server) sessionMiddleware(next http.Handler) http.Handler { return } - if err := s.sessions.clientTransaction(token, func(session *sessionData) error { + recorder := server.NewHTTPResponseRecorder(w) + if err := s.sessions.clientTransaction(token, func(session *sessionData) (bool, error) { expectedHost := session.Rrequest.SessionRequest().Base().Host if expectedHost != "" && expectedHost != r.Host { - server.WriteError(w, server.ErrorUnauthorized, "Host mismatch") - return nil + server.WriteError(recorder, server.ErrorUnauthorized, "Host mismatch") + return false, nil } - next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), "session", session))) + hashBefore := session.hash() + next.ServeHTTP(recorder, r.WithContext(context.WithValue(r.Context(), "session", session))) + hashAfter := session.hash() + sessionUpdated := hashBefore != hashAfter + + // SSE bypasses the middleware and flushes the response writer directly. + // SSE should not have changed the session state, so we return here. + if recorder.Flushed { + if sessionUpdated { + return false, errors.New("handler flushed the response writer and changed session state") + } + return false, nil + } // Write session result to context for irmac.go functions. result := session.Result @@ -581,15 +595,18 @@ func (s *Server) sessionMiddleware(next http.Handler) http.Handler { *resultValue.(*server.SessionResult) = *result } - return nil + return sessionUpdated, nil }); err != nil { - if _, ok := err.(*UnknownSessionError); ok { + if recorder.Flushed { + s.conf.Logger.WithError(err).Error("Session middleware: error could not be written to client") + } else if _, ok := err.(*UnknownSessionError); ok { server.WriteError(w, server.ErrorSessionUnknown, "") } else { server.WriteError(w, server.ErrorInternal, "") } return } + recorder.Flush() }) } @@ -617,3 +634,68 @@ func (s *Server) pairingMiddleware(next http.Handler) http.Handler { next.ServeHTTP(w, r) }) } + +func (s *Server) serverSentEventsHandler(initialSession *sessionData, updateChan chan *sessionData) { + timeoutTime := time.Now().Add(initialSession.timeout(s.conf)) + + // Close the channels when this function returns. + defer func() { + s.serverSentEvents.CloseChannel("session/" + string(initialSession.RequestorToken)) + s.serverSentEvents.CloseChannel("session/" + string(initialSession.ClientToken)) + s.serverSentEvents.CloseChannel("frontendsession/" + string(initialSession.ClientToken)) + }() + + currStatus := initialSession.Status + for { + select { + case update, ok := <-updateChan: + if !ok { + return + } + if currStatus == update.Status { + continue + } + currStatus = update.Status + + frontendStatusBytes, err := json.Marshal(update.frontendSessionStatus()) + if err != nil { + s.conf.Logger.Error(err) + return + } + + s.serverSentEvents.SendMessage("session/"+string(update.RequestorToken), + sse.SimpleMessage(fmt.Sprintf(`"%s"`, currStatus)), + ) + s.serverSentEvents.SendMessage("session/"+string(update.ClientToken), + sse.SimpleMessage(fmt.Sprintf(`"%s"`, currStatus)), + ) + s.serverSentEvents.SendMessage("frontendsession/"+string(update.ClientToken), + sse.SimpleMessage(string(frontendStatusBytes)), + ) + if currStatus.Finished() { + return + } + timeoutTime = time.Now().Add(update.timeout(s.conf)) + case <-time.After(time.Until(timeoutTime)): + frontendStatus := irma.FrontendSessionStatus{ + Status: irma.ServerStatusTimeout, + } + frontendStatusBytes, err := json.Marshal(frontendStatus) + if err != nil { + s.conf.Logger.Error(err) + return + } + + s.serverSentEvents.SendMessage("session/"+string(initialSession.RequestorToken), + sse.SimpleMessage(fmt.Sprintf(`"%s"`, frontendStatus.Status)), + ) + s.serverSentEvents.SendMessage("session/"+string(initialSession.ClientToken), + sse.SimpleMessage(fmt.Sprintf(`"%s"`, frontendStatus.Status)), + ) + s.serverSentEvents.SendMessage("frontendsession/"+string(initialSession.ClientToken), + sse.SimpleMessage(string(frontendStatusBytes)), + ) + return + } + } +} diff --git a/server/irmaserver/sessions.go b/server/irmaserver/sessions.go index 033f8bae3..66334c6ae 100644 --- a/server/irmaserver/sessions.go +++ b/server/irmaserver/sessions.go @@ -49,8 +49,8 @@ type responseCache struct { type sessionStore interface { add(*sessionData) error - transaction(irma.RequestorToken, func(*sessionData) error) error - clientTransaction(irma.ClientToken, func(*sessionData) error) error + transaction(irma.RequestorToken, func(*sessionData) (bool, error)) error + clientTransaction(irma.ClientToken, func(*sessionData) (bool, error)) error subscribeUpdates(irma.RequestorToken) (chan *sessionData, error) stop() } @@ -124,7 +124,7 @@ func (s *memorySessionStore) add(session *sessionData) error { return nil } -func (s *memorySessionStore) transaction(t irma.RequestorToken, handler func(session *sessionData) error) error { +func (s *memorySessionStore) transaction(t irma.RequestorToken, handler func(session *sessionData) (bool, error)) error { s.RLock() memSes := s.requestor[t] s.RUnlock() @@ -135,7 +135,7 @@ func (s *memorySessionStore) transaction(t irma.RequestorToken, handler func(ses return s.handleTransaction(memSes, handler) } -func (s *memorySessionStore) clientTransaction(t irma.ClientToken, handler func(session *sessionData) error) error { +func (s *memorySessionStore) clientTransaction(t irma.ClientToken, handler func(session *sessionData) (bool, error)) error { s.RLock() memSes := s.client[t] s.RUnlock() @@ -146,47 +146,39 @@ func (s *memorySessionStore) clientTransaction(t irma.ClientToken, handler func( return s.handleTransaction(memSes, handler) } -func (s *memorySessionStore) handleTransaction(memSes *memorySessionData, handler func(session *sessionData) error) error { +func (s *memorySessionStore) handleTransaction(memSes *memorySessionData, handler func(session *sessionData) (bool, error)) error { // The session struct contains pointers to other structs, so we need to give the handler a deep copy to prevent side effects. + sesBefore := memSes.sessionData ses := &sessionData{} memSes.Lock() - err := copyObject(memSes.sessionData, ses) + err := copyObject(sesBefore, ses) memSes.Unlock() if err != nil { return err } - // Hashing the current session data needs to take place before the timeout check to detect all changes. - hashBefore := ses.hash() - if !ses.Status.Finished() && ses.timeout(s.conf) <= 0 { s.conf.Logger.WithFields(logrus.Fields{"session": ses.RequestorToken}).Info("Session expired") ses.setStatus(irma.ServerStatusTimeout, s.conf) } - if err := handler(ses); err != nil { + if update, err := handler(ses); !update || err != nil { return err } - // Check if the session has changed. - hashAfter := ses.hash() - if hashBefore == hashAfter { - return nil - } - // Make a deep copy of the session data, so we can update it in memory without side effects. - sesCopy := &sessionData{} - if err := copyObject(ses, sesCopy); err != nil { + sesAfter := &sessionData{} + if err := copyObject(ses, sesAfter); err != nil { return err } // Check if the session has changed by another routine, and if not, update it in memory. memSes.Lock() defer memSes.Unlock() - if memSes.hash() != hashBefore { + if sesBefore != memSes.sessionData { return errors.New("session changed by another routine") } - memSes.sessionData = sesCopy + memSes.sessionData = sesAfter go func() { for _, channel := range s.updateChannels[ses.RequestorToken] { @@ -226,12 +218,12 @@ func (s *memorySessionStore) deleteExpired() { expired := make([]irma.RequestorToken, 0, len(toCheck)) for token := range toCheck { - if err := s.transaction(token, func(session *sessionData) error { + if err := s.transaction(token, func(session *sessionData) (bool, error) { if session.ttl(s.conf) <= 0 { s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Deleting expired session") expired = append(expired, token) } - return nil + return false, nil }); err != nil { s.conf.Logger.WithFields(logrus.Fields{"session": token}).WithError(err).Error("Error while deleting expired session") } @@ -276,7 +268,7 @@ func (s *redisSessionStore) add(session *sessionData) error { return nil } -func (s *redisSessionStore) transaction(t irma.RequestorToken, handler func(session *sessionData) error) error { +func (s *redisSessionStore) transaction(t irma.RequestorToken, handler func(session *sessionData) (bool, error)) error { val, err := s.client.Get(context.Background(), requestorTokenLookupPrefix+string(t)).Result() if err == redis.Nil { return server.LogError(&UnknownSessionError{t, ""}) @@ -293,7 +285,7 @@ func (s *redisSessionStore) transaction(t irma.RequestorToken, handler func(sess return s.clientTransaction(clientToken, handler) } -func (s *redisSessionStore) clientTransaction(t irma.ClientToken, handler func(session *sessionData) error) error { +func (s *redisSessionStore) clientTransaction(t irma.ClientToken, handler func(session *sessionData) (bool, error)) error { if err := s.client.Watch(context.Background(), func(tx *redis.Tx) error { getResult := tx.Get(context.Background(), clientTokenLookupPrefix+string(t)) if getResult.Err() == redis.Nil { @@ -311,25 +303,16 @@ func (s *redisSessionStore) clientTransaction(t irma.ClientToken, handler func(s s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("Session received from Redis datastore") - // Hashing the current session data needs to take place before the timeout check to detect all changes. - hashBefore := session.hash() - // Timeout check if !session.Status.Finished() && session.timeout(s.conf) <= 0 { s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Session expired") session.setStatus(irma.ServerStatusTimeout, s.conf) } - if err := handler(session); err != nil { + if update, err := handler(session); !update || err != nil { return err } - // Check if the session has changed - hashAfter := session.hash() - if hashBefore == hashAfter { - return nil - } - // If the session has changed, update it in Redis sessionJSON, err := json.Marshal(session) if err != nil { diff --git a/server/keyshare/myirmaserver/server.go b/server/keyshare/myirmaserver/server.go index fd8d61553..2c03f9a78 100644 --- a/server/keyshare/myirmaserver/server.go +++ b/server/keyshare/myirmaserver/server.go @@ -733,26 +733,28 @@ func (s *Server) handleAddEmail(w http.ResponseWriter, r *http.Request) { func (s *Server) sessionMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + recorder := server.NewHTTPResponseRecorder(w) if err := s.sessionFromCookie(r, func(session *session) error { if session.UserID == nil { s.conf.Logger.Info("Malformed request: user not logged in") - server.WriteError(w, server.ErrorInvalidRequest, "not logged in") + server.WriteError(recorder, server.ErrorInvalidRequest, "not logged in") return nil } - next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), "session", session))) + next.ServeHTTP(recorder, r.WithContext(context.WithValue(r.Context(), "session", session))) return nil }); err != nil { - if err == errUnknownSession { + if recorder.Flushed { + s.conf.Logger.WithError(err).Error("Session middleware: error could not be written to client") + } else if err == errUnknownSession { server.WriteError(w, server.ErrorInvalidRequest, "not logged in") - return + } else { + keyshare.WriteError(w, err) } - - // TODO: this might lead to a duplicate write to w. Check if this is a problem. - // This also happens in userMiddleware in irmaserver. - keyshare.WriteError(w, err) + return } + recorder.Flush() }) } From c1c31d382d8895f11e67ba6a3bdf9562689b6a8b Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Wed, 18 Oct 2023 15:40:53 +0200 Subject: [PATCH 06/25] Improvement: use context in sessionStore --- server/irmaserver/api.go | 31 ++++--- server/irmaserver/helpers.go | 61 +++++++++++- server/irmaserver/sessions.go | 117 +++++++----------------- server/irmaserver/sessions_test.go | 8 +- server/keyshare/myirmaserver/session.go | 21 +++-- 5 files changed, 129 insertions(+), 109 deletions(-) diff --git a/server/irmaserver/api.go b/server/irmaserver/api.go index 12b4c4718..9ea60af9b 100644 --- a/server/irmaserver/api.go +++ b/server/irmaserver/api.go @@ -256,7 +256,7 @@ func (s *Server) startNextSession( } request.Base().DevelopmentMode = !s.conf.Production - ses, err := s.newSession(action, rrequest, disclosed, FrontendAuth) + ses, err := s.newSession(context.Background(), action, rrequest, disclosed, FrontendAuth) if err != nil { return nil, "", nil, err } @@ -273,14 +273,16 @@ func (s *Server) startNextSession( if handler != nil { go func() { - updateChan, err := s.sessions.subscribeUpdates(ses.RequestorToken) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + updateChan, err := s.sessions.subscribeUpdates(ctx, ses.RequestorToken) if err != nil { s.conf.Logger.WithError(err).Error("Failed to subscribe to session status changes for handler") return } for update := range updateChan { if update.Status.Finished() { - if err := s.sessions.transaction(ses.RequestorToken, func(ses *sessionData) (bool, error) { + if err := s.sessions.transaction(ctx, ses.RequestorToken, func(ses *sessionData) (bool, error) { handler(ses.Result) return false, nil }); err != nil { @@ -320,7 +322,7 @@ func GetSessionResult(requestorToken irma.RequestorToken) (*server.SessionResult return s.GetSessionResult(requestorToken) } func (s *Server) GetSessionResult(requestorToken irma.RequestorToken) (res *server.SessionResult, err error) { - err = s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { + err = s.sessions.transaction(context.Background(), requestorToken, func(session *sessionData) (bool, error) { res = session.Result return false, nil }) @@ -332,7 +334,7 @@ func GetRequest(requestorToken irma.RequestorToken) (irma.RequestorRequest, erro return s.GetRequest(requestorToken) } func (s *Server) GetRequest(requestorToken irma.RequestorToken) (req irma.RequestorRequest, err error) { - err = s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { + err = s.sessions.transaction(context.Background(), requestorToken, func(session *sessionData) (bool, error) { req = session.Rrequest return false, nil }) @@ -344,7 +346,7 @@ func CancelSession(requestorToken irma.RequestorToken) error { return s.CancelSession(requestorToken) } func (s *Server) CancelSession(requestorToken irma.RequestorToken) (err error) { - err = s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { + err = s.sessions.transaction(context.Background(), requestorToken, func(session *sessionData) (bool, error) { session.handleDelete(s.conf) return true, nil }) @@ -359,7 +361,7 @@ func SetFrontendOptions(requestorToken irma.RequestorToken, request *irma.Fronte return s.SetFrontendOptions(requestorToken, request) } func (s *Server) SetFrontendOptions(requestorToken irma.RequestorToken, request *irma.FrontendOptionsRequest) (o *irma.SessionOptions, err error) { - err = s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { + err = s.sessions.transaction(context.Background(), requestorToken, func(session *sessionData) (bool, error) { o, err = session.updateFrontendOptions(request) return true, err }) @@ -372,7 +374,7 @@ func PairingCompleted(requestorToken irma.RequestorToken) error { return s.PairingCompleted(requestorToken) } func (s *Server) PairingCompleted(requestorToken irma.RequestorToken) error { - return s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { + return s.sessions.transaction(context.Background(), requestorToken, func(session *sessionData) (bool, error) { return true, session.pairingCompleted(s.conf) }) } @@ -394,7 +396,7 @@ func (s *Server) SubscribeServerSentEvents(w http.ResponseWriter, r *http.Reques Component: server.ComponentSession, Arg: string(token), })) - return s.sessions.transaction(token, func(session *sessionData) (bool, error) { + return s.sessions.transaction(context.Background(), token, func(session *sessionData) (bool, error) { return false, s.subscribeServerSentEvents(w, r, session, true) }) } @@ -425,7 +427,7 @@ func (s *Server) subscribeServerSentEvents(w http.ResponseWriter, r *http.Reques s.activeSSEHandlersMutex.Unlock() if !activeHandler { - updateChan, err := s.sessions.subscribeUpdates(session.RequestorToken) + updateChan, err := s.sessions.subscribeUpdates(context.Background(), session.RequestorToken) if err != nil { return err } @@ -468,20 +470,25 @@ func (s *Server) SessionStatus(requestorToken irma.RequestorToken) (statusChan c return nil, errors.New("SessionStatus cannot be used in combination with Redis/etcd.") } - updateChan, err := s.sessions.subscribeUpdates(requestorToken) + ctx, cancel := context.WithCancel(context.Background()) + updateChan, err := s.sessions.subscribeUpdates(ctx, requestorToken) if err != nil { + cancel() return nil, err } var timeoutTime time.Time - if err := s.sessions.transaction(requestorToken, func(session *sessionData) (bool, error) { + if err := s.sessions.transaction(ctx, requestorToken, func(session *sessionData) (bool, error) { timeoutTime = time.Now().Add(session.timeout(s.conf)) return false, nil }); err != nil { + cancel() return nil, err } statusChan = make(chan irma.ServerStatus, 4) go func() { + defer cancel() + var currStatus irma.ServerStatus for { select { diff --git a/server/irmaserver/helpers.go b/server/irmaserver/helpers.go index a4c1fe067..26a00a6ee 100644 --- a/server/irmaserver/helpers.go +++ b/server/irmaserver/helpers.go @@ -10,6 +10,7 @@ import ( "log" "net/http" "reflect" + "strings" "time" "github.com/alexandrevicenzi/go-sse" @@ -27,6 +28,8 @@ import ( "github.com/sirupsen/logrus" ) +var one *big.Int = big.NewInt(1) + // Session helpers func (session *sessionData) markAlive(conf *server.Configuration) { @@ -567,7 +570,7 @@ func (s *Server) sessionMiddleware(next http.Handler) http.Handler { } recorder := server.NewHTTPResponseRecorder(w) - if err := s.sessions.clientTransaction(token, func(session *sessionData) (bool, error) { + if err := s.sessions.clientTransaction(r.Context(), token, func(session *sessionData) (bool, error) { expectedHost := session.Rrequest.SessionRequest().Base().Host if expectedHost != "" && expectedHost != r.Host { server.WriteError(recorder, server.ErrorUnauthorized, "Host mismatch") @@ -699,3 +702,59 @@ func (s *Server) serverSentEventsHandler(initialSession *sessionData, updateChan } } } + +func (s *Server) newSession( + ctx context.Context, + action irma.Action, + request irma.RequestorRequest, + disclosed irma.AttributeConDisCon, + frontendAuth irma.FrontendAuthorization, +) (*sessionData, error) { + clientToken := irma.ClientToken(common.NewSessionToken()) + requestorToken := irma.RequestorToken(common.NewSessionToken()) + if len(frontendAuth) == 0 { + frontendAuth = irma.FrontendAuthorization(common.NewSessionToken()) + } + + base := request.SessionRequest().Base() + if s.conf.AugmentClientReturnURL && base.AugmentReturnURL && base.ClientReturnURL != "" { + if strings.Contains(base.ClientReturnURL, "?") { + base.ClientReturnURL += "&token=" + string(requestorToken) + } else { + base.ClientReturnURL += "?token=" + string(requestorToken) + } + } + + ses := &sessionData{ + Action: action, + Rrequest: request, + LastActive: time.Now(), + RequestorToken: requestorToken, + ClientToken: clientToken, + Status: irma.ServerStatusInitialized, + Result: &server.SessionResult{ + LegacySession: request.SessionRequest().Base().Legacy(), + Token: requestorToken, + Type: action, + Status: irma.ServerStatusInitialized, + }, + Options: irma.SessionOptions{ + LDContext: irma.LDContextSessionOptions, + PairingMethod: irma.PairingMethodNone, + }, + FrontendAuth: frontendAuth, + ImplicitDisclosure: disclosed, + } + + s.conf.Logger.WithFields(logrus.Fields{"session": ses.RequestorToken}).Debug("New session started") + nonce, _ := gabi.GenerateNonce() + base.Nonce = nonce + base.Context = one + + err := s.sessions.add(ctx, ses) + if err != nil { + return nil, err + } + + return ses, nil +} diff --git a/server/irmaserver/sessions.go b/server/irmaserver/sessions.go index 66334c6ae..15715acbf 100644 --- a/server/irmaserver/sessions.go +++ b/server/irmaserver/sessions.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "strings" "sync" "time" @@ -12,9 +11,7 @@ import ( "github.com/go-redis/redis/v8" "github.com/privacybydesign/gabi" - "github.com/privacybydesign/gabi/big" irma "github.com/privacybydesign/irmago" - "github.com/privacybydesign/irmago/internal/common" "github.com/privacybydesign/irmago/server" "github.com/sirupsen/logrus" @@ -48,10 +45,10 @@ type responseCache struct { } type sessionStore interface { - add(*sessionData) error - transaction(irma.RequestorToken, func(*sessionData) (bool, error)) error - clientTransaction(irma.ClientToken, func(*sessionData) (bool, error)) error - subscribeUpdates(irma.RequestorToken) (chan *sessionData, error) + add(context.Context, *sessionData) error + transaction(context.Context, irma.RequestorToken, func(*sessionData) (bool, error)) error + clientTransaction(context.Context, irma.ClientToken, func(*sessionData) (bool, error)) error + subscribeUpdates(context.Context, irma.RequestorToken) (chan *sessionData, error) stop() } @@ -115,7 +112,7 @@ var ( maxFrontendProtocolVersion = irma.NewVersion(1, 1) ) -func (s *memorySessionStore) add(session *sessionData) error { +func (s *memorySessionStore) add(ctx context.Context, session *sessionData) error { s.Lock() defer s.Unlock() memSes := &memorySessionData{sessionData: session} @@ -124,7 +121,7 @@ func (s *memorySessionStore) add(session *sessionData) error { return nil } -func (s *memorySessionStore) transaction(t irma.RequestorToken, handler func(session *sessionData) (bool, error)) error { +func (s *memorySessionStore) transaction(ctx context.Context, t irma.RequestorToken, handler func(session *sessionData) (bool, error)) error { s.RLock() memSes := s.requestor[t] s.RUnlock() @@ -135,7 +132,7 @@ func (s *memorySessionStore) transaction(t irma.RequestorToken, handler func(ses return s.handleTransaction(memSes, handler) } -func (s *memorySessionStore) clientTransaction(t irma.ClientToken, handler func(session *sessionData) (bool, error)) error { +func (s *memorySessionStore) clientTransaction(ctx context.Context, t irma.ClientToken, handler func(session *sessionData) (bool, error)) error { s.RLock() memSes := s.client[t] s.RUnlock() @@ -188,7 +185,7 @@ func (s *memorySessionStore) handleTransaction(memSes *memorySessionData, handle return nil } -func (s *memorySessionStore) subscribeUpdates(token irma.RequestorToken) (chan *sessionData, error) { +func (s *memorySessionStore) subscribeUpdates(ctx context.Context, token irma.RequestorToken) (chan *sessionData, error) { statusChan := make(chan *sessionData) s.Lock() defer s.Unlock() @@ -218,7 +215,7 @@ func (s *memorySessionStore) deleteExpired() { expired := make([]irma.RequestorToken, 0, len(toCheck)) for token := range toCheck { - if err := s.transaction(token, func(session *sessionData) (bool, error) { + if err := s.transaction(context.Background(), token, func(session *sessionData) (bool, error) { if session.ttl(s.conf) <= 0 { s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Deleting expired session") expired = append(expired, token) @@ -243,33 +240,37 @@ func (s *memorySessionStore) deleteExpired() { } } -func (s *redisSessionStore) add(session *sessionData) error { +func (s *redisSessionStore) add(ctx context.Context, session *sessionData) error { sessionJSON, err := json.Marshal(session) if err != nil { return server.LogError(err) } ttl := session.ttl(s.conf) - conn := s.client.Conn(context.Background()) - if err := conn.Set(context.Background(), requestorTokenLookupPrefix+string(session.RequestorToken), string(session.ClientToken), ttl).Err(); err != nil { - return logAsRedisError(err) - } - if err := conn.Set(context.Background(), clientTokenLookupPrefix+string(session.ClientToken), sessionJSON, ttl).Err(); err != nil { - return logAsRedisError(err) - } + if err := s.client.Watch(ctx, func(tx *redis.Tx) error { + if err := tx.Set(ctx, requestorTokenLookupPrefix+string(session.RequestorToken), string(session.ClientToken), ttl).Err(); err != nil { + return err + } + if err := tx.Set(ctx, clientTokenLookupPrefix+string(session.ClientToken), sessionJSON, ttl).Err(); err != nil { + return err + } - if s.client.FailoverMode { - if err := s.client.Wait(context.Background(), 1, time.Second).Err(); err != nil { - return logAsRedisError(err) + if s.client.FailoverMode { + if err := s.client.Wait(ctx, 1, time.Second).Err(); err != nil { + return err + } } + return nil + }); err != nil { + return logAsRedisError(err) } s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("session added or updated in Redis datastore") return nil } -func (s *redisSessionStore) transaction(t irma.RequestorToken, handler func(session *sessionData) (bool, error)) error { - val, err := s.client.Get(context.Background(), requestorTokenLookupPrefix+string(t)).Result() +func (s *redisSessionStore) transaction(ctx context.Context, t irma.RequestorToken, handler func(session *sessionData) (bool, error)) error { + val, err := s.client.Get(ctx, requestorTokenLookupPrefix+string(t)).Result() if err == redis.Nil { return server.LogError(&UnknownSessionError{t, ""}) } else if err != nil { @@ -282,12 +283,12 @@ func (s *redisSessionStore) transaction(t irma.RequestorToken, handler func(sess } s.conf.Logger.WithFields(logrus.Fields{"session": t, "clientToken": clientToken}).Debug("clientToken found in Redis datastore") - return s.clientTransaction(clientToken, handler) + return s.clientTransaction(ctx, clientToken, handler) } -func (s *redisSessionStore) clientTransaction(t irma.ClientToken, handler func(session *sessionData) (bool, error)) error { - if err := s.client.Watch(context.Background(), func(tx *redis.Tx) error { - getResult := tx.Get(context.Background(), clientTokenLookupPrefix+string(t)) +func (s *redisSessionStore) clientTransaction(ctx context.Context, t irma.ClientToken, handler func(session *sessionData) (bool, error)) error { + if err := s.client.Watch(ctx, func(tx *redis.Tx) error { + getResult := tx.Get(ctx, clientTokenLookupPrefix+string(t)) if getResult.Err() == redis.Nil { // Both session and error need to be returned. The session will already be locked and needs to // be passed along, so it can be unlocked later. @@ -319,13 +320,13 @@ func (s *redisSessionStore) clientTransaction(t irma.ClientToken, handler func(s return server.LogError(err) } - err = tx.Set(context.Background(), clientTokenLookupPrefix+string(t), sessionJSON, 0).Err() + err = tx.Set(ctx, clientTokenLookupPrefix+string(t), sessionJSON, 0).Err() if err != nil { return logAsRedisError(err) } if s.client.FailoverMode { - if err := tx.Wait(context.Background(), 1, time.Second).Err(); err != nil { + if err := tx.Wait(ctx, 1, time.Second).Err(); err != nil { return logAsRedisError(err) } } @@ -336,7 +337,7 @@ func (s *redisSessionStore) clientTransaction(t irma.ClientToken, handler func(s return nil } -func (s *redisSessionStore) subscribeUpdates(token irma.RequestorToken) (chan *sessionData, error) { +func (s *redisSessionStore) subscribeUpdates(ctx context.Context, token irma.RequestorToken) (chan *sessionData, error) { return nil, errors.New("not implemented") } @@ -348,58 +349,6 @@ func (s *redisSessionStore) stop() { s.conf.Logger.Info("Redis client closed successfully") } -var one *big.Int = big.NewInt(1) - -func (s *Server) newSession(action irma.Action, request irma.RequestorRequest, disclosed irma.AttributeConDisCon, FrontendAuth irma.FrontendAuthorization) (*sessionData, error) { - clientToken := irma.ClientToken(common.NewSessionToken()) - requestorToken := irma.RequestorToken(common.NewSessionToken()) - if len(FrontendAuth) == 0 { - FrontendAuth = irma.FrontendAuthorization(common.NewSessionToken()) - } - - base := request.SessionRequest().Base() - if s.conf.AugmentClientReturnURL && base.AugmentReturnURL && base.ClientReturnURL != "" { - if strings.Contains(base.ClientReturnURL, "?") { - base.ClientReturnURL += "&token=" + string(requestorToken) - } else { - base.ClientReturnURL += "?token=" + string(requestorToken) - } - } - - ses := &sessionData{ - Action: action, - Rrequest: request, - LastActive: time.Now(), - RequestorToken: requestorToken, - ClientToken: clientToken, - Status: irma.ServerStatusInitialized, - Result: &server.SessionResult{ - LegacySession: request.SessionRequest().Base().Legacy(), - Token: requestorToken, - Type: action, - Status: irma.ServerStatusInitialized, - }, - Options: irma.SessionOptions{ - LDContext: irma.LDContextSessionOptions, - PairingMethod: irma.PairingMethodNone, - }, - FrontendAuth: FrontendAuth, - ImplicitDisclosure: disclosed, - } - - s.conf.Logger.WithFields(logrus.Fields{"session": ses.RequestorToken}).Debug("New session started") - nonce, _ := gabi.GenerateNonce() - base.Nonce = nonce - base.Context = one - - err := s.sessions.add(ses) - if err != nil { - return nil, err - } - - return ses, nil -} - func logAsRedisError(err error) error { return server.LogError(&RedisError{err}) } diff --git a/server/irmaserver/sessions_test.go b/server/irmaserver/sessions_test.go index a3bbce4af..d7aafaa25 100644 --- a/server/irmaserver/sessions_test.go +++ b/server/irmaserver/sessions_test.go @@ -1,11 +1,13 @@ package irmaserver import ( - "github.com/privacybydesign/irmago/internal/test" + "context" "path/filepath" "testing" "time" + "github.com/privacybydesign/irmago/internal/test" + irma "github.com/privacybydesign/irmago" "github.com/privacybydesign/irmago/server" "github.com/sirupsen/logrus" @@ -76,7 +78,7 @@ func TestMemoryStoreNoDeadlock(t *testing.T) { req, err := server.ParseSessionRequest(`{"request":{"@context":"https://irma.app/ld/request/disclosure/v2","context":"AQ==","nonce":"MtILupG0g0J23GNR1YtupQ==","devMode":true,"disclose":[[[{"type":"test.test.email.email","value":"example@example.com"}]]]}}`) require.NoError(t, err) - session, err := s.newSession(irma.ActionDisclosing, req, nil, "") + session, err := s.newSession(context.Background(), irma.ActionDisclosing, req, nil, "") require.NoError(t, err) memSessions, ok := s.sessions.(*memorySessionStore) @@ -103,7 +105,7 @@ func TestMemoryStoreNoDeadlock(t *testing.T) { // Make a new session; this involves adding it to the memory session store. go func() { - _, _ = s.newSession(irma.ActionDisclosing, req, nil, "") + _, _ = s.newSession(context.Background(), irma.ActionDisclosing, req, nil, "") addingCompleted = true }() diff --git a/server/keyshare/myirmaserver/session.go b/server/keyshare/myirmaserver/session.go index f4ef34f96..e1c57157e 100644 --- a/server/keyshare/myirmaserver/session.go +++ b/server/keyshare/myirmaserver/session.go @@ -91,16 +91,19 @@ func (s *redisSessionStore) add(ctx context.Context, ses session) error { } ttl := time.Until(ses.Expiry).Seconds() - conn := s.client.Conn(ctx) - if err := conn.Set(ctx, sessionLookupPrefix+ses.Token, string(bytes), time.Duration(ttl)*time.Second).Err(); err != nil { - s.logger.WithError(err).Error("failed to store session") - return errRedis - } - if s.client.FailoverMode { - if err := conn.Wait(ctx, 1, time.Second).Err(); err != nil { - s.logger.WithError(err).Error("failed to persist session") - return errRedis + if err := s.client.Watch(ctx, func(tx *redis.Tx) error { + if err := tx.Set(ctx, sessionLookupPrefix+ses.Token, string(bytes), time.Duration(ttl)*time.Second).Err(); err != nil { + return err + } + if s.client.FailoverMode { + if err := tx.Wait(ctx, 1, time.Second).Err(); err != nil { + return err + } } + return nil + }); err != nil { + s.logger.WithError(err).Error("failed to add session") + return errRedis } return nil } From 6b61dad03d8a3982df7cedef52182313c33ea6f7 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Wed, 18 Oct 2023 23:21:46 +0200 Subject: [PATCH 07/25] Chore: prevent superfluous logging --- server/conf.go | 2 +- server/irmaserver/api.go | 2 +- server/irmaserver/helpers.go | 3 --- server/irmaserver/sessions.go | 10 ++++++++-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/server/conf.go b/server/conf.go index 8c2a6a7ee..a4927b025 100644 --- a/server/conf.go +++ b/server/conf.go @@ -160,7 +160,7 @@ func (conf *Configuration) Check() error { } if conf.EnableSSE && conf.StoreType == "redis" { - return errors.New("Currently server-sent events (SSE) cannot be used simultaneously with the Redis/etcd session store.") + return errors.New("Currently server-sent events (SSE) cannot be used simultaneously with the Redis session store.") } return nil diff --git a/server/irmaserver/api.go b/server/irmaserver/api.go index 9ea60af9b..2bd883481 100644 --- a/server/irmaserver/api.go +++ b/server/irmaserver/api.go @@ -467,7 +467,7 @@ func SessionStatus(requestorToken irma.RequestorToken) (chan irma.ServerStatus, } func (s *Server) SessionStatus(requestorToken irma.RequestorToken) (statusChan chan irma.ServerStatus, err error) { if s.conf.StoreType == "redis" { - return nil, errors.New("SessionStatus cannot be used in combination with Redis/etcd.") + return nil, errors.New("SessionStatus cannot be used in combination with Redis.") } ctx, cancel := context.WithCancel(context.Background()) diff --git a/server/irmaserver/helpers.go b/server/irmaserver/helpers.go index 26a00a6ee..07b897e85 100644 --- a/server/irmaserver/helpers.go +++ b/server/irmaserver/helpers.go @@ -40,9 +40,6 @@ func (session *sessionData) markAlive(conf *server.Configuration) { } func (session *sessionData) setStatus(status irma.ServerStatus, conf *server.Configuration) { - conf.Logger. - WithFields(logrus.Fields{"session": session.RequestorToken, "status": status}). - Info("Session status updated") session.Status = status session.Result.Status = status diff --git a/server/irmaserver/sessions.go b/server/irmaserver/sessions.go index 15715acbf..de7c0c520 100644 --- a/server/irmaserver/sessions.go +++ b/server/irmaserver/sessions.go @@ -155,7 +155,6 @@ func (s *memorySessionStore) handleTransaction(memSes *memorySessionData, handle } if !ses.Status.Finished() && ses.timeout(s.conf) <= 0 { - s.conf.Logger.WithFields(logrus.Fields{"session": ses.RequestorToken}).Info("Session expired") ses.setStatus(irma.ServerStatusTimeout, s.conf) } @@ -163,6 +162,10 @@ func (s *memorySessionStore) handleTransaction(memSes *memorySessionData, handle return err } + s.conf.Logger. + WithFields(logrus.Fields{"session": ses.RequestorToken, "status": ses.Status}). + Info("Session updated") + // Make a deep copy of the session data, so we can update it in memory without side effects. sesAfter := &sessionData{} if err := copyObject(ses, sesAfter); err != nil { @@ -306,7 +309,6 @@ func (s *redisSessionStore) clientTransaction(ctx context.Context, t irma.Client // Timeout check if !session.Status.Finished() && session.timeout(s.conf) <= 0 { - s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Info("Session expired") session.setStatus(irma.ServerStatusTimeout, s.conf) } @@ -314,6 +316,10 @@ func (s *redisSessionStore) clientTransaction(ctx context.Context, t irma.Client return err } + s.conf.Logger. + WithFields(logrus.Fields{"session": session.RequestorToken, "status": session.Status}). + Info("Session updated") + // If the session has changed, update it in Redis sessionJSON, err := json.Marshal(session) if err != nil { From da4d890969800a5e8c9276654549b8713c1871f5 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Wed, 18 Oct 2023 23:58:15 +0200 Subject: [PATCH 08/25] Feat: add support for Redis ACLs --- irma/cmd/config.go | 2 ++ irma/cmd/keyshare-myirma.go | 2 ++ irma/cmd/keyshare-server.go | 2 ++ irma/cmd/server.go | 2 ++ server/conf.go | 21 ++++++++++++++++++--- server/irmaserver/sessions.go | 15 ++++++++++----- server/keyshare/myirmaserver/session.go | 9 +++++++-- 7 files changed, 43 insertions(+), 10 deletions(-) diff --git a/irma/cmd/config.go b/irma/cmd/config.go index 4fed7e350..7a2473536 100644 --- a/irma/cmd/config.go +++ b/irma/cmd/config.go @@ -83,9 +83,11 @@ func configureIRMAServer() (*server.Configuration, error) { return nil, errors.New("When Redis is used as session data store, either --redis-addr or --redis-sentinel-addrs must be specified.") } + conf.RedisSettings.Username = viper.GetString("redis_user") if conf.RedisSettings.Password = viper.GetString("redis_pw"); conf.RedisSettings.Password == "" && !viper.GetBool("redis_allow_empty_password") { return nil, errors.New("When Redis is used as session data store, a non-empty Redis password must be specified with the --redis-pw flag. This restriction can be relaxed by setting the --redis-allow-empty-password flag to true.") } + conf.RedisSettings.ACLPrefix = viper.GetString("redis_acl_prefix") conf.RedisSettings.DB = viper.GetInt("redis_db") diff --git a/irma/cmd/keyshare-myirma.go b/irma/cmd/keyshare-myirma.go index 3afb5a1fd..57f96300b 100644 --- a/irma/cmd/keyshare-myirma.go +++ b/irma/cmd/keyshare-myirma.go @@ -66,8 +66,10 @@ func init() { flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") + flags.String("redis-user", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") + flags.String("redis-acl-prefix", "", "Redis ACL prefix for keys") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") flags.String("redis-tls-cert", "", "use Redis TLS with specific certificate or certificate authority") flags.String("redis-tls-cert-file", "", "use Redis TLS path to specific certificate or certificate authority") diff --git a/irma/cmd/keyshare-server.go b/irma/cmd/keyshare-server.go index 5b7ba871e..b4b0b9df3 100644 --- a/irma/cmd/keyshare-server.go +++ b/irma/cmd/keyshare-server.go @@ -63,8 +63,10 @@ func init() { flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") + flags.String("redis-user", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") + flags.String("redis-acl-prefix", "", "Redis ACL prefix for keys") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") flags.String("redis-tls-cert", "", "use Redis TLS with specific certificate or certificate authority") flags.String("redis-tls-cert-file", "", "use Redis TLS path to specific certificate or certificate authority") diff --git a/irma/cmd/server.go b/irma/cmd/server.go index 23479c3c6..d661d175e 100644 --- a/irma/cmd/server.go +++ b/irma/cmd/server.go @@ -127,8 +127,10 @@ func setFlags(cmd *cobra.Command, production bool) error { flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") + flags.String("redis-user", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") + flags.String("redis-acl-prefix", "", "Redis ACL prefix for keys") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") flags.String("redis-tls-cert", "", "use Redis TLS with specific certificate or certificate authority") flags.String("redis-tls-cert-file", "", "use Redis TLS path to specific certificate or certificate authority") diff --git a/server/conf.go b/server/conf.go index a4927b025..0edef7c7c 100644 --- a/server/conf.go +++ b/server/conf.go @@ -105,6 +105,7 @@ type Configuration struct { type RedisClient struct { *redis.Client FailoverMode bool + KeyPrefix string } type RedisSettings struct { @@ -113,9 +114,15 @@ type RedisSettings struct { SentinelMasterName string `json:"sentinel_master_name,omitempty" mapstructure:"sentinel_master_name"` AcceptInconsistencyRisk bool `json:"accept_inconsistency_risk,omitempty" mapstructure:"accept_inconsistency_risk"` + // Username for Redis authentication. If username is empty, the default user is used. + Username string `json:"username,omitempty" mapstructure:"username"` + // Password for Redis authentication. Password string `json:"password,omitempty" mapstructure:"password"` - DB int `json:"db,omitempty" mapstructure:"db"` - // TODO: ACL prefix? + // ACLPrefix prefixes all Redis keys with the specified string in the format "aclprefix:key". + // This can be used for key permissions in the Redis ACL system. If ACLPrefix is empty, no prefix is used. + ACLPrefix string `json:"acl_prefix,omitempty" mapstructure:"acl_prefix"` + + DB int `json:"db,omitempty" mapstructure:"db"` TLSCertificate string `json:"tls_cert,omitempty" mapstructure:"tls_cert"` TLSCertificateFile string `json:"tls_cert_file,omitempty" mapstructure:"tls_cert_file"` @@ -475,7 +482,15 @@ func (conf *Configuration) RedisClient() (*RedisClient, error) { conf.Logger.Warn("Redis replication factor is less than 2, this may cause availability issues") } } - conf.redisClient = &RedisClient{Client: cl, FailoverMode: failoverMode} + var keyPrefix string + if conf.RedisSettings.ACLPrefix != "" { + keyPrefix = conf.RedisSettings.ACLPrefix + ":" + } + conf.redisClient = &RedisClient{ + Client: cl, + FailoverMode: failoverMode, + KeyPrefix: keyPrefix, + } return conf.redisClient, nil } diff --git a/server/irmaserver/sessions.go b/server/irmaserver/sessions.go index de7c0c520..2d06244f6 100644 --- a/server/irmaserver/sessions.go +++ b/server/irmaserver/sessions.go @@ -251,10 +251,15 @@ func (s *redisSessionStore) add(ctx context.Context, session *sessionData) error ttl := session.ttl(s.conf) if err := s.client.Watch(ctx, func(tx *redis.Tx) error { - if err := tx.Set(ctx, requestorTokenLookupPrefix+string(session.RequestorToken), string(session.ClientToken), ttl).Err(); err != nil { + if err := tx.Set( + ctx, + s.client.KeyPrefix+requestorTokenLookupPrefix+string(session.RequestorToken), + string(session.ClientToken), + ttl, + ).Err(); err != nil { return err } - if err := tx.Set(ctx, clientTokenLookupPrefix+string(session.ClientToken), sessionJSON, ttl).Err(); err != nil { + if err := tx.Set(ctx, s.client.KeyPrefix+string(session.ClientToken), sessionJSON, ttl).Err(); err != nil { return err } @@ -273,7 +278,7 @@ func (s *redisSessionStore) add(ctx context.Context, session *sessionData) error } func (s *redisSessionStore) transaction(ctx context.Context, t irma.RequestorToken, handler func(session *sessionData) (bool, error)) error { - val, err := s.client.Get(ctx, requestorTokenLookupPrefix+string(t)).Result() + val, err := s.client.Get(ctx, s.client.KeyPrefix+requestorTokenLookupPrefix+string(t)).Result() if err == redis.Nil { return server.LogError(&UnknownSessionError{t, ""}) } else if err != nil { @@ -291,7 +296,7 @@ func (s *redisSessionStore) transaction(ctx context.Context, t irma.RequestorTok func (s *redisSessionStore) clientTransaction(ctx context.Context, t irma.ClientToken, handler func(session *sessionData) (bool, error)) error { if err := s.client.Watch(ctx, func(tx *redis.Tx) error { - getResult := tx.Get(ctx, clientTokenLookupPrefix+string(t)) + getResult := tx.Get(ctx, s.client.KeyPrefix+clientTokenLookupPrefix+string(t)) if getResult.Err() == redis.Nil { // Both session and error need to be returned. The session will already be locked and needs to // be passed along, so it can be unlocked later. @@ -326,7 +331,7 @@ func (s *redisSessionStore) clientTransaction(ctx context.Context, t irma.Client return server.LogError(err) } - err = tx.Set(ctx, clientTokenLookupPrefix+string(t), sessionJSON, 0).Err() + err = tx.Set(ctx, s.client.KeyPrefix+clientTokenLookupPrefix+string(t), sessionJSON, 0).Err() if err != nil { return logAsRedisError(err) } diff --git a/server/keyshare/myirmaserver/session.go b/server/keyshare/myirmaserver/session.go index e1c57157e..c303976e2 100644 --- a/server/keyshare/myirmaserver/session.go +++ b/server/keyshare/myirmaserver/session.go @@ -92,7 +92,12 @@ func (s *redisSessionStore) add(ctx context.Context, ses session) error { ttl := time.Until(ses.Expiry).Seconds() if err := s.client.Watch(ctx, func(tx *redis.Tx) error { - if err := tx.Set(ctx, sessionLookupPrefix+ses.Token, string(bytes), time.Duration(ttl)*time.Second).Err(); err != nil { + if err := tx.Set( + ctx, + s.client.KeyPrefix+sessionLookupPrefix+ses.Token, + string(bytes), + time.Duration(ttl)*time.Second, + ).Err(); err != nil { return err } if s.client.FailoverMode { @@ -109,7 +114,7 @@ func (s *redisSessionStore) add(ctx context.Context, ses session) error { } func (s *redisSessionStore) update(ctx context.Context, token string, handler func(ses *session) error) error { - key := sessionLookupPrefix + token + key := s.client.KeyPrefix + sessionLookupPrefix + token err := s.client.Watch(ctx, func(tx *redis.Tx) error { bytes, err := tx.Get(ctx, key).Bytes() From c05bfe7f5d747a8f6e4bbd1a3879134f8c813fbb Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 19 Oct 2023 09:43:57 +0200 Subject: [PATCH 09/25] Improvement: use username as ACL key prefix --- irma/cmd/config.go | 2 +- irma/cmd/keyshare-myirma.go | 4 ++-- irma/cmd/keyshare-server.go | 4 ++-- irma/cmd/server.go | 4 ++-- server/conf.go | 10 +++++----- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/irma/cmd/config.go b/irma/cmd/config.go index 7a2473536..9024e8318 100644 --- a/irma/cmd/config.go +++ b/irma/cmd/config.go @@ -87,7 +87,7 @@ func configureIRMAServer() (*server.Configuration, error) { if conf.RedisSettings.Password = viper.GetString("redis_pw"); conf.RedisSettings.Password == "" && !viper.GetBool("redis_allow_empty_password") { return nil, errors.New("When Redis is used as session data store, a non-empty Redis password must be specified with the --redis-pw flag. This restriction can be relaxed by setting the --redis-allow-empty-password flag to true.") } - conf.RedisSettings.ACLPrefix = viper.GetString("redis_acl_prefix") + conf.RedisSettings.ACLUseKeyPrefixes = viper.GetBool("redis_acl_use_key_prefixes") conf.RedisSettings.DB = viper.GetInt("redis_db") diff --git a/irma/cmd/keyshare-myirma.go b/irma/cmd/keyshare-myirma.go index 57f96300b..25721415a 100644 --- a/irma/cmd/keyshare-myirma.go +++ b/irma/cmd/keyshare-myirma.go @@ -66,10 +66,10 @@ func init() { flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") - flags.String("redis-user", "", "Redis server username (when using ACLs)") + flags.String("redis-username", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") - flags.String("redis-acl-prefix", "", "Redis ACL prefix for keys") + flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username (username:key)") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") flags.String("redis-tls-cert", "", "use Redis TLS with specific certificate or certificate authority") flags.String("redis-tls-cert-file", "", "use Redis TLS path to specific certificate or certificate authority") diff --git a/irma/cmd/keyshare-server.go b/irma/cmd/keyshare-server.go index b4b0b9df3..bb26a30b9 100644 --- a/irma/cmd/keyshare-server.go +++ b/irma/cmd/keyshare-server.go @@ -63,10 +63,10 @@ func init() { flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") - flags.String("redis-user", "", "Redis server username (when using ACLs)") + flags.String("redis-username", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") - flags.String("redis-acl-prefix", "", "Redis ACL prefix for keys") + flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username (username:key)") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") flags.String("redis-tls-cert", "", "use Redis TLS with specific certificate or certificate authority") flags.String("redis-tls-cert-file", "", "use Redis TLS path to specific certificate or certificate authority") diff --git a/irma/cmd/server.go b/irma/cmd/server.go index d661d175e..84b8c2be8 100644 --- a/irma/cmd/server.go +++ b/irma/cmd/server.go @@ -127,10 +127,10 @@ func setFlags(cmd *cobra.Command, production bool) error { flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") - flags.String("redis-user", "", "Redis server username (when using ACLs)") + flags.String("redis-username", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") - flags.String("redis-acl-prefix", "", "Redis ACL prefix for keys") + flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username (username:key)") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") flags.String("redis-tls-cert", "", "use Redis TLS with specific certificate or certificate authority") flags.String("redis-tls-cert-file", "", "use Redis TLS path to specific certificate or certificate authority") diff --git a/server/conf.go b/server/conf.go index 0edef7c7c..7ec731e0f 100644 --- a/server/conf.go +++ b/server/conf.go @@ -118,9 +118,9 @@ type RedisSettings struct { Username string `json:"username,omitempty" mapstructure:"username"` // Password for Redis authentication. Password string `json:"password,omitempty" mapstructure:"password"` - // ACLPrefix prefixes all Redis keys with the specified string in the format "aclprefix:key". - // This can be used for key permissions in the Redis ACL system. If ACLPrefix is empty, no prefix is used. - ACLPrefix string `json:"acl_prefix,omitempty" mapstructure:"acl_prefix"` + // ACLUseKeyPrefixes ensures all Redis keys are prefixed with the username in the format "username:key". + // This can be used for key permissions in the Redis ACL system. If ACLUseKeyPrefixes is false, no prefix is used. + ACLUseKeyPrefixes bool `json:"acl_use_key_prefixes,omitempty" mapstructure:"acl_use_key_prefixes"` DB int `json:"db,omitempty" mapstructure:"db"` @@ -483,8 +483,8 @@ func (conf *Configuration) RedisClient() (*RedisClient, error) { } } var keyPrefix string - if conf.RedisSettings.ACLPrefix != "" { - keyPrefix = conf.RedisSettings.ACLPrefix + ":" + if conf.RedisSettings.ACLUseKeyPrefixes { + keyPrefix = conf.RedisSettings.Username + ":" } conf.redisClient = &RedisClient{ Client: cl, From e4fd8a7b3733d495979ab6200fc0866b8594894e Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 19 Oct 2023 09:49:03 +0200 Subject: [PATCH 10/25] Chore: hide Redis Cluster in help texts --- irma/cmd/keyshare-myirma.go | 4 ++-- irma/cmd/keyshare-server.go | 4 ++-- irma/cmd/server.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/irma/cmd/keyshare-myirma.go b/irma/cmd/keyshare-myirma.go index 25721415a..5c191655e 100644 --- a/irma/cmd/keyshare-myirma.go +++ b/irma/cmd/keyshare-myirma.go @@ -65,11 +65,11 @@ func init() { flags.String("redis-addr", "", "Redis address, to be specified as host:port") flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") - flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") + flags.Bool("redis-accept-inconsistency-risk", false, "accept the risk of inconsistent session state when using Redis Sentinel") flags.String("redis-username", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") - flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username (username:key)") + flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username for ACLs (username:key)") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") flags.String("redis-tls-cert", "", "use Redis TLS with specific certificate or certificate authority") flags.String("redis-tls-cert-file", "", "use Redis TLS path to specific certificate or certificate authority") diff --git a/irma/cmd/keyshare-server.go b/irma/cmd/keyshare-server.go index bb26a30b9..87b567adf 100644 --- a/irma/cmd/keyshare-server.go +++ b/irma/cmd/keyshare-server.go @@ -62,11 +62,11 @@ func init() { flags.String("redis-addr", "", "Redis address, to be specified as host:port") flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") - flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") + flags.Bool("redis-accept-inconsistency-risk", false, "accept the risk of inconsistent session state when using Redis Sentinel") flags.String("redis-username", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") - flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username (username:key)") + flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username for ACLs (username:key)") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") flags.String("redis-tls-cert", "", "use Redis TLS with specific certificate or certificate authority") flags.String("redis-tls-cert-file", "", "use Redis TLS path to specific certificate or certificate authority") diff --git a/irma/cmd/server.go b/irma/cmd/server.go index 84b8c2be8..d029b72c7 100644 --- a/irma/cmd/server.go +++ b/irma/cmd/server.go @@ -126,11 +126,11 @@ func setFlags(cmd *cobra.Command, production bool) error { flags.String("redis-addr", "", "Redis address, to be specified as host:port") flags.StringSlice("redis-sentinel-addrs", nil, "Redis Sentinel addresses, to be specified as host:port") flags.String("redis-sentinel-master-name", "", "Redis Sentinel master name") - flags.Bool("redis-accept-inconsistency-risk", false, "Accept the risk of inconsistent session state when using Redis Sentinel or Redis Cluster") + flags.Bool("redis-accept-inconsistency-risk", false, "accept the risk of inconsistent session state when using Redis Sentinel") flags.String("redis-username", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") - flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username (username:key)") + flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username for ACLs (username:key)") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") flags.String("redis-tls-cert", "", "use Redis TLS with specific certificate or certificate authority") flags.String("redis-tls-cert-file", "", "use Redis TLS path to specific certificate or certificate authority") From b9f0fe807077cc15d964a25365fd6b826135c852 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 19 Oct 2023 10:33:08 +0200 Subject: [PATCH 11/25] Fix: redis-username flag is not read --- irma/cmd/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/irma/cmd/config.go b/irma/cmd/config.go index 9024e8318..4d77af21e 100644 --- a/irma/cmd/config.go +++ b/irma/cmd/config.go @@ -83,7 +83,7 @@ func configureIRMAServer() (*server.Configuration, error) { return nil, errors.New("When Redis is used as session data store, either --redis-addr or --redis-sentinel-addrs must be specified.") } - conf.RedisSettings.Username = viper.GetString("redis_user") + conf.RedisSettings.Username = viper.GetString("redis_username") if conf.RedisSettings.Password = viper.GetString("redis_pw"); conf.RedisSettings.Password == "" && !viper.GetBool("redis_allow_empty_password") { return nil, errors.New("When Redis is used as session data store, a non-empty Redis password must be specified with the --redis-pw flag. This restriction can be relaxed by setting the --redis-allow-empty-password flag to true.") } From e2336735ee21cc4c6440392b3f7b9bc650e96558 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 19 Oct 2023 10:42:56 +0200 Subject: [PATCH 12/25] Improvement: cancel context when session handler returns --- server/irmaserver/api.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/irmaserver/api.go b/server/irmaserver/api.go index 2bd883481..742dda74a 100644 --- a/server/irmaserver/api.go +++ b/server/irmaserver/api.go @@ -427,11 +427,14 @@ func (s *Server) subscribeServerSentEvents(w http.ResponseWriter, r *http.Reques s.activeSSEHandlersMutex.Unlock() if !activeHandler { - updateChan, err := s.sessions.subscribeUpdates(context.Background(), session.RequestorToken) + ctx, cancel := context.WithCancel(context.Background()) + updateChan, err := s.sessions.subscribeUpdates(ctx, session.RequestorToken) if err != nil { + cancel() return err } go func() { + defer cancel() s.serverSentEventsHandler(session, updateChan) s.activeSSEHandlersMutex.Lock() defer s.activeSSEHandlersMutex.Unlock() From 61043839366dfce2cf92da9d039f0414cc06b62c Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 19 Oct 2023 11:31:00 +0200 Subject: [PATCH 13/25] Chore: improve logging when adding session to Redis --- server/irmaserver/sessions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/irmaserver/sessions.go b/server/irmaserver/sessions.go index 2d06244f6..c921c0776 100644 --- a/server/irmaserver/sessions.go +++ b/server/irmaserver/sessions.go @@ -273,7 +273,7 @@ func (s *redisSessionStore) add(ctx context.Context, session *sessionData) error return logAsRedisError(err) } - s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("session added or updated in Redis datastore") + s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("Session added in Redis datastore") return nil } From 968f6c3e8c030edfd1953f65ccb2835f3059bdad Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 19 Oct 2023 11:44:40 +0200 Subject: [PATCH 14/25] Chore: updated CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d526647a..de27f93ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +### Added +- Support for Redis in Sentinel mode +- Redis support for `irma keyshare server` and `irma keyshare myirmaserver` + +### Changed +- Using optimistic locking in the `irma server` instead of pessimistic locking ### Internal - Fixed failing tests due to expired test.test2 idemix key From 1bc83e0cee1232055d7ff771f5c985321c3a9f37 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 19 Oct 2023 13:31:05 +0200 Subject: [PATCH 15/25] Fix: clientTokenLookupPrefix not added everywhere --- server/irmaserver/helpers.go | 2 ++ server/irmaserver/sessions.go | 44 ++++++++++++++------------------ server/requestorserver/server.go | 1 + 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/server/irmaserver/helpers.go b/server/irmaserver/helpers.go index 07b897e85..0c090b297 100644 --- a/server/irmaserver/helpers.go +++ b/server/irmaserver/helpers.go @@ -600,8 +600,10 @@ func (s *Server) sessionMiddleware(next http.Handler) http.Handler { if recorder.Flushed { s.conf.Logger.WithError(err).Error("Session middleware: error could not be written to client") } else if _, ok := err.(*UnknownSessionError); ok { + s.conf.Logger.WithError(err).Warn("Session middleware: unknown session") server.WriteError(w, server.ErrorSessionUnknown, "") } else { + s.conf.Logger.WithError(err).Error("Session middleware: error") server.WriteError(w, server.ErrorInternal, "") } return diff --git a/server/irmaserver/sessions.go b/server/irmaserver/sessions.go index c921c0776..dea2e72a6 100644 --- a/server/irmaserver/sessions.go +++ b/server/irmaserver/sessions.go @@ -127,7 +127,7 @@ func (s *memorySessionStore) transaction(ctx context.Context, t irma.RequestorTo s.RUnlock() if memSes == nil { - return server.LogError(&UnknownSessionError{t, ""}) + return &UnknownSessionError{t, ""} } return s.handleTransaction(memSes, handler) } @@ -138,7 +138,7 @@ func (s *memorySessionStore) clientTransaction(ctx context.Context, t irma.Clien s.RUnlock() if memSes == nil { - return server.LogError(&UnknownSessionError{"", t}) + return &UnknownSessionError{"", t} } return s.handleTransaction(memSes, handler) } @@ -246,7 +246,7 @@ func (s *memorySessionStore) deleteExpired() { func (s *redisSessionStore) add(ctx context.Context, session *sessionData) error { sessionJSON, err := json.Marshal(session) if err != nil { - return server.LogError(err) + return err } ttl := session.ttl(s.conf) @@ -257,20 +257,20 @@ func (s *redisSessionStore) add(ctx context.Context, session *sessionData) error string(session.ClientToken), ttl, ).Err(); err != nil { - return err + return &RedisError{err} } - if err := tx.Set(ctx, s.client.KeyPrefix+string(session.ClientToken), sessionJSON, ttl).Err(); err != nil { - return err + if err := tx.Set(ctx, s.client.KeyPrefix+clientTokenLookupPrefix+string(session.ClientToken), sessionJSON, ttl).Err(); err != nil { + return &RedisError{err} } if s.client.FailoverMode { if err := s.client.Wait(ctx, 1, time.Second).Err(); err != nil { - return err + return &RedisError{err} } } return nil }); err != nil { - return logAsRedisError(err) + return err } s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("Session added in Redis datastore") @@ -280,14 +280,14 @@ func (s *redisSessionStore) add(ctx context.Context, session *sessionData) error func (s *redisSessionStore) transaction(ctx context.Context, t irma.RequestorToken, handler func(session *sessionData) (bool, error)) error { val, err := s.client.Get(ctx, s.client.KeyPrefix+requestorTokenLookupPrefix+string(t)).Result() if err == redis.Nil { - return server.LogError(&UnknownSessionError{t, ""}) + return &UnknownSessionError{t, ""} } else if err != nil { - return logAsRedisError(err) + return &RedisError{err} } clientToken, err := irma.ParseClientToken(val) if err != nil { - return logAsRedisError(err) + return &RedisError{err} } s.conf.Logger.WithFields(logrus.Fields{"session": t, "clientToken": clientToken}).Debug("clientToken found in Redis datastore") @@ -298,16 +298,14 @@ func (s *redisSessionStore) clientTransaction(ctx context.Context, t irma.Client if err := s.client.Watch(ctx, func(tx *redis.Tx) error { getResult := tx.Get(ctx, s.client.KeyPrefix+clientTokenLookupPrefix+string(t)) if getResult.Err() == redis.Nil { - // Both session and error need to be returned. The session will already be locked and needs to - // be passed along, so it can be unlocked later. - return server.LogError(&UnknownSessionError{"", t}) + return &UnknownSessionError{"", t} } else if getResult.Err() != nil { - return logAsRedisError(getResult.Err()) + return &RedisError{getResult.Err()} } session := &sessionData{} if err := json.Unmarshal([]byte(getResult.Val()), &session); err != nil { - return logAsRedisError(err) + return err } s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("Session received from Redis datastore") @@ -328,22 +326,22 @@ func (s *redisSessionStore) clientTransaction(ctx context.Context, t irma.Client // If the session has changed, update it in Redis sessionJSON, err := json.Marshal(session) if err != nil { - return server.LogError(err) + return err } err = tx.Set(ctx, s.client.KeyPrefix+clientTokenLookupPrefix+string(t), sessionJSON, 0).Err() if err != nil { - return logAsRedisError(err) + return &RedisError{err} } if s.client.FailoverMode { if err := tx.Wait(ctx, 1, time.Second).Err(); err != nil { - return logAsRedisError(err) + return &RedisError{err} } } return nil }); err != nil { - return logAsRedisError(err) + return err } return nil } @@ -355,11 +353,7 @@ func (s *redisSessionStore) subscribeUpdates(ctx context.Context, token irma.Req func (s *redisSessionStore) stop() { err := s.client.Close() if err != nil { - _ = logAsRedisError(err) + s.conf.Logger.WithError(err).Error("Error closing Redis client") } s.conf.Logger.Info("Redis client closed successfully") } - -func logAsRedisError(err error) error { - return server.LogError(&RedisError{err}) -} diff --git a/server/requestorserver/server.go b/server/requestorserver/server.go index 363a1f514..0e5f2f2b3 100644 --- a/server/requestorserver/server.go +++ b/server/requestorserver/server.go @@ -522,6 +522,7 @@ func (s *Server) createSession(w http.ResponseWriter, requestor string, rrequest qr, requestorToken, frontendRequest, err := s.irmaserv.StartSession(rrequest, nil) if err != nil { if _, ok := err.(*irmaserver.RedisError); ok { + s.conf.Logger.WithError(err).Error("Failed to start session") server.WriteError(w, server.ErrorInternal, "") } else { server.WriteError(w, server.ErrorInvalidRequest, err.Error()) From 31bdffd530b4835860b5433cc908b8407f4f26ef Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 19 Oct 2023 20:01:15 +0200 Subject: [PATCH 16/25] Fix: ttl not always set correctly on Redis keys --- server/irmaserver/sessions.go | 43 ++++++++++++++------ server/keyshare/myirmaserver/server.go | 2 +- server/keyshare/myirmaserver/session.go | 13 ++++-- server/keyshare/myirmaserver/session_test.go | 22 ++++++++-- 4 files changed, 60 insertions(+), 20 deletions(-) diff --git a/server/irmaserver/sessions.go b/server/irmaserver/sessions.go index dea2e72a6..99cbd3abe 100644 --- a/server/irmaserver/sessions.go +++ b/server/irmaserver/sessions.go @@ -246,10 +246,13 @@ func (s *memorySessionStore) deleteExpired() { func (s *redisSessionStore) add(ctx context.Context, session *sessionData) error { sessionJSON, err := json.Marshal(session) if err != nil { - return err + return &RedisError{err} } ttl := session.ttl(s.conf) + if ttl <= 0 { + return &RedisError{errors.New("session ttl is in the past")} + } if err := s.client.Watch(ctx, func(tx *redis.Tx) error { if err := tx.Set( ctx, @@ -257,20 +260,25 @@ func (s *redisSessionStore) add(ctx context.Context, session *sessionData) error string(session.ClientToken), ttl, ).Err(); err != nil { - return &RedisError{err} + return err } - if err := tx.Set(ctx, s.client.KeyPrefix+clientTokenLookupPrefix+string(session.ClientToken), sessionJSON, ttl).Err(); err != nil { - return &RedisError{err} + if err := tx.Set( + ctx, + s.client.KeyPrefix+clientTokenLookupPrefix+string(session.ClientToken), + sessionJSON, + ttl, + ).Err(); err != nil { + return err } if s.client.FailoverMode { if err := s.client.Wait(ctx, 1, time.Second).Err(); err != nil { - return &RedisError{err} + return err } } return nil }); err != nil { - return err + return &RedisError{err} } s.conf.Logger.WithFields(logrus.Fields{"session": session.RequestorToken}).Debug("Session added in Redis datastore") @@ -295,12 +303,12 @@ func (s *redisSessionStore) transaction(ctx context.Context, t irma.RequestorTok } func (s *redisSessionStore) clientTransaction(ctx context.Context, t irma.ClientToken, handler func(session *sessionData) (bool, error)) error { - if err := s.client.Watch(ctx, func(tx *redis.Tx) error { + err := s.client.Watch(ctx, func(tx *redis.Tx) error { getResult := tx.Get(ctx, s.client.KeyPrefix+clientTokenLookupPrefix+string(t)) if getResult.Err() == redis.Nil { return &UnknownSessionError{"", t} } else if getResult.Err() != nil { - return &RedisError{getResult.Err()} + return getResult.Err() } session := &sessionData{} @@ -329,19 +337,28 @@ func (s *redisSessionStore) clientTransaction(ctx context.Context, t irma.Client return err } - err = tx.Set(ctx, s.client.KeyPrefix+clientTokenLookupPrefix+string(t), sessionJSON, 0).Err() - if err != nil { - return &RedisError{err} + ttl := session.ttl(s.conf) + if ttl <= 0 { + return errors.New("session ttl is in the past") } + if err := tx.Set(ctx, s.client.KeyPrefix+clientTokenLookupPrefix+string(t), sessionJSON, ttl).Err(); err != nil { + return err + } + if err := tx.Expire(ctx, s.client.KeyPrefix+requestorTokenLookupPrefix+string(session.RequestorToken), ttl).Err(); err != nil { + return err + } if s.client.FailoverMode { if err := tx.Wait(ctx, 1, time.Second).Err(); err != nil { - return &RedisError{err} + return err } } return nil - }); err != nil { + }) + if _, ok := err.(*UnknownSessionError); ok { return err + } else if err != nil { + return &RedisError{err} } return nil } diff --git a/server/keyshare/myirmaserver/server.go b/server/keyshare/myirmaserver/server.go index 2c03f9a78..fe9424b1a 100644 --- a/server/keyshare/myirmaserver/server.go +++ b/server/keyshare/myirmaserver/server.go @@ -45,7 +45,7 @@ func New(conf *Configuration) (*Server, error) { if err != nil { return nil, err } - store = &redisSessionStore{client: cl} + store = &redisSessionStore{client: cl, logger: conf.Logger} default: return nil, errors.New("unsupported session store type") } diff --git a/server/keyshare/myirmaserver/session.go b/server/keyshare/myirmaserver/session.go index c303976e2..de42aa5fa 100644 --- a/server/keyshare/myirmaserver/session.go +++ b/server/keyshare/myirmaserver/session.go @@ -90,13 +90,16 @@ func (s *redisSessionStore) add(ctx context.Context, ses session) error { return err } - ttl := time.Until(ses.Expiry).Seconds() + ttl := time.Until(ses.Expiry) + if ttl <= 0 { + return errors.New("session expiry time is in the past") + } if err := s.client.Watch(ctx, func(tx *redis.Tx) error { if err := tx.Set( ctx, s.client.KeyPrefix+sessionLookupPrefix+ses.Token, string(bytes), - time.Duration(ttl)*time.Second, + ttl, ).Err(); err != nil { return err } @@ -138,7 +141,11 @@ func (s *redisSessionStore) update(ctx context.Context, token string, handler fu return err } - if err := tx.Set(ctx, key, string(updatedBytes), time.Until(session.Expiry)).Err(); err != nil { + ttl := time.Until(session.Expiry) + if ttl <= 0 { + return errors.New("session expiry time is in the past") + } + if err := tx.Set(ctx, key, string(updatedBytes), ttl).Err(); err != nil { return err } diff --git a/server/keyshare/myirmaserver/session_test.go b/server/keyshare/myirmaserver/session_test.go index f75348108..9b7eae9a7 100644 --- a/server/keyshare/myirmaserver/session_test.go +++ b/server/keyshare/myirmaserver/session_test.go @@ -5,13 +5,27 @@ import ( "testing" "time" + // "github.com/alicebob/miniredis/v2" + "github.com/alicebob/miniredis/v2" + "github.com/go-redis/redis/v8" irma "github.com/privacybydesign/irmago" + "github.com/privacybydesign/irmago/server" "github.com/stretchr/testify/assert" ) -func TestSessions(t *testing.T) { - store := newMemorySessionStore() +func TestMemorySessionStore(t *testing.T) { + testSessions(t, newMemorySessionStore(), time.Sleep) +} + +func TestRedisSessionStore(t *testing.T) { + mr := miniredis.NewMiniRedis() + mr.Start() + defer mr.Close() + client := redis.NewClient(&redis.Options{Addr: mr.Host() + ":" + mr.Port()}) + testSessions(t, &redisSessionStore{client: &server.RedisClient{Client: client}, logger: server.Logger}, mr.FastForward) +} +func testSessions(t *testing.T, store sessionStore, sleepFn func(time.Duration)) { s := session{ Token: "token", Expiry: time.Now().Add(1 * time.Second), @@ -21,6 +35,8 @@ func TestSessions(t *testing.T) { session2, err := getSession(store, s.Token) assert.NoError(t, err) + assert.Equal(t, s.Expiry.Unix(), session2.Expiry.Unix()) + s.Expiry = session2.Expiry // Time is not exactly equal, so set it to the same value assert.Equal(t, s, session2) emailSessionToken := irma.RequestorToken("emailtoken") @@ -43,7 +59,7 @@ func TestSessions(t *testing.T) { assert.NoError(t, err) assert.Equal(t, session4.Token, s.Token) - time.Sleep(2 * time.Second) + sleepFn(2 * time.Second) store.flush() From 8a6ae1913b6d387ded25ecdfa20ffdae72cb7e71 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 19 Oct 2023 20:03:41 +0200 Subject: [PATCH 17/25] Chore: log proofP JWT only in trace mode --- server/irmaserver/helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/irmaserver/helpers.go b/server/irmaserver/helpers.go index 0c090b297..ee16edb8f 100644 --- a/server/irmaserver/helpers.go +++ b/server/irmaserver/helpers.go @@ -289,7 +289,7 @@ func (session *sessionData) getProofP(commitments *irma.IssueCommitmentMessage, if !contains { return nil, errors.Errorf("no keyshare proof included for scheme %s", scheme.Name()) } - conf.Logger.Debug("Parsing keyshare ProofP JWT: ", str) + conf.Logger.Trace("Parsing keyshare ProofP JWT: ", str) claims := &struct { jwt.StandardClaims ProofP *gabi.ProofP From 86334a1bea44f628ecd7e1b320ea9faf8ca198cc Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Thu, 19 Oct 2023 20:42:47 +0200 Subject: [PATCH 18/25] Fix: session result handler not called on timeout --- server/irmaserver/api.go | 59 ++++++------------------------------ server/irmaserver/helpers.go | 45 +++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 50 deletions(-) diff --git a/server/irmaserver/api.go b/server/irmaserver/api.go index 742dda74a..48323db74 100644 --- a/server/irmaserver/api.go +++ b/server/irmaserver/api.go @@ -273,16 +273,14 @@ func (s *Server) startNextSession( if handler != nil { go func() { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - updateChan, err := s.sessions.subscribeUpdates(ctx, ses.RequestorToken) + statusChan, err := s.sessionStatusChannel(context.Background(), ses.RequestorToken, ses.timeout(s.conf)) if err != nil { - s.conf.Logger.WithError(err).Error("Failed to subscribe to session status changes for handler") + s.conf.Logger.WithError(err).Error("Failed to subscribe to session status updates for handler") return } - for update := range updateChan { - if update.Status.Finished() { - if err := s.sessions.transaction(ctx, ses.RequestorToken, func(ses *sessionData) (bool, error) { + for status := range statusChan { + if status.Finished() { + if err := s.sessions.transaction(context.Background(), ses.RequestorToken, func(ses *sessionData) (bool, error) { handler(ses.Result) return false, nil }); err != nil { @@ -473,52 +471,13 @@ func (s *Server) SessionStatus(requestorToken irma.RequestorToken) (statusChan c return nil, errors.New("SessionStatus cannot be used in combination with Redis.") } - ctx, cancel := context.WithCancel(context.Background()) - updateChan, err := s.sessions.subscribeUpdates(ctx, requestorToken) - if err != nil { - cancel() - return nil, err - } - var timeoutTime time.Time - if err := s.sessions.transaction(ctx, requestorToken, func(session *sessionData) (bool, error) { - timeoutTime = time.Now().Add(session.timeout(s.conf)) + var timeout time.Duration + if err := s.sessions.transaction(context.Background(), requestorToken, func(session *sessionData) (bool, error) { + timeout = session.timeout(s.conf) return false, nil }); err != nil { - cancel() return nil, err } - statusChan = make(chan irma.ServerStatus, 4) - go func() { - defer cancel() - - var currStatus irma.ServerStatus - for { - select { - case update, ok := <-updateChan: - if !ok { - close(statusChan) - return - } - if currStatus == update.Status { - continue - } - currStatus = update.Status - - statusChan <- currStatus - - if currStatus.Finished() { - close(statusChan) - return - } - timeoutTime = time.Now().Add(update.timeout(s.conf)) - case <-time.After(time.Until(timeoutTime)): - statusChan <- irma.ServerStatusTimeout - close(statusChan) - return - } - } - }() - - return statusChan, nil + return s.sessionStatusChannel(context.Background(), requestorToken, timeout) } diff --git a/server/irmaserver/helpers.go b/server/irmaserver/helpers.go index ee16edb8f..a57f55f25 100644 --- a/server/irmaserver/helpers.go +++ b/server/irmaserver/helpers.go @@ -702,6 +702,51 @@ func (s *Server) serverSentEventsHandler(initialSession *sessionData, updateChan } } +func (s *Server) sessionStatusChannel(ctx context.Context, token irma.RequestorToken, initialTimeout time.Duration) ( + chan irma.ServerStatus, error) { + ctx, cancel := context.WithCancel(ctx) + updateChan, err := s.sessions.subscribeUpdates(ctx, token) + if err != nil { + cancel() + return nil, err + } + + statusChan := make(chan irma.ServerStatus, 4) + timeoutTime := time.Now().Add(initialTimeout) + go func() { + defer cancel() + + var currStatus irma.ServerStatus + for { + select { + case update, ok := <-updateChan: + if !ok { + close(statusChan) + return + } + if currStatus == update.Status { + continue + } + currStatus = update.Status + + statusChan <- currStatus + + if currStatus.Finished() { + close(statusChan) + return + } + timeoutTime = time.Now().Add(update.timeout(s.conf)) + case <-time.After(time.Until(timeoutTime)): + statusChan <- irma.ServerStatusTimeout + close(statusChan) + return + } + } + }() + + return statusChan, nil +} + func (s *Server) newSession( ctx context.Context, action irma.Action, From 2d7f091fce7378ee3be2bba62f09b01ee5817a77 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Tue, 24 Oct 2023 16:24:54 +0200 Subject: [PATCH 19/25] Chore: delete commented import --- server/keyshare/myirmaserver/session_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/server/keyshare/myirmaserver/session_test.go b/server/keyshare/myirmaserver/session_test.go index 9b7eae9a7..c3c4037aa 100644 --- a/server/keyshare/myirmaserver/session_test.go +++ b/server/keyshare/myirmaserver/session_test.go @@ -5,7 +5,6 @@ import ( "testing" "time" - // "github.com/alicebob/miniredis/v2" "github.com/alicebob/miniredis/v2" "github.com/go-redis/redis/v8" irma "github.com/privacybydesign/irmago" From 7b7fb4d68bf13f74c6076b2bc1e1d9e212576f9a Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Mon, 20 Nov 2023 10:23:12 +0100 Subject: [PATCH 20/25] Feat: use username and passport for Redis Sentinel auth --- irma/cmd/config.go | 2 ++ irma/cmd/keyshare-myirma.go | 2 ++ irma/cmd/keyshare-server.go | 2 ++ irma/cmd/server.go | 2 ++ server/conf.go | 19 ++++++++++++++----- 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/irma/cmd/config.go b/irma/cmd/config.go index 4d77af21e..b2c365453 100644 --- a/irma/cmd/config.go +++ b/irma/cmd/config.go @@ -87,6 +87,8 @@ func configureIRMAServer() (*server.Configuration, error) { if conf.RedisSettings.Password = viper.GetString("redis_pw"); conf.RedisSettings.Password == "" && !viper.GetBool("redis_allow_empty_password") { return nil, errors.New("When Redis is used as session data store, a non-empty Redis password must be specified with the --redis-pw flag. This restriction can be relaxed by setting the --redis-allow-empty-password flag to true.") } + conf.RedisSettings.SentinelUsername = viper.GetString("redis_sentinel_username") + conf.RedisSettings.SentinelPassword = viper.GetString("redis_sentinel_pw") conf.RedisSettings.ACLUseKeyPrefixes = viper.GetBool("redis_acl_use_key_prefixes") conf.RedisSettings.DB = viper.GetInt("redis_db") diff --git a/irma/cmd/keyshare-myirma.go b/irma/cmd/keyshare-myirma.go index 5c191655e..39f535526 100644 --- a/irma/cmd/keyshare-myirma.go +++ b/irma/cmd/keyshare-myirma.go @@ -68,6 +68,8 @@ func init() { flags.Bool("redis-accept-inconsistency-risk", false, "accept the risk of inconsistent session state when using Redis Sentinel") flags.String("redis-username", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") + flags.String("redis-sentinel-username", "", "Redis Sentinel username (when using ACLs)") + flags.String("redis-sentinel-pw", "", "Redis Sentinel password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username for ACLs (username:key)") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") diff --git a/irma/cmd/keyshare-server.go b/irma/cmd/keyshare-server.go index 87b567adf..fdaf11fc0 100644 --- a/irma/cmd/keyshare-server.go +++ b/irma/cmd/keyshare-server.go @@ -65,6 +65,8 @@ func init() { flags.Bool("redis-accept-inconsistency-risk", false, "accept the risk of inconsistent session state when using Redis Sentinel") flags.String("redis-username", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") + flags.String("redis-sentinel-username", "", "Redis Sentinel username (when using ACLs)") + flags.String("redis-sentinel-pw", "", "Redis Sentinel password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username for ACLs (username:key)") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") diff --git a/irma/cmd/server.go b/irma/cmd/server.go index d029b72c7..3a3e01f97 100644 --- a/irma/cmd/server.go +++ b/irma/cmd/server.go @@ -129,6 +129,8 @@ func setFlags(cmd *cobra.Command, production bool) error { flags.Bool("redis-accept-inconsistency-risk", false, "accept the risk of inconsistent session state when using Redis Sentinel") flags.String("redis-username", "", "Redis server username (when using ACLs)") flags.String("redis-pw", "", "Redis server password") + flags.String("redis-sentinel-username", "", "Redis Sentinel username (when using ACLs)") + flags.String("redis-sentinel-pw", "", "Redis Sentinel password") flags.Bool("redis-allow-empty-password", false, "explicitly allow an empty string as Redis password") flags.Bool("redis-acl-use-key-prefixes", false, "if enabled all Redis keys will be prefixed with the username for ACLs (username:key)") flags.Int("redis-db", 0, "database to be selected after connecting to the server (default 0)") diff --git a/server/conf.go b/server/conf.go index 7ec731e0f..ddf80e66b 100644 --- a/server/conf.go +++ b/server/conf.go @@ -122,6 +122,11 @@ type RedisSettings struct { // This can be used for key permissions in the Redis ACL system. If ACLUseKeyPrefixes is false, no prefix is used. ACLUseKeyPrefixes bool `json:"acl_use_key_prefixes,omitempty" mapstructure:"acl_use_key_prefixes"` + // SentinelUsername for Redis Sentinel authentication. If sentinel_username is empty, the default user is used. + SentinelUsername string `json:"sentinel_username,omitempty" mapstructure:"sentinel_username"` + // SentinelPassword for Redis Sentinel authentication. + SentinelPassword string `json:"sentinel_password,omitempty" mapstructure:"sentinel_password"` + DB int `json:"db,omitempty" mapstructure:"db"` TLSCertificate string `json:"tls_cert,omitempty" mapstructure:"tls_cert"` @@ -454,15 +459,19 @@ func (conf *Configuration) RedisClient() (*RedisClient, error) { var cl *redis.Client if len(conf.RedisSettings.SentinelAddrs) > 0 { cl = redis.NewFailoverClient(&redis.FailoverOptions{ - MasterName: conf.RedisSettings.SentinelMasterName, - SentinelAddrs: conf.RedisSettings.SentinelAddrs, - Password: conf.RedisSettings.Password, - DB: conf.RedisSettings.DB, - TLSConfig: tlsConfig, + MasterName: conf.RedisSettings.SentinelMasterName, + SentinelAddrs: conf.RedisSettings.SentinelAddrs, + Username: conf.RedisSettings.Username, + Password: conf.RedisSettings.Password, + SentinelUsername: conf.RedisSettings.SentinelUsername, + SentinelPassword: conf.RedisSettings.SentinelPassword, + DB: conf.RedisSettings.DB, + TLSConfig: tlsConfig, }) } else { cl = redis.NewClient(&redis.Options{ Addr: conf.RedisSettings.Addr, + Username: conf.RedisSettings.Username, Password: conf.RedisSettings.Password, DB: conf.RedisSettings.DB, TLSConfig: tlsConfig, From f10f9ca960c44342aec135781a85635dfdf378a9 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Wed, 22 Nov 2023 13:09:26 +0100 Subject: [PATCH 21/25] Feat: health endpoint for health checks --- CHANGELOG.md | 1 + server/keyshare/keyshareserver/server.go | 4 ++++ server/keyshare/myirmaserver/server.go | 4 ++++ server/requestorserver/server.go | 4 ++++ 4 files changed, 13 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82ce7793b..3a5dfcd93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Support for Redis in Sentinel mode - Redis support for `irma keyshare server` and `irma keyshare myirmaserver` +- `/health` endpoint for `irma server`, `irma keyshare server` and `irma keyshare myirmaserver` ### Changed - Using optimistic locking in the `irma server` instead of pessimistic locking diff --git a/server/keyshare/keyshareserver/server.go b/server/keyshare/keyshareserver/server.go index bc191732d..2c0d00b2c 100644 --- a/server/keyshare/keyshareserver/server.go +++ b/server/keyshare/keyshareserver/server.go @@ -118,6 +118,10 @@ func (s *Server) Handler() http.Handler { s.routeHandler(router) + router.Get("/health", func(w http.ResponseWriter, r *http.Request) { + server.WriteString(w, "OK") + }) + router.Route("/api/v1", func(r chi.Router) { s.routeHandler(r) }) diff --git a/server/keyshare/myirmaserver/server.go b/server/keyshare/myirmaserver/server.go index fe9424b1a..21f8107c9 100644 --- a/server/keyshare/myirmaserver/server.go +++ b/server/keyshare/myirmaserver/server.go @@ -107,6 +107,10 @@ func (s *Server) Handler() http.Handler { opts := server.LogOptions{Response: true, Headers: true, From: false, EncodeBinary: false} router.Use(server.LogMiddleware("keyshare-myirma", opts)) + router.Get("/health", func(w http.ResponseWriter, r *http.Request) { + server.WriteString(w, "OK") + }) + // Login/logout router.Post("/login/irma", s.handleIrmaLogin) router.Post("/login/email", s.handleEmailLogin) diff --git a/server/requestorserver/server.go b/server/requestorserver/server.go index 0e5f2f2b3..e795b2b67 100644 --- a/server/requestorserver/server.go +++ b/server/requestorserver/server.go @@ -198,6 +198,10 @@ func (s *Server) Handler() http.Handler { r.Use(cors.New(corsOptions).Handler) r.Use(server.LogMiddleware("requestor", log)) + router.Get("/health", func(w http.ResponseWriter, r *http.Request) { + server.WriteString(w, "OK") + }) + // Server routes r.Route("/session", func(r chi.Router) { r.Post("/", s.handleCreateSession) From 23ed15923aaaf18a3f42dd6e23164bb8686bf763 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Fri, 24 Nov 2023 11:41:37 +0100 Subject: [PATCH 22/25] Feat: read keyshare storage fallback keys from directory --- CHANGELOG.md | 1 + irma/cmd/keyshare-server.go | 16 +++++------ server/keyshare/keyshareserver/conf.go | 40 +++++++++++++++++++++----- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a5dfcd93..a78a9030d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Using optimistic locking in the `irma server` instead of pessimistic locking +- `storage-fallback-key-file` option of `irma keyshare server` being replaced by `storage-fallback-keys-dir` option ## [0.14.2] - 2023-10-25 ### Fixed diff --git a/irma/cmd/keyshare-server.go b/irma/cmd/keyshare-server.go index fdaf11fc0..b87f3faac 100644 --- a/irma/cmd/keyshare-server.go +++ b/irma/cmd/keyshare-server.go @@ -81,7 +81,7 @@ func init() { flags.String("jwt-issuer", keysharecore.JWTIssuerDefault, "JWT issuer used in \"iss\" field") flags.Int("jwt-pin-expiry", keysharecore.JWTPinExpiryDefault, "Expiry of PIN JWT in seconds") flags.String("storage-primary-key-file", "", "Primary key used for encrypting and decrypting secure containers") - flags.StringSlice("storage-fallback-key-file", nil, "Fallback key(s) used to decrypt older secure containers") + flags.String("storage-fallback-keys-dir", "", "Directory containing fallback key(s) used to decrypt older secure containers (only .key files are considered; the storage primary key file and hidden files are ignored)") headers["keyshare-attribute"] = "Keyshare server attribute issued during registration" flags.String("keyshare-attribute", "", "Attribute identifier that contains username") @@ -132,13 +132,13 @@ func configureKeyshareServer(cmd *cobra.Command) (*keyshareserver.Configuration, DBConnMaxIdleTime: viper.GetInt("db_max_idle_time"), DBConnMaxOpenTime: viper.GetInt("db_max_open_time"), - JwtKeyID: viper.GetUint32("jwt_privkey_id"), - JwtPrivateKey: viper.GetString("jwt_privkey"), - JwtPrivateKeyFile: viper.GetString("jwt_privkey_file"), - JwtIssuer: viper.GetString("jwt_issuer"), - JwtPinExpiry: viper.GetInt("jwt_pin_expiry"), - StoragePrimaryKeyFile: viper.GetString("storage_primary_key_file"), - StorageFallbackKeyFiles: viper.GetStringSlice("storage_fallback_key_file"), + JwtKeyID: viper.GetUint32("jwt_privkey_id"), + JwtPrivateKey: viper.GetString("jwt_privkey"), + JwtPrivateKeyFile: viper.GetString("jwt_privkey_file"), + JwtIssuer: viper.GetString("jwt_issuer"), + JwtPinExpiry: viper.GetInt("jwt_pin_expiry"), + StoragePrimaryKeyFile: viper.GetString("storage_primary_key_file"), + StorageFallbackKeysDir: viper.GetString("storage_fallback_keys_dir"), KeyshareAttribute: irma.NewAttributeTypeIdentifier(viper.GetString("keyshare_attribute")), diff --git a/server/keyshare/keyshareserver/conf.go b/server/keyshare/keyshareserver/conf.go index d464caa1d..8403bc158 100644 --- a/server/keyshare/keyshareserver/conf.go +++ b/server/keyshare/keyshareserver/conf.go @@ -4,6 +4,8 @@ import ( "encoding/binary" "html/template" "os" + "path" + "path/filepath" "strings" "time" @@ -49,8 +51,8 @@ type Configuration struct { JwtPrivateKey string `json:"jwt_privkey" mapstructure:"jwt_privkey"` JwtPrivateKeyFile string `json:"jwt_privkey_file" mapstructure:"jwt_privkey_file"` // Decryption keys used for user secrets - StorageFallbackKeyFiles []string `json:"storage_fallback_key_files" mapstructure:"storage_fallback_key_files"` - StoragePrimaryKeyFile string `json:"storage_primary_key_file" mapstructure:"storage_primary_key_file"` + StorageFallbackKeysDir string `json:"storage_fallback_keys_dir" mapstructure:"storage_fallback_keys_dir"` + StoragePrimaryKeyFile string `json:"storage_primary_key_file" mapstructure:"storage_primary_key_file"` // Keyshare attribute to issue during registration KeyshareAttribute irma.AttributeTypeIdentifier `json:"keyshare_attribute" mapstructure:"keyshare_attribute"` @@ -161,7 +163,11 @@ func setupCore(conf *Configuration) (*keysharecore.Core, error) { if err != nil { return nil, server.LogError(errors.WrapPrefix(err, "failed to read keyshare server jwt key", 0)) } - decKeyID, decKey, err := readAESKey(conf.StoragePrimaryKeyFile) + storagePrimaryKeyFilePath, err := filepath.Abs(conf.StoragePrimaryKeyFile) + if err != nil { + return nil, server.LogError(errors.WrapPrefix(err, "failed to get absolute path of primary storage key", 0)) + } + decKeyID, decKey, err := readAESKey(storagePrimaryKeyFilePath) if err != nil { return nil, server.LogError(errors.WrapPrefix(err, "failed to load primary storage key", 0)) } @@ -174,12 +180,32 @@ func setupCore(conf *Configuration) (*keysharecore.Core, error) { JWTIssuer: conf.JwtIssuer, JWTPinExpiry: conf.JwtPinExpiry, }) - for _, keyFile := range conf.StorageFallbackKeyFiles { - id, key, err := readAESKey(keyFile) + if conf.StorageFallbackKeysDir != "" { + dirEntries, err := os.ReadDir(conf.StorageFallbackKeysDir) if err != nil { - return nil, server.LogError(errors.WrapPrefix(err, "failed to load fallback key "+keyFile, 0)) + return nil, server.LogError(errors.WrapPrefix(err, "failed to read fallback keys directory", 0)) + } + for _, dirEntry := range dirEntries { + if dirEntry.IsDir() || !strings.HasSuffix(dirEntry.Name(), ".key") || strings.HasPrefix(dirEntry.Name(), ".") { + conf.Logger.Warnf("Ignoring storage fallback key file %s", dirEntry.Name()) + continue + } + + pth, err := filepath.Abs(path.Join(conf.StorageFallbackKeysDir, dirEntry.Name())) + if err != nil { + return nil, server.LogError(errors.WrapPrefix(err, "failed to get absolute path of fallback key "+dirEntry.Name(), 0)) + } + if pth == storagePrimaryKeyFilePath { + conf.Logger.Debugf("Skipping primary storage key %s as fallback key", dirEntry.Name()) + continue + } + + id, key, err := readAESKey(pth) + if err != nil { + return nil, server.LogError(errors.WrapPrefix(err, "failed to load fallback key "+dirEntry.Name(), 0)) + } + core.DangerousAddDecryptionKey(id, key) } - core.DangerousAddDecryptionKey(id, key) } return core, nil From 439d83a3c8d0f2eb8eb7639aa6c106e1ca95130f Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Mon, 27 Nov 2023 11:56:39 +0100 Subject: [PATCH 23/25] Fix: STARTTLS not enabled while testing SMTP connection --- irma/cmd/config.go | 1 + server/keyshare/email.go | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/irma/cmd/config.go b/irma/cmd/config.go index b2c365453..e9e4ce517 100644 --- a/irma/cmd/config.go +++ b/irma/cmd/config.go @@ -35,6 +35,7 @@ func configureEmail() keyshare.EmailConfiguration { return keyshare.EmailConfiguration{ EmailServer: viper.GetString("email_server"), + EmailHostname: viper.GetString("email_hostname"), EmailAuth: emailAuth, EmailFrom: viper.GetString("email_from"), DefaultLanguage: viper.GetString("default_language"), diff --git a/server/keyshare/email.go b/server/keyshare/email.go index b8a01471a..dfc7354f1 100644 --- a/server/keyshare/email.go +++ b/server/keyshare/email.go @@ -2,6 +2,7 @@ package keyshare import ( "bytes" + "crypto/tls" "fmt" "html/template" "net" @@ -15,6 +16,7 @@ import ( type EmailConfiguration struct { EmailServer string `json:"email_server" mapstructure:"email_server"` + EmailHostname string `json:"email_hostname" mapstructure:"email_hostname"` EmailFrom string `json:"email_from" mapstructure:"email_from"` DefaultLanguage string `json:"default_language" mapstructure:"default_language"` EmailAuth smtp.Auth @@ -130,7 +132,18 @@ func (conf EmailConfiguration) VerifyEmailServer() error { if err != nil { return errors.Errorf("failed to connect to email server: %v", err) } + if conf.EmailHostname != "" { + if ok, _ := client.Extension("STARTTLS"); !ok { + return errors.Errorf("email hostname is specified but email server does not support STARTTLS") + } + if err := client.StartTLS(&tls.Config{ServerName: conf.EmailHostname}); err != nil { + return errors.Errorf("failed to start TLS on connection to email server: %v", err) + } + } if conf.EmailAuth != nil { + if conf.EmailHostname == "" && !strings.HasPrefix(conf.EmailServer, "localhost:") { + return errors.Errorf("email authentication is enabled but email server is neither using TLS nor running on localhost") + } if err = client.Auth(conf.EmailAuth); err != nil { return errors.Errorf("failed to authenticate to email server: %v", err) } From 63ffadd95cb367135f4c1199a85227db18d025c5 Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Mon, 27 Nov 2023 12:02:33 +0100 Subject: [PATCH 24/25] Chore: log when keyshare components start listening on port --- irma/cmd/keyshare-root.go | 1 + 1 file changed, 1 insertion(+) diff --git a/irma/cmd/keyshare-root.go b/irma/cmd/keyshare-root.go index bd82a4f88..64811c004 100644 --- a/irma/cmd/keyshare-root.go +++ b/irma/cmd/keyshare-root.go @@ -31,6 +31,7 @@ type stoppableServer interface { func runServer(serv stoppableServer, logger *logrus.Logger) { // Determine full listening address. fullAddr := fmt.Sprintf("%s:%d", viper.GetString("listen_addr"), viper.GetInt("port")) + logger.Info("Server listening at ", fullAddr) // Load TLS configuration TLSConfig := configureTLS() From 4b170388317bd679245795210dd6affe859a3cbf Mon Sep 17 00:00:00 2001 From: Ivar Derksen Date: Mon, 27 Nov 2023 13:58:09 +0100 Subject: [PATCH 25/25] Fix: connect timeouts to email server not detected --- server/keyshare/email.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/keyshare/email.go b/server/keyshare/email.go index dfc7354f1..e07b0e9ac 100644 --- a/server/keyshare/email.go +++ b/server/keyshare/email.go @@ -9,6 +9,7 @@ import ( "net/mail" "net/smtp" "strings" + "time" "github.com/go-errors/errors" "github.com/privacybydesign/irmago/server" @@ -128,6 +129,13 @@ func (conf EmailConfiguration) VerifyEmailServer() error { return nil } + // smtp.Dial does not support timeouts, so we use net.DialTimeout instead. + conn, err := net.DialTimeout("tcp", conf.EmailServer, 10*time.Second) + if err != nil { + return errors.Errorf("failed to connect to email server: %v", err) + } + conn.Close() + client, err := smtp.Dial(conf.EmailServer) if err != nil { return errors.Errorf("failed to connect to email server: %v", err)