go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
45.04k stars 5.49k forks source link

Redis store security improvement #25983

Closed smolinari closed 3 weeks ago

smolinari commented 1 year ago

Feature Description

As a Gitea and k8s admin using Gitea in a Kubernetes environment via the Gitea helm chart, I'd like to tighten the security around the usage of our Redis instance. To do this, we are adding keyspaces for our in-house applications.

As I understand the Gitea code, the different usages of Redis for session storage and cache (and anything else it uses Redis for) is using a simple key schema for each service using a prefix like session for session entries and cache for cache entries.

In order to be able to tighten Gitea's Redis usage, I'd like to suggest Gitea add an additional piece of information to the keys stored in Redis. For instance, add simply gitea. Then Gitea's session keys would look like gitea:session:xxxx and cache keys would looke like gitea:cache:xxxx (or something similar). It's called Keyspaces in Redis jargon.

https://redis.io/docs/manual/keyspace/

I'm not sure about the Redis driver for Go, but in the NodeJS driver, it is possible to set this kind of additional prefix at the point of making the connection, so developers won't need to remember it is there. It's called transparent key prefixing.

https://github.com/redis/ioredis#transparent-key-prefixing

Here is the doc section on locking down users/clients of a Redis cluster to a particular keyspace, so the client won't be able to call any other keys from any other applications (and hugely important for passing certain security standards).

https://redis.io/docs/management/security/acl/#key-permissions

I hope this is understandable. Thanks for reading and thanks in advance, if this can be worked on. And, I'll apologize up front, if this is already possible or rather being done in Gitea, but I don't think it is, thus with the current setup, Gitea, worst case, could even have name collisions with other applications.

Scott

Screenshots

No response

KN4CK3R commented 1 year ago

The redis library does not support prefixes but I think Gitea does if you pass prefix=foo in the connection string.

smolinari commented 1 year ago

prefix=foo

Thanks for replying. Can you elaborate on what you mean with the bit about Gitea and passing the parameter to the connection string? Any reference to what you mean in code or documentation by chance?

Scott

KN4CK3R commented 1 year ago

In your app.ini you have a connection string like redis://127.0.0.1:6379/0?pool_size=100&idle_timeout=180s There you could add redis://127.0.0.1:6379/0?pool_size=100&idle_timeout=180s&prefix=foo.

smolinari commented 1 year ago

Ok. I understand now, but I can't find any reference about this in the Redis docs. It's also not in the official URI scheme. https://www.iana.org/assignments/uri-schemes/prov/redis

And if that were possible, why would ioredis (the NodeJS driver) have it built into the driver?

I think you might be mistaken.

Scott

KN4CK3R commented 1 year ago

https://github.com/go-gitea/gitea/blob/d7a8d09da013f14bf795fa8efc3a0eac7f259d02/modules/session/redis.go#L124-L129

smolinari commented 1 year ago

Aha, hmm.... Ok. It's Gitea that uses the parameter. :thinking:

I'll give it a try and see what happens. If it works, I'll put in a PR to add it to the docs.

Scott

vladyslav-halchenko-gl commented 3 weeks ago

@smolinari did it work?....

smolinari commented 3 weeks ago

@vladyslav-halchenko-gl - I moved away from Gitea altogether. Sorry for the non-reply.

Scott