fix session signoff crash in pre-multi-conn AIM clients

foodgroup/auth.go

@@ -125,6 +125,9 @@ func (s AuthService) RegisterBOSSession(ctx context.Context, serverCookie state.
 	// set string containing OSCAR client name and version
 	sess.SetClientID(serverCookie.ClientID)
 
+	// indicate whether the client supports/wants multiple concurrent sessions
+	sess.SetMultiConnFlag(wire.MultiConnFlag(serverCookie.MultiConnFlag))
+
 	if u.DisplayScreenName.IsUIN() {
 		sess.SetUserInfoFlag(wire.OServiceUserFlagICQ)
 
@@ -364,6 +367,7 @@ type loginProperties struct {
 	isKerberosPlaintextAuth bool
 	isKerberosRoastedAuth   bool
 	isTOCAuth               bool
+	multiConnFlag           uint8
 	passwordHash            []byte
 	plaintextPassword       []byte
 	roastedPass             []byte
@@ -415,6 +419,11 @@ func (l *loginProperties) fromTLV(list wire.TLVList) error {
 		l.isFLAPAuth = true
 	}
 
+	// does the client support multiple concurrent sessions?
+	if multiConnFlags, found := list.Uint8(wire.LoginTLVTagsMultiConnFlags); found {
+		l.multiConnFlag = multiConnFlags
+	}
+
 	return nil
 }
 
@@ -514,9 +523,10 @@ func (s AuthService) createUser(ctx context.Context, props loginProperties, newU
 
 func (s AuthService) loginSuccessResponse(props loginProperties, advertisedHost string) (wire.TLVRestBlock, error) {
 	loginCookie := state.ServerCookie{
-		Service:    wire.BOS,
-		ScreenName: props.screenName,
-		ClientID:   props.clientID,
+		Service:       wire.BOS,
+		ScreenName:    props.screenName,
+		ClientID:      props.clientID,
+		MultiConnFlag: props.multiConnFlag,
 	}
 
 	buf := &bytes.Buffer{}

foodgroup/auth_test.go

@@ -52,6 +52,7 @@ func TestAuthService_BUCPLoginRequest(t *testing.T) {
 					TLVList: wire.TLVList{
 						wire.NewTLVBE(wire.LoginTLVTagsScreenName, user.DisplayScreenName),
 						wire.NewTLVBE(wire.LoginTLVTagsPasswordHash, user.StrongMD5Pass),
+						wire.NewTLVBE(wire.LoginTLVTagsMultiConnFlags, wire.MultiConnFlagsRecentClient),
 					},
 				},
 			},
@@ -69,7 +70,8 @@ func TestAuthService_BUCPLoginRequest(t *testing.T) {
 						{
 							dataIn: func() []byte {
 								loginCookie := state.ServerCookie{
-									ScreenName: user.DisplayScreenName,
+									ScreenName:    user.DisplayScreenName,
+									MultiConnFlag: uint8(wire.MultiConnFlagsRecentClient),
 								}
 								buf := &bytes.Buffer{}
 								assert.NoError(t, wire.MarshalBE(loginCookie, buf))

foodgroup/oservice_test.go

@@ -78,6 +78,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 								0x02, 'm', 'e',
 								0x0, // no client ID
 								0x0, // no chat cookie
+								0x0, // multi conn flag
 							},
 							cookieOut: []byte("the-cookie"),
 						},
@@ -124,6 +125,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 								0x02, 'm', 'e',
 								0x0, // no client ID
 								0x0, // no chat cookie
+								0x0, // multi conn flag
 							},
 							cookieOut: []byte("the-cookie"),
 						},
@@ -170,6 +172,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 								0x02, 'm', 'e',
 								0x0, // no client ID
 								0x0, // no chat cookie
+								0x0, // multi conn flag
 							},
 							cookieOut: []byte("the-cookie"),
 						},
@@ -216,6 +219,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 								0x02, 'm', 'e',
 								0x0, // no client ID
 								0x0, // no chat cookie
+								0x0, // multi conn flag
 							},
 							cookieOut: []byte("the-cookie"),
 						},
@@ -280,6 +284,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 									0x02, 'm', 'e', // screen name
 									0x00, // no client ID
 									0x11, '4', '-', '0', '-', 't', 'h', 'e', '-', 'c', 'h', 'a', 't', '-', 'r', 'o', 'o', 'm',
+									0x0, // multi conn flag
 								},
 								cookieOut: []byte("the-auth-cookie"),
 							},
@@ -327,6 +332,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 								0x02, 'm', 'e',
 								0x0, // no client ID
 								0x0, // no chat cookie
+								0x0, // multi conn flag
 							},
 							cookieOut: []byte("the-cookie"),
 						},
@@ -458,6 +464,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 								0x02, 'm', 'e',
 								0x0, // no client ID
 								0x0, // no chat cookie
+								0x0, // multi conn flag
 							},
 							cookieOut: []byte("the-cookie"),
 						},
@@ -509,6 +516,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 								0x02, 'm', 'e',
 								0x0, // no client ID
 								0x0, // no chat cookie
+								0x0, // multi conn flag
 							},
 							cookieOut: []byte("the-cookie"),
 						},
@@ -560,6 +568,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 								0x02, 'm', 'e',
 								0x0, // no client ID
 								0x0, // no chat cookie
+								0x0, // multi conn flag
 							},
 							cookieOut: []byte("the-cookie"),
 						},
@@ -611,6 +620,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 								0x02, 'm', 'e',
 								0x0, // no client ID
 								0x0, // no chat cookie
+								0x0, // multi conn flag
 							},
 							cookieOut: []byte("the-cookie"),
 						},
@@ -676,6 +686,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 									0x02, 'm', 'e', // screen name
 									0x00, // no client ID
 									0x11, '4', '-', '0', '-', 't', 'h', 'e', '-', 'c', 'h', 'a', 't', '-', 'r', 'o', 'o', 'm',
+									0x0, // multi conn flag
 								},
 								cookieOut: []byte("the-auth-cookie"),
 							},
@@ -728,6 +739,7 @@ func TestOServiceService_ServiceRequest(t *testing.T) {
 								0x02, 'm', 'e',
 								0x0, // no client ID
 								0x0, // no chat cookie
+								0x0, // multi conn flag
 							},
 							cookieOut: []byte("the-cookie"),
 						},

server/oscar/server.go

@@ -353,7 +353,7 @@ func (s oscarServer) authenticate(
 				},
 			)
 		} else {
-			return flapc.SendSignoffFrame(tlv)
+			return flapc.NewSignoff(tlv)
 		}
 	}
 
@@ -386,7 +386,7 @@ func (s oscarServer) processFLAPAuth(
 	if err != nil {
 		return err
 	}
-	return flapc.SendSignoffFrame(tlv)
+	return flapc.NewSignoff(tlv)
 }
 
 func (s oscarServer) processBUCPAuth(ctx context.Context, flapc *wire.FlapClient, advertisedHost string) error {
@@ -569,11 +569,17 @@ func (s oscarServer) dispatchIncomingMessages(
 			}
 		case <-sess.Closed():
 			block := wire.TLVRestBlock{}
-			// error code indicating user signed in a different location
-			block.Append(wire.NewTLVBE(0x0009, wire.OServiceDiscErrNewLogin))
-			// "more info" button
-			block.Append(wire.NewTLVBE(0x000b, "https://github.com/mk6i/retro-aim-server"))
-			if err := flapc.SendSignoffFrame(block); err != nil {
+			// add logoff reason to clients that support multi-conn
+			if sess.MultiConnFlag() != wire.MultiConnFlagsOldClient {
+				// error code indicating user signed in a different location
+				block.Append(wire.NewTLVBE(0x0009, wire.OServiceDiscErrNewLogin))
+				// "more info" button
+				block.Append(wire.NewTLVBE(0x000b, "https://github.com/mk6i/retro-aim-server"))
+				if err := flapc.NewSignoff(block); err != nil {
+					return fmt.Errorf("unable to gracefully disconnect user. %w", err)
+				}
+			}
+			if err := flapc.OldSignoff(); err != nil {
 				return fmt.Errorf("unable to gracefully disconnect user. %w", err)
 			}
 			return nil
@@ -581,11 +587,11 @@ func (s oscarServer) dispatchIncomingMessages(
 			block := wire.TLVRestBlock{}
 			// send explicit disconnect notification to client since proxies
 			// between client and server may not properly terminate connections
-			if err := flapc.SendSignoffFrame(block); err != nil {
+			if err := flapc.NewSignoff(block); err != nil {
 				return fmt.Errorf("unable to gracefully disconnect user. %w", err)
 			}
 			// application is shutting down
-			if err := flapc.Disconnect(); err != nil {
+			if err := flapc.OldSignoff(); err != nil {
 				return fmt.Errorf("unable to gracefully disconnect user. %w", err)
 			}
 			return nil

server/oscar/server_test.go

@@ -644,14 +644,17 @@ func Test_oscarServer_dispatchIncomingMessages_shutdownSignoff(t *testing.T) {
 	frame, err := flapc.ReceiveFLAP()
 	assert.NoError(t, err)
 	assert.Equal(t, wire.FLAPFrameSignoff, frame.FrameType)
+
+	wg.Done()
 }
 
-// Make sure the client receives disconnection signoff FLAP when the session
-// gets logged off by a new session.
-func Test_oscarServer_dispatchIncomingMessages_disconnect(t *testing.T) {
+// Make sure the client (which doesn't support multi-conn) receives
+// disconnection signoff FLAP when the session gets logged off by a new session.
+func Test_oscarServer_dispatchIncomingMessages_disconnect_old_client(t *testing.T) {
 	clientConn, serverConn := net.Pipe()
 	ctx := context.Background()
 	sess := state.NewSession()
+	sess.SetMultiConnFlag(wire.MultiConnFlagsRecentClient)
 
 	var wg sync.WaitGroup
 	wg.Add(1)
@@ -666,10 +669,42 @@ func Test_oscarServer_dispatchIncomingMessages_disconnect(t *testing.T) {
 	}()
 
 	sess.Close()
+
+	frame := wire.FLAPFrameDisconnect{}
+	assert.NoError(t, wire.UnmarshalBE(&frame, clientConn))
+	assert.Equal(t, wire.FLAPFrameSignoff, frame.FrameType)
+
+	wg.Done()
+}
+
+// Make sure the client (which supports multi-conn) receives disconnection
+// signoff FLAP when the session gets logged off by a new session.
+func Test_oscarServer_dispatchIncomingMessages_disconnect_new_client(t *testing.T) {
+	clientConn, serverConn := net.Pipe()
+	ctx := context.Background()
+	sess := state.NewSession()
+	sess.SetMultiConnFlag(wire.MultiConnFlagsRecentClient)
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		srv := oscarServer{
+			Logger: slog.Default(),
+		}
+		flapc := wire.NewFlapClient(0, serverConn, serverConn)
+		err := srv.dispatchIncomingMessages(ctx, wire.BOS, sess, flapc, serverConn, config.Listener{})
+		assert.NoError(t, err)
+	}()
+
+	sess.Close()
+
 	flapc := wire.NewFlapClient(0, clientConn, clientConn)
 	frame, err := flapc.ReceiveFLAP()
 	assert.NoError(t, err)
 	assert.Equal(t, wire.FLAPFrameSignoff, frame.FrameType)
+
+	wg.Done()
 }
 
 func Test_oscarServer_receiveSessMessages_BOS_integration(t *testing.T) {

state/cookie.go

@@ -19,10 +19,11 @@ const authCookieLen = 256
 // ServerCookie represents a token containing client metadata passed to the BOS
 // service upon connection.
 type ServerCookie struct {
-	Service    uint16
-	ScreenName DisplayScreenName `oscar:"len_prefix=uint8"`
-	ClientID   string            `oscar:"len_prefix=uint8"`
-	ChatCookie string            `oscar:"len_prefix=uint8"`
+	Service       uint16
+	ScreenName    DisplayScreenName `oscar:"len_prefix=uint8"`
+	ClientID      string            `oscar:"len_prefix=uint8"`
+	ChatCookie    string            `oscar:"len_prefix=uint8"`
+	MultiConnFlag uint8
 }
 
 func NewHMACCookieBaker() (HMACCookieBaker, error) {

state/session.go

@@ -73,6 +73,7 @@ type Session struct {
 	rateByClassID       [5]RateClassState
 	foodGroupVersions   [wire.MDir + 1]uint16
 	typingEventsEnabled bool
+	multiConnFlag       wire.MultiConnFlag
 }
 
 // NewSession returns a new instance of Session. By default, the user may have
@@ -572,3 +573,17 @@ func (s *Session) SetTypingEventsEnabled(enabled bool) {
 	defer s.mutex.Unlock()
 	s.typingEventsEnabled = enabled
 }
+
+// SetMultiConnFlag sets the multi-connection flag for this session.
+func (s *Session) SetMultiConnFlag(flag wire.MultiConnFlag) {
+	s.mutex.Lock()
+	defer s.mutex.Unlock()
+	s.multiConnFlag = flag
+}
+
+// MultiConnFlag retrieves the multi-connection flag for this session.
+func (s *Session) MultiConnFlag() wire.MultiConnFlag {
+	s.mutex.RLock()
+	defer s.mutex.RUnlock()
+	return s.multiConnFlag
+}

state/session_test.go

@@ -608,3 +608,17 @@ func TestSession_SetAndGetTypingEventsEnabled(t *testing.T) {
 	s.SetTypingEventsEnabled(false)
 	assert.False(t, s.TypingEventsEnabled())
 }
+
+func TestSession_SetAndGetMultiConnFlag(t *testing.T) {
+	s := NewSession()
+	assert.Zero(t, s.MultiConnFlag())
+
+	s.SetMultiConnFlag(wire.MultiConnFlagsOldClient)
+	assert.Equal(t, wire.MultiConnFlagsOldClient, s.MultiConnFlag())
+
+	s.SetMultiConnFlag(wire.MultiConnFlagsRecentClient)
+	assert.Equal(t, wire.MultiConnFlagsRecentClient, s.MultiConnFlag())
+
+	s.SetMultiConnFlag(wire.MultiConnFlagsSingleClient)
+	assert.Equal(t, wire.MultiConnFlagsSingleClient, s.MultiConnFlag())
+}

wire/frames.go

@@ -29,8 +29,9 @@ type FLAPFrame struct {
 
 // FLAPFrameDisconnect is the last FLAP frame sent to a client before
 // disconnection. It differs from FLAPFrame in that there is no payload length
-// prefix at the end, which causes Windows AIM clients to improperly handle
-// server disconnections, as when the regular FLAPFrame type is used.
+// prefix at the end, which causes pre-multi-conn Windows AIM clients to
+// improperly handle server disconnections, as when the regular FLAPFrame type
+// is used.
 type FLAPFrameDisconnect struct {
 	StartMarker uint8
 	FrameType   uint8
@@ -147,11 +148,21 @@ func (f *FlapClient) ReceiveFLAP() (FLAPFrame, error) {
 	return flap, err
 }
 
-// SendSignoffFrame sends a sign-off FLAP frame with attached TLVs as the last
-// request sent in the FLAP auth flow. This is unrelated to the Disconnect()
-// method, which sends a sign-off frame to terminate a BOS connection.
-// todo: combine this method with Disconnect()
-func (f *FlapClient) SendSignoffFrame(tlvs TLVRestBlock) error {
+// OldSignoff sends a signoff FLAP frame to clients that don't support multi-conn.
+func (f *FlapClient) OldSignoff() error {
+	f.mutex.Lock()
+	defer f.mutex.Unlock()
+
+	flap := FLAPFrameDisconnect{
+		StartMarker: 42,
+		FrameType:   FLAPFrameSignoff,
+		Sequence:    uint16(f.sequence),
+	}
+	return MarshalBE(flap, f.w)
+}
+
+// NewSignoff sends a signoff FLAP frame to multi-conn clients with metadata.
+func (f *FlapClient) NewSignoff(tlvs TLVRestBlock) error {
 	tlvBuf := &bytes.Buffer{}
 	if err := MarshalBE(tlvs, tlvBuf); err != nil {
 		return err
@@ -249,16 +260,3 @@ func (f *FlapClient) ReceiveSNAC(frame *SNACFrame, body any) error {
 	}
 	return UnmarshalBE(body, buf)
 }
-
-// Disconnect sends a signoff FLAP frame.
-func (f *FlapClient) Disconnect() error {
-	f.mutex.Lock()
-	defer f.mutex.Unlock()
-
-	flap := FLAPFrameDisconnect{
-		StartMarker: 42,
-		FrameType:   FLAPFrameSignoff,
-		Sequence:    uint16(f.sequence),
-	}
-	return MarshalBE(flap, f.w)
-}

wire/snacs.go

@@ -92,6 +92,7 @@ const (
 	LoginTLVTagsAuthorizationCookie     uint16 = 0x06
 	LoginTLVTagsErrorSubcode            uint16 = 0x08
 	LoginTLVTagsPasswordHash            uint16 = 0x25
+	LoginTLVTagsMultiConnFlags          uint16 = 0x4A
 	LoginTLVTagsRoastedKerberosPassword uint16 = 0x1335
 	LoginTLVTagsRoastedTOCPassword      uint16 = 0x1337
 	LoginTLVTagsPlaintextPassword       uint16 = 0x1338
@@ -113,6 +114,14 @@ const (
 
 )
 
+type MultiConnFlag uint8
+
+const (
+	MultiConnFlagsOldClient    MultiConnFlag = 0x0 // client doesn't support multi-conn
+	MultiConnFlagsRecentClient MultiConnFlag = 0x1 // client supports multi-conn
+	MultiConnFlagsSingleClient MultiConnFlag = 0x3 // client supports multi-conn but only wants 1 concurrent session
+)
+
 //
 // Capability IDs
 //