From 3df0c22948bf104b2e83a99629de1c71010e8b84 Mon Sep 17 00:00:00 2001 From: Storm Dragon Date: Sun, 24 May 2026 00:26:48 -0400 Subject: [PATCH] Update a couple permissions things, no longer allow anonymous users. --- diskwriter/diskwriter.go | 5 +- diskwriter/diskwriter_test.go | 39 ++++++++++ distro-packages/Arch-Linux/skald-git/PKGBUILD | 2 +- hall/hall.go | 8 +- hall/hall_test.go | 77 +++++++++++++++++++ skald.md | 7 +- 6 files changed, 134 insertions(+), 4 deletions(-) diff --git a/diskwriter/diskwriter.go b/diskwriter/diskwriter.go index 2b0c20e..96adb54 100644 --- a/diskwriter/diskwriter.go +++ b/diskwriter/diskwriter.go @@ -196,6 +196,9 @@ func (client *Client) PushConn(g *hall.Hall, id string, up conn.Up, tracks []con g.WallOps("Write to disk: " + err.Error()) return err } + if down == nil { + return nil + } client.down[up.Id()] = down return nil @@ -699,7 +702,7 @@ func newDiskConn(client *Client, up conn.Up, remoteTracks []conn.UpTrack) (*disk } if len(tracks) == 0 { - return nil, errors.New("no usable tracks found") + return nil, nil } down := &diskConn{ diff --git a/diskwriter/diskwriter_test.go b/diskwriter/diskwriter_test.go index 62ad411..215a4e1 100644 --- a/diskwriter/diskwriter_test.go +++ b/diskwriter/diskwriter_test.go @@ -8,6 +8,9 @@ import ( "strings" "testing" "time" + + "git.stormux.org/storm/skald/conn" + "git.stormux.org/storm/skald/hall" ) func TestSanitise(t *testing.T) { @@ -195,6 +198,32 @@ func TestMixFrameReturnsNilForSilence(t *testing.T) { } } +func TestNewDiskConnIgnoresConnectionWithoutRecordableTracks(t *testing.T) { + down, err := newDiskConn(&Client{}, &fakeUp{id: "up1"}, nil) + if err != nil { + t.Fatal(err) + } + if down != nil { + t.Fatalf("expected no disk connection for no tracks, got %#v", down) + } +} + +func TestPushConnDoesNotStoreConnectionWithoutRecordableTracks(t *testing.T) { + g := &hall.Hall{} + client := &Client{ + hall: g, + recorder: &recorder{}, + down: make(map[string]*diskConn), + } + err := client.PushConn(g, "up1", &fakeUp{id: "up1"}, nil, "") + if err != nil { + t.Fatal(err) + } + if len(client.down) != 0 { + t.Fatalf("stored %d down connections, expected 0", len(client.down)) + } +} + func TestRecorderCreatesPlayableOgg(t *testing.T) { if _, err := exec.LookPath("ffmpeg"); err != nil { t.Skip("ffmpeg not installed") @@ -275,3 +304,13 @@ func pcmFrame(sample int16) []byte { } return out } + +type fakeUp struct { + id string +} + +func (u *fakeUp) AddLocal(conn.Down) error { return nil } +func (u *fakeUp) DelLocal(conn.Down) bool { return true } +func (u *fakeUp) Id() string { return u.id } +func (u *fakeUp) Label() string { return "audio" } +func (u *fakeUp) User() (string, string) { return "user-id", "Username" } diff --git a/distro-packages/Arch-Linux/skald-git/PKGBUILD b/distro-packages/Arch-Linux/skald-git/PKGBUILD index 9af69ec..db03293 100644 --- a/distro-packages/Arch-Linux/skald-git/PKGBUILD +++ b/distro-packages/Arch-Linux/skald-git/PKGBUILD @@ -2,7 +2,7 @@ # shellcheck shell=bash disable=SC2034,SC2154 pkgname=skald-git -pkgver=0.0.0.r1531.g08b484f +pkgver=0.0.0.r1532.g3e06de7 pkgrel=1 pkgdesc='Audio-only hall-based conferencing server' arch=('x86_64' 'aarch64') diff --git a/hall/hall.go b/hall/hall.go index 8c7cf97..8d2fb71 100644 --- a/hall/hall.go +++ b/hall/hall.go @@ -560,6 +560,12 @@ func AddClient(hall string, c Client, creds ClientCredentials) (*Hall, error) { c.SetUsername(username) c.SetPermissions(perms) + for _, cc := range clients { + if !isSystemClient(cc) && cc.Username() == username { + return nil, UserError("username already in use") + } + } + if !member("op", perms) { if g.locked != nil { m := *g.locked @@ -1005,7 +1011,7 @@ func (g *Hall) userExists(username string) bool { // usernames might lead to security vulnaribilities. // For now, we just do the minimal validation that avoids path traversal. func validUsername(username string) bool { - return username == "" || validHallName(username) + return username != "" && validHallName(username) } // called locked diff --git a/hall/hall_test.go b/hall/hall_test.go index 223d2ec..596ab42 100644 --- a/hall/hall_test.go +++ b/hall/hall_test.go @@ -238,6 +238,83 @@ func strPtr(s string) *string { return &s } +func TestEmptyUsernameIsRejected(t *testing.T) { + halls.halls = nil + oldDirectory := Directory + Directory = t.TempDir() + t.Cleanup(func() { + Directory = oldDirectory + halls.halls = nil + }) + + err := os.WriteFile( + filepath.Join(Directory, "empty-username-test.json"), + []byte(`{ + "wildcard-user": {"password": "secret", "permissions": "present"} + }`), + 0600, + ) + if err != nil { + t.Fatal(err) + } + + _, err = AddClient("empty-username-test", &testClient{id: "user1"}, ClientCredentials{ + Username: strPtr(""), + Password: "secret", + }) + if err == nil { + t.Fatal("empty username unexpectedly joined") + } +} + +func TestDuplicateUsernameIsRejected(t *testing.T) { + halls.halls = nil + oldDirectory := Directory + Directory = t.TempDir() + t.Cleanup(func() { + Directory = oldDirectory + halls.halls = nil + }) + + err := os.WriteFile( + filepath.Join(Directory, "duplicate-username-test.json"), + []byte(`{ + "wildcard-user": {"password": "secret", "permissions": "present"} + }`), + 0600, + ) + if err != nil { + t.Fatal(err) + } + + _, err = AddClient("duplicate-username-test", &testClient{id: "user1"}, ClientCredentials{ + Username: strPtr("bob1"), + Password: "secret", + }) + if err != nil { + t.Fatal(err) + } + + _, err = AddClient("duplicate-username-test", &testClient{id: "user2"}, ClientCredentials{ + Username: strPtr("bob1"), + Password: "secret", + }) + if err == nil { + t.Fatal("duplicate username unexpectedly joined") + } + if got, want := err.Error(), "username already in use"; got != want { + t.Fatalf("duplicate username error = %q, want %q", got, want) + } + + _, err = AddClient("duplicate-username-test", &testClient{id: "user3"}, ClientCredentials{ + Username: strPtr("bob2"), + Password: "secret", + }) + if err != nil { + t.Fatalf("different username should join: %v", err) + } +} + func TestLockedHallJoinMessage(t *testing.T) { halls.halls = nil oldDirectory := Directory diff --git a/skald.md b/skald.md index 4bc306d..466c4e6 100644 --- a/skald.md +++ b/skald.md @@ -376,7 +376,7 @@ are optional. The following fields are allowed: - `unrestricted-tokens`: if true, then ordinary users (without the "op" privilege) are allowed to create tokens; - - `allow-anonymous`: if true, then users may connect with an empty username; + - `allow-anonymous`: obsolete and ignored. Empty usernames are rejected; - `auto-subhalls`: if true, then subhalls of the form `hall/subhall` are automatically created when first accessed; @@ -410,6 +410,11 @@ following strings: - `caption`: a user with the right to display captions (only); - `admin`: a user with the right to administer the hall (only). +Usernames must be non-empty and unique among connected non-system clients in a +hall. If a `wildcard-user` is configured, users may choose arbitrary names +that are not listed in `users`, but two connected users may not use the same +name. + The value of the `codecs` field is an array of codecs allowed in the hall. Supported audio codecs include `"opus"`, `"g722"`, `"pcmu"` and `"pcma"`. Opus is the default and is the codec used by Skald's