Improve HomeKit TLV format parser

Security Patch: Sanitize credentials on websocket error messages

internal/api/ws/ws.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/AlexxIT/go2rtc/internal/api"
 	"github.com/AlexxIT/go2rtc/internal/app"
+	"github.com/AlexxIT/go2rtc/pkg/core"
 	"github.com/gorilla/websocket"
 	"github.com/rs/zerolog"
 )
@@ -132,7 +133,8 @@ func apiWS(w http.ResponseWriter, r *http.Request) {
 		if handler := wsHandlers[msg.Type]; handler != nil {
 			go func() {
 				if err = handler(tr, msg); err != nil {
-					tr.Write(&Message{Type: "error", Value: msg.Type + ": " + err.Error()})
+					errMsg := core.StripUserinfo(err.Error())
+					tr.Write(&Message{Type: "error", Value: msg.Type + ": " + errMsg})
 				}
 			}()
 		}

internal/app/config.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 
 	"github.com/AlexxIT/go2rtc/pkg/shell"
 	"github.com/AlexxIT/go2rtc/pkg/yaml"
@@ -18,11 +19,16 @@ func LoadConfig(v any) {
 	}
 }
 
+var configMu sync.Mutex
+
 func PatchConfig(path []string, value any) error {
 	if ConfigPath == "" {
 		return errors.New("config file disabled")
 	}
 
+	configMu.Lock()
+	defer configMu.Unlock()
+
 	// empty config is OK
 	b, _ := os.ReadFile(ConfigPath)
 

pkg/core/core_test.go

@@ -118,3 +118,17 @@ func TestName(t *testing.T) {
 	// stage3
 	_ = prod2.Stop()
 }
+
+func TestStripUserinfo(t *testing.T) {
+	s := `streams:
+  test:
+    - ffmpeg:rtsp://username:password@10.1.2.3:554/stream1
+    - ffmpeg:rtsp://10.1.2.3:554/stream1@#video=copy
+`
+	s = StripUserinfo(s)
+	require.Equal(t, `streams:
+  test:
+    - ffmpeg:rtsp://***@10.1.2.3:554/stream1
+    - ffmpeg:rtsp://10.1.2.3:554/stream1@#video=copy
+`, s)
+}

pkg/core/helpers.go

@@ -2,6 +2,7 @@ package core
 
 import (
 	"crypto/rand"
+	"regexp"
 	"runtime"
 	"strconv"
 	"strings"
@@ -77,3 +78,14 @@ func Caller() string {
 	_, file, line, _ := runtime.Caller(1)
 	return file + ":" + strconv.Itoa(line)
 }
+
+const (
+	unreserved = `A-Za-z0-9-._~`
+	subdelims  = `!$&'()*+,;=`
+	userinfo   = unreserved + subdelims + `%:`
+)
+
+func StripUserinfo(s string) string {
+	sanitizer := regexp.MustCompile(`://[` + userinfo + `]+@`)
+	return sanitizer.ReplaceAllString(s, `://***@`)
+}

pkg/hap/helpers.go

@@ -71,11 +71,17 @@ type JSONCharacter struct {
 	Event  any    `json:"ev,omitempty"`
 }
 
+// 4.2.1.2 Invalid Setup Codes
+const insecurePINs = "00000000 11111111 22222222 33333333 44444444 55555555 66666666 77777777 88888888 99999999 12345678 87654321"
+
 func SanitizePin(pin string) (string, error) {
 	s := strings.ReplaceAll(pin, "-", "")
 	if len(s) != 8 {
 		return "", errors.New("hap: wrong PIN format: " + pin)
 	}
+	if strings.Contains(insecurePINs, s) {
+		return "", errors.New("hap: insecure PIN: " + pin)
+	}
 	// 123-45-678
 	return s[:3] + "-" + s[3:5] + "-" + s[5:], nil
 }

pkg/hap/tlv8/tlv8.go

@@ -46,6 +46,8 @@ func Marshal(v any) ([]byte, error) {
 	}
 
 	switch kind {
+	case reflect.Slice:
+		return appendSlice(nil, value)
 	case reflect.Struct:
 		return appendStruct(nil, value)
 	}
@@ -53,6 +55,23 @@ func Marshal(v any) ([]byte, error) {
 	return nil, errors.New("tlv8: not implemented: " + kind.String())
 }
 
+// separator the most confusing meaning in the documentation.
+// It can have a value of 0x00 or 0xFF or even 0x05.
+const separator = 0xFF
+
+func appendSlice(b []byte, value reflect.Value) ([]byte, error) {
+	for i := 0; i < value.Len(); i++ {
+		if i > 0 {
+			b = append(b, separator, 0)
+		}
+		var err error
+		if b, err = appendStruct(b, value.Index(i)); err != nil {
+			return nil, err
+		}
+	}
+	return b, nil
+}
+
 func appendStruct(b []byte, value reflect.Value) ([]byte, error) {
 	valueType := value.Type()
 
@@ -121,7 +140,7 @@ func appendValue(b []byte, tag byte, value reflect.Value) ([]byte, error) {
 	case reflect.Slice:
 		for i := 0; i < value.Len(); i++ {
 			if i > 0 {
-				b = append(b, 0, 0)
+				b = append(b, separator, 0)
 			}
 			if b, err = appendValue(b, tag, value.Index(i)); err != nil {
 				return nil, err
@@ -179,64 +198,86 @@ func Unmarshal(data []byte, v any) error {
 		kind = value.Kind()
 	}
 
-	if kind != reflect.Struct {
+	switch kind {
-		return errors.New("tlv8: not implemented: " + kind.String())
+	case reflect.Slice:
+		return unmarshalSlice(data, value)
+	case reflect.Struct:
+		return unmarshalStruct(data, value)
 	}
 
-	return unmarshalStruct(data, value)
+	return errors.New("tlv8: not implemented: " + kind.String())
 }
 
-func unmarshalStruct(b []byte, value reflect.Value) error {
+// unmarshalTLV can return two types of errors:
-	var waitSlice bool
+// - critical and then the value of []byte will be nil
+// - not critical and then []byte will contain the value
+func unmarshalTLV(b []byte, value reflect.Value) ([]byte, error) {
+	if len(b) < 2 {
+		return nil, errors.New("tlv8: wrong size: " + value.Type().Name())
+	}
 
-	for len(b) >= 2 {
+	t := b[0]
-		t := b[0]
+	l := int(b[1])
-		l := int(b[1])
 
-		// array item divider
+	// array item divider (t == 0x00 || t == 0xFF)
-		if t == 0 && l == 0 {
+	if l == 0 {
-			b = b[2:]
+		return b[2:], errors.New("tlv8: zero item")
-			waitSlice = true
+	}
-			continue
+
+	var v []byte
+
+	for {
+		if len(b) < 2+l {
+			return nil, errors.New("tlv8: wrong size: " + value.Type().Name())
 		}
 
-		var v []byte
+		v = append(v, b[2:2+l]...)
+		b = b[2+l:]
 
-		for {
+		// if size == 255 and same tag - continue read big payload
-			if len(b) < 2+l {
+		if l < 255 || len(b) < 2 || b[0] != t {
-				return errors.New("tlv8: wrong size: " + value.Type().Name())
+			break
+		}
+
+		l = int(b[1])
+	}
+
+	tag := strconv.Itoa(int(t))
+
+	valueField, ok := getStructField(value, tag)
+	if !ok {
+		return b, fmt.Errorf("tlv8: can't find T=%d,L=%d,V=%x for: %s", t, l, v, value.Type().Name())
+	}
+
+	if err := unmarshalValue(v, valueField); err != nil {
+		return nil, err
+	}
+
+	return b, nil
+}
+
+func unmarshalSlice(b []byte, value reflect.Value) error {
+	valueIndex := value.Index(growSlice(value))
+	for len(b) > 0 {
+		var err error
+		if b, err = unmarshalTLV(b, valueIndex); err != nil {
+			if b != nil {
+				valueIndex = value.Index(growSlice(value))
+				continue
 			}
-
-			v = append(v, b[2:2+l]...)
-			b = b[2+l:]
-
-			// if size == 255 and same tag - continue read big payload
-			if l < 255 || len(b) < 2 || b[0] != t {
-				break
-			}
-
-			l = int(b[1])
-		}
-
-		tag := strconv.Itoa(int(t))
-
-		valueField, ok := getStructField(value, tag)
-		if !ok {
-			return fmt.Errorf("tlv8: can't find T=%d,L=%d,V=%x for: %s", t, l, v, value.Type().Name())
-		}
-
-		if waitSlice {
-			if valueField.Kind() != reflect.Slice {
-				return fmt.Errorf("tlv8: should be slice T=%d,L=%d,V=%x for: %s", t, l, v, value.Type().Name())
-			}
-			waitSlice = false
-		}
-
-		if err := unmarshalValue(v, valueField); err != nil {
 			return err
 		}
 	}
+	return nil
+}
 
+func unmarshalStruct(b []byte, value reflect.Value) error {
+	for len(b) > 0 {
+		var err error
+		if b, err = unmarshalTLV(b, value); b == nil && err != nil {
+			return err
+		}
+	}
 	return nil
 }
 

pkg/hap/tlv8/tlv8_test.go

@@ -2,6 +2,7 @@ package tlv8
 
 import (
 	"encoding/hex"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -107,3 +108,49 @@ func TestInterface(t *testing.T) {
 
 	require.Equal(t, src, dst)
 }
+
+func TestSlice1(t *testing.T) {
+	var v struct {
+		VideoAttrs []struct {
+			Width     uint16 `tlv8:"1"`
+			Height    uint16 `tlv8:"2"`
+			Framerate uint8  `tlv8:"3"`
+		} `tlv8:"3"`
+	}
+
+	s := `030b010280070202380403011e ff00 030b010200050202d00203011e`
+	b1, err := hex.DecodeString(strings.ReplaceAll(s, " ", ""))
+	require.NoError(t, err)
+
+	err = Unmarshal(b1, &v)
+	require.NoError(t, err)
+
+	require.Len(t, v.VideoAttrs, 2)
+
+	b2, err := Marshal(v)
+	require.NoError(t, err)
+
+	require.Equal(t, b1, b2)
+}
+
+func TestSlice2(t *testing.T) {
+	var v []struct {
+		Width     uint16 `tlv8:"1"`
+		Height    uint16 `tlv8:"2"`
+		Framerate uint8  `tlv8:"3"`
+	}
+
+	s := `010280070202380403011e ff00 010200050202d00203011e`
+	b1, err := hex.DecodeString(strings.ReplaceAll(s, " ", ""))
+	require.NoError(t, err)
+
+	err = Unmarshal(b1, &v)
+	require.NoError(t, err)
+
+	require.Len(t, v, 2)
+
+	b2, err := Marshal(v)
+	require.NoError(t, err)
+
+	require.Equal(t, b1, b2)
+}