Skip to content

Commit eb6db92

Browse files
authored
Implement wings security fixes (#156)
* Implement pterodactyl security fixes * Implement pterodactyl 292 changes (#158) * Implement pterodactyl 292 changes Add the same change as pterodactyl/wings#292 This adds configuration for the `machone-id` file that is required by hytale Creates and manages machine-id files on a per-server basis Adds code to remove machine-id files when a server is deleted as well. It also adds a group file for use along with the passwd file Updated config for passwd Updated mounts to not set default except for the the correct default. * Update machine-id generation Moved machine-id generation code outside of server create only called during initial server creation Create machine-id file for servers that already exists if the file is missing. Makes sure tempdir is created on wings start * remove append removes the append where not needed * Implement pterodactyl security fixes
1 parent 1e7c8ce commit eb6db92

File tree

15 files changed

+392
-51
lines changed

15 files changed

+392
-51
lines changed

router/router.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ func Configure(m *wserver.Manager, client remote.Client) *gin.Engine {
6666
protected.GET("/api/servers", getAllServers)
6767
protected.POST("/api/servers", postCreateServer)
6868
protected.DELETE("/api/transfers/:server", deleteTransfer)
69+
protected.POST("/api/deauthorize-user", postDeauthorizeUser)
6970

7071
// These are server specific routes, and require that the request be authorized, and
7172
// that the server exist on the Daemon.

router/router_server.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,8 @@ func deleteServer(c *gin.Context) {
303303
// Adds any of the JTIs passed through in the body to the deny list for the websocket
304304
// preventing any JWT generated before the current time from being used to connect to
305305
// the socket or send along commands.
306+
//
307+
// deprecated: prefer /api/deauthorize-user
306308
func postServerDenyWSTokens(c *gin.Context) {
307309
var data struct {
308310
JTIs []string `json:"jtis"`

router/router_server_ws.go

Lines changed: 88 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,17 @@ package router
22

33
import (
44
"context"
5+
"encoding/json"
6+
"net/http"
57
"time"
68

9+
"emperror.dev/errors"
710
"github.com/gin-gonic/gin"
8-
"github.com/goccy/go-json"
911
ws "github.com/gorilla/websocket"
10-
1112
"github.com/pelican-dev/wings/router/middleware"
1213
"github.com/pelican-dev/wings/router/websocket"
14+
"github.com/pelican-dev/wings/server"
15+
"golang.org/x/time/rate"
1316
)
1417

1518
var expectedCloseCodes = []int{
@@ -25,6 +28,27 @@ func getServerWebsocket(c *gin.Context) {
2528
manager := middleware.ExtractManager(c)
2629
s, _ := manager.Get(c.Param("server"))
2730

31+
// Limit the total number of websockets that can be opened at any one time for
32+
// a server instance. This applies across all users connected to the server, and
33+
// is not applied on a per-user basis.
34+
//
35+
// todo: it would be great to make this per-user instead, but we need to modify
36+
// how we even request this endpoint in order for that to be possible. Some type
37+
// of signed identifier in the URL that is verified on this end and set by the
38+
// panel using a shared secret is likely the easiest option. The benefit of that
39+
// is that we can both scope things to the user before authentication, and also
40+
// verify that the JWT provided by the panel is assigned to the same user.
41+
if s.Websockets().Len() >= 30 {
42+
c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{
43+
"error": "Too many open websocket connections.",
44+
})
45+
46+
return
47+
}
48+
49+
c.Header("Content-Security-Policy", "default-src 'self'")
50+
c.Header("X-Frame-Options", "DENY")
51+
2852
// Create a context that can be canceled when the user disconnects from this
2953
// socket that will also cancel listeners running in separate threads. If the
3054
// connection itself is terminated listeners using this context will also be
@@ -37,53 +61,101 @@ func getServerWebsocket(c *gin.Context) {
3761
middleware.CaptureAndAbort(c, err)
3862
return
3963
}
40-
defer handler.Connection.Close()
4164

4265
// Track this open connection on the server so that we can close them all programmatically
4366
// if the server is deleted.
4467
s.Websockets().Push(handler.Uuid(), &cancel)
4568
handler.Logger().Debug("opening connection to server websocket")
69+
defer s.Websockets().Remove(handler.Uuid())
4670

47-
defer func() {
48-
s.Websockets().Remove(handler.Uuid())
49-
handler.Logger().Debug("closing connection to server websocket")
71+
go func() {
72+
select {
73+
// When the main context is canceled (through disconnect, server deletion, or server
74+
// suspension) close the connection itself.
75+
case <-ctx.Done():
76+
handler.Logger().Debug("closing connection to server websocket")
77+
if err := handler.Connection.Close(); err != nil {
78+
handler.Logger().WithError(err).Error("failed to close websocket connection")
79+
}
80+
break
81+
}
5082
}()
5183

52-
// If the server is deleted we need to send a close message to the connected client
53-
// so that they disconnect since there will be no more events sent along. Listen for
54-
// the request context being closed to break this loop, otherwise this routine will
55-
// be left hanging in the background.
5684
go func() {
5785
select {
5886
case <-ctx.Done():
59-
break
87+
return
88+
// If the server is deleted we need to send a close message to the connected client
89+
// so that they disconnect since there will be no more events sent along. Listen for
90+
// the request context being closed to break this loop, otherwise this routine will
91+
//be left hanging in the background.
6092
case <-s.Context().Done():
61-
_ = handler.Connection.WriteControl(ws.CloseMessage, ws.FormatCloseMessage(ws.CloseGoingAway, "server deleted"), time.Now().Add(time.Second*5))
93+
cancel()
6294
break
6395
}
6496
}()
6597

66-
for {
67-
j := websocket.Message{}
98+
// Due to how websockets are handled we need to connect to the socket
99+
// and _then_ abort it if the server is suspended. You cannot capture
100+
// the HTTP response in the websocket client, thus we connect and then
101+
// immediately close with failure.
102+
if s.IsSuspended() {
103+
_ = handler.Connection.WriteMessage(ws.CloseMessage, ws.FormatCloseMessage(4409, "server is suspended"))
68104

69-
_, p, err := handler.Connection.ReadMessage()
105+
return
106+
}
107+
108+
// There is a separate rate limiter that applies to individual message types
109+
// within the actual websocket logic handler. _This_ rate limiter just exists
110+
// to avoid enormous floods of data through the socket since we need to parse
111+
// JSON each time. This rate limit realistically should never be hit since this
112+
// would require sending 50+ messages a second over the websocket (no more than
113+
// 10 per 200ms).
114+
var throttled bool
115+
rl := rate.NewLimiter(rate.Every(time.Millisecond*200), 10)
116+
117+
for {
118+
t, p, err := handler.Connection.ReadMessage()
70119
if err != nil {
71120
if ws.IsUnexpectedCloseError(err, expectedCloseCodes...) {
72121
handler.Logger().WithField("error", err).Warn("error handling websocket message for server")
73122
}
74123
break
75124
}
76125

126+
if !rl.Allow() {
127+
if !throttled {
128+
throttled = true
129+
_ = handler.Connection.WriteJSON(websocket.Message{Event: websocket.ThrottledEvent, Args: []string{"global"}})
130+
}
131+
continue
132+
}
133+
134+
throttled = false
135+
136+
// If the message isn't a format we expect, or the length of the message is far larger
137+
// than we'd ever expect, drop it. The websocket upgrader logic does enforce a maximum
138+
// _compressed_ message size of 4Kb but that could decompress to a much larger amount
139+
// of data.
140+
if t != ws.TextMessage || len(p) > 32_768 {
141+
continue
142+
}
143+
77144
// Discard and JSON parse errors into the void and don't continue processing this
78145
// specific socket request. If we did a break here the client would get disconnected
79146
// from the socket, which is NOT what we want to do.
147+
var j websocket.Message
80148
if err := json.Unmarshal(p, &j); err != nil {
81149
continue
82150
}
83151

84152
go func(msg websocket.Message) {
85153
if err := handler.HandleInbound(ctx, msg); err != nil {
86-
_ = handler.SendErrorJson(msg, err)
154+
if errors.Is(err, server.ErrSuspended) {
155+
cancel()
156+
} else {
157+
_ = handler.SendErrorJson(msg, err)
158+
}
87159
}
88160
}(j)
89161
}

router/router_system.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/pelican-dev/wings/config"
1515
"github.com/pelican-dev/wings/internal/diagnostics"
1616
"github.com/pelican-dev/wings/router/middleware"
17+
"github.com/pelican-dev/wings/router/tokens"
1718
"github.com/pelican-dev/wings/server"
1819
"github.com/pelican-dev/wings/server/installer"
1920
"github.com/pelican-dev/wings/system"
@@ -256,3 +257,33 @@ func postUpdateConfiguration(c *gin.Context) {
256257
Applied: true,
257258
})
258259
}
260+
261+
func postDeauthorizeUser(c *gin.Context) {
262+
var data struct {
263+
User string `json:"user"`
264+
Servers []string `json:"servers"`
265+
}
266+
267+
if err := c.BindJSON(&data); err != nil {
268+
return
269+
}
270+
271+
// todo: disconnect websockets more gracefully
272+
m := middleware.ExtractManager(c)
273+
if len(data.Servers) > 0 {
274+
for _, uuid := range data.Servers {
275+
if s, ok := m.Get(uuid); ok {
276+
s.Websockets().CancelAll()
277+
s.Sftp().Cancel(data.User)
278+
tokens.DenyForServer(s.ID(), data.User)
279+
}
280+
}
281+
} else {
282+
for _, s := range m.All() {
283+
s.Websockets().CancelAll()
284+
s.Sftp().Cancel(data.User)
285+
}
286+
}
287+
288+
c.Status(http.StatusNoContent)
289+
}

router/tokens/websocket.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,29 @@ var wingsBootTime = time.Now()
2424
// This is used to allow the Panel to revoke tokens en-masse for a given user & server
2525
// combination since the JTI for tokens is just MD5(user.id + server.uuid). When a server
2626
// is booted this listing is fetched from the panel and the Websocket is dynamically updated.
27+
//
28+
// deprecated: prefer use of userDenylist
2729
var denylist sync.Map
30+
var userDenylist sync.Map
2831

2932
// Adds a JTI to the denylist by marking any JWTs generated before the current time as
3033
// being invalid if they use the same JTI.
34+
//
35+
// deprecated: prefer the use of DenyForServer
3136
func DenyJTI(jti string) {
3237
log.WithField("jti", jti).Debugf("adding \"%s\" to JTI denylist", jti)
3338

3439
denylist.Store(jti, time.Now())
3540
}
3641

42+
// DenyForServer adds a user UUID to the denylist marking any existing JWTs issued
43+
// to the user as being invalid. This is associated with the user.
44+
func DenyForServer(s string, u string) {
45+
log.WithField("user_uuid", u).WithField("server_uuid", s).Debugf("denying all JWTs created at or before current time for user \"%s\"", u)
46+
47+
userDenylist.Store(strings.Join([]string{s, u}, ":"), time.Now())
48+
}
49+
3750
// WebsocketPayload defines the JWT payload for a websocket connection. This JWT is passed along to
3851
// the websocket after it has been connected to by sending an "auth" event.
3952
type WebsocketPayload struct {
@@ -79,12 +92,21 @@ func (p *WebsocketPayload) Denylisted() bool {
7992

8093
// Finally, if the token was issued before a time that is currently denied for this
8194
// token instance, ignore the permissions response.
95+
//
96+
// This list is deprecated, but we maintain the check here so that custom instances
97+
// are able to continue working. We'll remove it in a future release.
8298
if t, ok := denylist.Load(p.JWTID); ok {
8399
if p.IssuedAt.Time.Before(t.(time.Time)) {
84100
return true
85101
}
86102
}
87103

104+
if t, ok := userDenylist.Load(strings.Join([]string{p.ServerUUID, p.UserUUID}, ":")); ok {
105+
if p.IssuedAt.Time.Before(t.(time.Time)) {
106+
return true
107+
}
108+
}
109+
88110
return false
89111
}
90112

router/websocket/limiter.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
package websocket
2+
3+
import (
4+
"sync"
5+
"time"
6+
7+
"golang.org/x/time/rate"
8+
)
9+
10+
type LimiterBucket struct {
11+
mu sync.RWMutex
12+
limits map[Event]*rate.Limiter
13+
throttles map[Event]bool
14+
}
15+
16+
func (h *Handler) IsThrottled(e Event) bool {
17+
l := h.limiter.For(e)
18+
19+
h.limiter.mu.Lock()
20+
defer h.limiter.mu.Unlock()
21+
22+
if l.Allow() {
23+
h.limiter.throttles[e] = false
24+
25+
return false
26+
}
27+
28+
// If not allowed, track the throttling and send an event over the wire
29+
// if one wasn't already sent in the same throttling period.
30+
if v, ok := h.limiter.throttles[e]; !v || !ok {
31+
h.limiter.throttles[e] = true
32+
h.Logger().WithField("event", e).Debug("throttling websocket due to event volume")
33+
34+
_ = h.unsafeSendJson(&Message{Event: ThrottledEvent, Args: []string{string(e)}})
35+
}
36+
37+
return true
38+
}
39+
40+
func NewLimiter() *LimiterBucket {
41+
return &LimiterBucket{
42+
limits: make(map[Event]*rate.Limiter, 4),
43+
throttles: make(map[Event]bool, 4),
44+
}
45+
}
46+
47+
// For returns the internal rate limiter for the given event type. In most
48+
// cases this is a shared rate limiter for events, but certain "heavy" or low-frequency
49+
// events implement their own limiters.
50+
func (l *LimiterBucket) For(e Event) *rate.Limiter {
51+
name := limiterName(e)
52+
53+
l.mu.RLock()
54+
if v, ok := l.limits[name]; ok {
55+
l.mu.RUnlock()
56+
return v
57+
}
58+
59+
l.mu.RUnlock()
60+
l.mu.Lock()
61+
defer l.mu.Unlock()
62+
63+
limit, burst := limitValuesFor(e)
64+
l.limits[name] = rate.NewLimiter(limit, burst)
65+
66+
return l.limits[name]
67+
}
68+
69+
// limitValuesFor returns the underlying limit and burst value for the given event.
70+
func limitValuesFor(e Event) (rate.Limit, int) {
71+
// Twice every five seconds.
72+
if e == AuthenticationEvent || e == SendServerLogsEvent {
73+
return rate.Every(time.Second * 5), 2
74+
}
75+
76+
// 10 per second.
77+
if e == SendCommandEvent {
78+
return rate.Every(time.Second), 10
79+
}
80+
81+
// 4 per second.
82+
return rate.Every(time.Second), 4
83+
}
84+
85+
func limiterName(e Event) Event {
86+
if e == AuthenticationEvent || e == SendServerLogsEvent || e == SendCommandEvent {
87+
return e
88+
}
89+
90+
return "_default"
91+
}

router/websocket/listeners.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func (h *Handler) listenForServerEvents(ctx context.Context) error {
129129
continue
130130
}
131131
var sendErr error
132-
message := Message{Event: e.Topic}
132+
message := Message{Event: Event(e.Topic)}
133133
if str, ok := e.Data.(string); ok {
134134
message.Args = []string{str}
135135
} else if b, ok := e.Data.([]byte); ok {
@@ -147,7 +147,7 @@ func (h *Handler) listenForServerEvents(ctx context.Context) error {
147147
continue
148148
}
149149
}
150-
onError(message.Event, sendErr)
150+
onError(string(message.Event), sendErr)
151151
}
152152
break
153153
}

0 commit comments

Comments
 (0)