mirror of
https://github.com/multipleof4/ntfy.git
synced 2026-10-08 21:05:21 +00:00
Fix ACL case sensitivity issues in sqlite
This commit is contained in:
@@ -1926,6 +1926,10 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release
|
||||
|
||||
### ntfy server v2.24.0 (UNRELEASED)
|
||||
|
||||
**Security issues:**
|
||||
|
||||
* Fix case-insensitive ACL topic matching on SQLite: an access control rule for `secret` no longer also matches a request for `SECRET`. SQLite's `LIKE` is case-insensitive for ASCII by default; PostgreSQL was unaffected (no ticket)
|
||||
*
|
||||
**Features:**
|
||||
|
||||
* Add opt-in in-memory ACL cache (`auth-access-cache`) that serves topic authorization without a database round-trip; off by default, intended for high-volume servers
|
||||
|
||||
@@ -312,7 +312,13 @@ func NewSQLiteManager(filename, startupQueries string, config *Config) (*Manager
|
||||
if !util.FileExists(parentDir) {
|
||||
return nil, fmt.Errorf("user database directory %s does not exist or is not accessible", parentDir)
|
||||
}
|
||||
d, err := sql.Open("sqlite3", filename)
|
||||
// Open with case-sensitive LIKE. ACL topic matching is done via LIKE (see
|
||||
// selectTopicPerms), and SQLite's LIKE is case-insensitive for ASCII by
|
||||
// default -- without this, an ACL rule for "secret" would also match a
|
||||
// request for "SECRET", which is a security iisue. PostgreSQL's LIKE is
|
||||
// already case-sensitive, so this only affects SQLite. The pragma is
|
||||
// applied to every pooled connection by the driver.
|
||||
d, err := sql.Open("sqlite3", fmt.Sprintf("%s?_case_sensitive_like=on", filename))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -2414,6 +2414,55 @@ func TestAccessCache_FullReloadDoesNotClobberConcurrentRevoke(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// TestAuthorizeTopicAccess_TopicMatchingIsCaseSensitive guards against ACL
|
||||
// topic matching being case-insensitive. SQLite's LIKE is case-insensitive for
|
||||
// ASCII by default, which would let a request for "SECRET" match an ACL rule
|
||||
// for "secret" -- a security hole. PostgreSQL's LIKE is already case-sensitive.
|
||||
// NewSQLiteManager opens the database with case_sensitive_like enabled to close
|
||||
// this gap. This exercises the direct-DB path (cache disabled), which is the
|
||||
// path that runs the LIKE query; the in-memory cache is independently
|
||||
// case-sensitive (Go map keys / case-sensitive regex).
|
||||
func TestAuthorizeTopicAccess_TopicMatchingIsCaseSensitive(t *testing.T) {
|
||||
forEachBackend(t, func(t *testing.T, newManager newManagerFunc) {
|
||||
a := newManager(&Config{
|
||||
DefaultAccess: PermissionDenyAll,
|
||||
BcryptCost: bcrypt.MinCost,
|
||||
AccessCacheEnabled: false, // exercise the direct-DB LIKE path
|
||||
})
|
||||
t.Cleanup(func() { a.Close() })
|
||||
|
||||
require.Nil(t, a.AddUser("ben", "mypass", RoleUser, false))
|
||||
require.Nil(t, a.AllowAccess("ben", "secret", PermissionReadWrite)) // exact rule
|
||||
require.Nil(t, a.AllowAccess("ben", "team*", PermissionReadWrite)) // wildcard rule, stored as "team%"
|
||||
|
||||
// The exact rule is honored verbatim.
|
||||
read, write, found, err := a.authorizeTopicAccess("ben", "secret")
|
||||
require.Nil(t, err)
|
||||
require.True(t, found)
|
||||
require.True(t, read)
|
||||
require.True(t, write)
|
||||
|
||||
// Case variants of the exact rule must NOT match.
|
||||
for _, topic := range []string{"SECRET", "Secret", "sEcReT"} {
|
||||
_, _, found, err := a.authorizeTopicAccess("ben", topic)
|
||||
require.Nil(t, err)
|
||||
require.False(t, found, "ACL rule for \"secret\" must not match %q (case-insensitive match is a security hole)", topic)
|
||||
}
|
||||
|
||||
// The wildcard rule is honored for the matching case.
|
||||
_, _, found, err = a.authorizeTopicAccess("ben", "team-rocket")
|
||||
require.Nil(t, err)
|
||||
require.True(t, found)
|
||||
|
||||
// Case variants of the wildcard prefix must NOT match.
|
||||
for _, topic := range []string{"TEAM-rocket", "Team-rocket", "TEAMING"} {
|
||||
_, _, found, err := a.authorizeTopicAccess("ben", topic)
|
||||
require.Nil(t, err)
|
||||
require.False(t, found, "wildcard rule for \"team*\" must not match %q", topic)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestStoreReservations(t *testing.T) {
|
||||
forEachStoreBackend(t, func(t *testing.T, manager *Manager) {
|
||||
require.Nil(t, manager.AddUser("phil", "mypass", RoleUser, false))
|
||||
|
||||
Reference in New Issue
Block a user