diff --git a/builder.go b/builder.go index 4e02f1c8..ba2d743e 100644 --- a/builder.go +++ b/builder.go @@ -118,6 +118,8 @@ func (builder *serverBuilder) build() (*Server, error) { builder.featureFlagProvider = &unleash.NullFeatureFlagProvider{} } + unleash.Init(builder.featureFlagProvider) + s := &Server{ dataDir: builder.dataDir, databaseDir: builder.databaseDir, diff --git a/imap/structure.go b/imap/structure.go index 24b15c3a..3953074a 100644 --- a/imap/structure.go +++ b/imap/structure.go @@ -2,11 +2,18 @@ package imap import ( "bytes" + "errors" "strings" + "github.com/ProtonMail/gluon/internal/unleash" + "github.com/ProtonMail/gluon/internal/unleash/featureflags" "github.com/ProtonMail/gluon/rfc822" ) +var errorMaximumMIMEStructureDepthExceeded = errors.New("maximum mime structure depth exceeded") + +const maxMIMEStructureDepth = 64 + func Structure(section *rfc822.Section) (string, string, error) { bodyBuilder := strings.Builder{} structureBuilder := strings.Builder{} @@ -14,7 +21,7 @@ func Structure(section *rfc822.Section) (string, string, error) { writer := dualParListWriter{b1: &bodyBuilder, b2: &structureBuilder} c := newParamListWithGroup(&writer) - if err := structure(section, &c, &writer); err != nil { + if err := structure(section, &c, &writer, 0); err != nil { return "", "", err } @@ -26,17 +33,30 @@ func Structure(section *rfc822.Section) (string, string, error) { return body, structure, nil } -func structure(section *rfc822.Section, fields *paramList, writer *dualParListWriter) error { +func structure(section *rfc822.Section, fields *paramList, writer *dualParListWriter, depth int) error { + maxMimeStructureDepthDisabled := true + + featureFlagValueProvider := unleash.Get() + if featureFlagValueProvider != nil { + maxMimeStructureDepthDisabled = featureFlagValueProvider.GetFlagValue(featureflags.MaximumMIMEStructureDepthDisabled) + } + + if !maxMimeStructureDepthDisabled { + if depth > maxMIMEStructureDepth { + return errorMaximumMIMEStructureDepthExceeded + } + } + children, err := section.Children() if err != nil { return err } if len(children) == 0 { - return singlePartStructure(section, fields, writer) + return singlePartStructure(section, fields, writer, depth) } - if err := childStructures(section, fields, writer); err != nil { + if err := childStructures(section, fields, writer, depth); err != nil { return err } @@ -61,7 +81,7 @@ func structure(section *rfc822.Section, fields *paramList, writer *dualParListWr return nil } -func singlePartStructure(section *rfc822.Section, fields *paramList, writer *dualParListWriter) error { +func singlePartStructure(section *rfc822.Section, fields *paramList, writer *dualParListWriter, depth int) error { header, err := section.ParseHeader() if err != nil { return err @@ -97,7 +117,7 @@ func singlePartStructure(section *rfc822.Section, fields *paramList, writer *dua cstruct := fields.newChildList(writer) - if err := structure(child, &cstruct, writer); err != nil { + if err := structure(child, &cstruct, writer, depth+1); err != nil { return err } @@ -117,7 +137,7 @@ func singlePartStructure(section *rfc822.Section, fields *paramList, writer *dua return nil } -func childStructures(section *rfc822.Section, c *paramList, writer *dualParListWriter) error { +func childStructures(section *rfc822.Section, c *paramList, writer *dualParListWriter, depth int) error { children, err := section.Children() if err != nil { return err @@ -126,7 +146,7 @@ func childStructures(section *rfc822.Section, c *paramList, writer *dualParListW for _, child := range children { cl := c.newChildList(writer) - if err := structure(child, &cl, writer); err != nil { + if err := structure(child, &cl, writer, depth+1); err != nil { return err } diff --git a/imap/structure_test.go b/imap/structure_test.go index 44c46fb1..c2fe878a 100644 --- a/imap/structure_test.go +++ b/imap/structure_test.go @@ -5,6 +5,8 @@ import ( "path/filepath" "testing" + "github.com/ProtonMail/gluon/internal/unleash" + "github.com/ProtonMail/gluon/internal/unleash/featureflags" "github.com/stretchr/testify/require" ) @@ -75,15 +77,15 @@ hey there bro } func TestParseInvalidCharsInContenType(t *testing.T) { - const literal = `From: Nathaniel Borenstein -To: Ned Freed -Subject: Sample message -MIME-Version: 1.0 -Content-type: multipart/mixed; boundary="simple boundary" - -This is the preamble. It is to be ignored, though it -is a handy place for mail composers to include an -explanatory note to non-MIME compliant readers. + const literal = `From: Nathaniel Borenstein +To: Ned Freed +Subject: Sample message +MIME-Version: 1.0 +Content-type: multipart/mixed; boundary="simple boundary" + +This is the preamble. It is to be ignored, though it +is a handy place for mail composers to include an +explanatory note to non-MIME compliant readers. --simple boundary Content-type: text/plain; charset=us-ascii @@ -95,7 +97,7 @@ X-Pm-Content-Encryption: on-import To: someone Subject: Fwd: embedded -Content-type: multipart/mixed; boundary="embedded-boundary" +Content-type: multipart/mixed; boundary="embedded-boundary" --embedded-boundary Content-Type: GIF �ɮ�; @@ -119,15 +121,15 @@ This is the epilogue. It is also to be ignored. } func TestParseInvalidMimeType(t *testing.T) { - const literal = `From: Nathaniel Borenstein -To: Ned Freed -Subject: Sample message -MIME-Version: 1.0 -Content-type: multipart/mixed; boundary="simple boundary" - -This is the preamble. It is to be ignored, though it -is a handy place for mail composers to include an -explanatory note to non-MIME compliant readers. + const literal = `From: Nathaniel Borenstein +To: Ned Freed +Subject: Sample message +MIME-Version: 1.0 +Content-type: multipart/mixed; boundary="simple boundary" + +This is the preamble. It is to be ignored, though it +is a handy place for mail composers to include an +explanatory note to non-MIME compliant readers. --simple boundary Content-type: text/plain; charset=us-ascii @@ -139,7 +141,7 @@ X-Pm-Content-Encryption: on-import To: someone Subject: Fwd: embedded -Content-type: multipart/mixed; boundary="embedded-boundary" +Content-type: multipart/mixed; boundary="embedded-boundary" --embedded-boundary Content-Type: application/; @@ -184,3 +186,49 @@ func FuzzNewParsedMessage(f *testing.F) { _, _ = NewParsedMessage(inputData) }) } + +func TestMaxMIMEStructureDepthExceeded_KillSwitch_Disabled(t *testing.T) { + flags := map[string]bool{ + featureflags.MaximumMIMEStructureDepthDisabled: false, + } + mockProvider := unleash.NewMockFeatureFlagValueProvider(flags) + unleash.Init(mockProvider) + + eml, err := os.ReadFile(filepath.Join("testdata", "mime-structure-depth.eml")) + require.NoError(t, err) + _, err = NewParsedMessage(eml) + require.Error(t, err) + require.ErrorIs(t, err, errorMaximumMIMEStructureDepthExceeded) + + t.Cleanup(func() { + unleash.Init(nil) + }) +} + +func TestMaxMIMEStructureDepthExceeded_KillSwitch_Enabled(t *testing.T) { + flags := map[string]bool{ + featureflags.MaximumMIMEStructureDepthDisabled: true, + } + mockProvider := unleash.NewMockFeatureFlagValueProvider(flags) + unleash.Init(mockProvider) + + eml, err := os.ReadFile(filepath.Join("testdata", "mime-structure-depth.eml")) + require.NoError(t, err) + _, err = NewParsedMessage(eml) + require.NoError(t, err) + + t.Cleanup(func() { + unleash.Init(nil) + }) +} + +func TestMaxMIMEStructureDepthExceeded_NoFFProvider(t *testing.T) { + eml, err := os.ReadFile(filepath.Join("testdata", "mime-structure-depth.eml")) + require.NoError(t, err) + _, err = NewParsedMessage(eml) + require.NoError(t, err) + + t.Cleanup(func() { + unleash.Init(nil) + }) +} diff --git a/imap/testdata/mime-structure-depth.eml b/imap/testdata/mime-structure-depth.eml new file mode 100644 index 00000000..75f1bb22 --- /dev/null +++ b/imap/testdata/mime-structure-depth.eml @@ -0,0 +1,267 @@ +Content-Type: multipart/mixed; boundary="b0" + +--b0 +Content-Type: multipart/mixed; boundary="b1" + +--b1 +Content-Type: multipart/mixed; boundary="b2" + +--b2 +Content-Type: multipart/mixed; boundary="b3" + +--b3 +Content-Type: multipart/mixed; boundary="b4" + +--b4 +Content-Type: multipart/mixed; boundary="b5" + +--b5 +Content-Type: multipart/mixed; boundary="b6" + +--b6 +Content-Type: multipart/mixed; boundary="b7" + +--b7 +Content-Type: multipart/mixed; boundary="b8" + +--b8 +Content-Type: multipart/mixed; boundary="b9" + +--b9 +Content-Type: multipart/mixed; boundary="b10" + +--b10 +Content-Type: multipart/mixed; boundary="b11" + +--b11 +Content-Type: multipart/mixed; boundary="b12" + +--b12 +Content-Type: multipart/mixed; boundary="b13" + +--b13 +Content-Type: multipart/mixed; boundary="b14" + +--b14 +Content-Type: multipart/mixed; boundary="b15" + +--b15 +Content-Type: multipart/mixed; boundary="b16" + +--b16 +Content-Type: multipart/mixed; boundary="b17" + +--b17 +Content-Type: multipart/mixed; boundary="b18" + +--b18 +Content-Type: multipart/mixed; boundary="b19" + +--b19 +Content-Type: multipart/mixed; boundary="b20" + +--b20 +Content-Type: multipart/mixed; boundary="b21" + +--b21 +Content-Type: multipart/mixed; boundary="b22" + +--b22 +Content-Type: multipart/mixed; boundary="b23" + +--b23 +Content-Type: multipart/mixed; boundary="b24" + +--b24 +Content-Type: multipart/mixed; boundary="b25" + +--b25 +Content-Type: multipart/mixed; boundary="b26" + +--b26 +Content-Type: multipart/mixed; boundary="b27" + +--b27 +Content-Type: multipart/mixed; boundary="b28" + +--b28 +Content-Type: multipart/mixed; boundary="b29" + +--b29 +Content-Type: multipart/mixed; boundary="b30" + +--b30 +Content-Type: multipart/mixed; boundary="b31" + +--b31 +Content-Type: multipart/mixed; boundary="b32" + +--b32 +Content-Type: multipart/mixed; boundary="b33" + +--b33 +Content-Type: multipart/mixed; boundary="b34" + +--b34 +Content-Type: multipart/mixed; boundary="b35" + +--b35 +Content-Type: multipart/mixed; boundary="b36" + +--b36 +Content-Type: multipart/mixed; boundary="b37" + +--b37 +Content-Type: multipart/mixed; boundary="b38" + +--b38 +Content-Type: multipart/mixed; boundary="b39" + +--b39 +Content-Type: multipart/mixed; boundary="b40" + +--b40 +Content-Type: multipart/mixed; boundary="b41" + +--b41 +Content-Type: multipart/mixed; boundary="b42" + +--b42 +Content-Type: multipart/mixed; boundary="b43" + +--b43 +Content-Type: multipart/mixed; boundary="b44" + +--b44 +Content-Type: multipart/mixed; boundary="b45" + +--b45 +Content-Type: multipart/mixed; boundary="b46" + +--b46 +Content-Type: multipart/mixed; boundary="b47" + +--b47 +Content-Type: multipart/mixed; boundary="b48" + +--b48 +Content-Type: multipart/mixed; boundary="b49" + +--b49 +Content-Type: multipart/mixed; boundary="b50" + +--b50 +Content-Type: multipart/mixed; boundary="b51" + +--b51 +Content-Type: multipart/mixed; boundary="b52" + +--b52 +Content-Type: multipart/mixed; boundary="b53" + +--b53 +Content-Type: multipart/mixed; boundary="b54" + +--b54 +Content-Type: multipart/mixed; boundary="b55" + +--b55 +Content-Type: multipart/mixed; boundary="b56" + +--b56 +Content-Type: multipart/mixed; boundary="b57" + +--b57 +Content-Type: multipart/mixed; boundary="b58" + +--b58 +Content-Type: multipart/mixed; boundary="b59" + +--b59 +Content-Type: multipart/mixed; boundary="b60" + +--b60 +Content-Type: multipart/mixed; boundary="b61" + +--b61 +Content-Type: multipart/mixed; boundary="b62" + +--b62 +Content-Type: multipart/mixed; boundary="b63" + +--b63 +Content-Type: multipart/mixed; boundary="b64" + +--b64 +Content-Type: multipart/mixed; boundary="b65" + +--b65 +Content-Type: text/plain + +leaf +--b65-- +--b64-- +--b63-- +--b62-- +--b61-- +--b60-- +--b59-- +--b58-- +--b57-- +--b56-- +--b55-- +--b54-- +--b53-- +--b52-- +--b51-- +--b50-- +--b49-- +--b48-- +--b47-- +--b46-- +--b45-- +--b44-- +--b43-- +--b42-- +--b41-- +--b40-- +--b39-- +--b38-- +--b37-- +--b36-- +--b35-- +--b34-- +--b33-- +--b32-- +--b31-- +--b30-- +--b29-- +--b28-- +--b27-- +--b26-- +--b25-- +--b24-- +--b23-- +--b22-- +--b21-- +--b20-- +--b19-- +--b18-- +--b17-- +--b16-- +--b15-- +--b14-- +--b13-- +--b12-- +--b11-- +--b10-- +--b9-- +--b8-- +--b7-- +--b6-- +--b5-- +--b4-- +--b3-- +--b2-- +--b1-- +--b0-- diff --git a/internal/unleash/featureflags/flags.go b/internal/unleash/featureflags/flags.go index 0e723acf..fe931ad1 100644 --- a/internal/unleash/featureflags/flags.go +++ b/internal/unleash/featureflags/flags.go @@ -6,4 +6,5 @@ const ( ConnectionLimiterDisabled = "InboxBridgeGluonConnectionLimiterDisabled" ConnectionLimiterDefaultLimitsDisabled = "InboxBridgeGluonConnectionLimiterDefaultLimitsDisabled" ConnectionCounterConnectionsLimitDisabled = "InboxBridgeGluonRollingCounterConnectionLimitDisabled" + MaximumMIMEStructureDepthDisabled = "InboxBridgeGluonMaximumMimeStructureDepthLimitDisabled" ) diff --git a/internal/unleash/mock.go b/internal/unleash/mock.go new file mode 100644 index 00000000..fbab1fb4 --- /dev/null +++ b/internal/unleash/mock.go @@ -0,0 +1,15 @@ +package unleash + +type MockFeatureFlagValueProvider struct { + flags map[string]bool +} + +func NewMockFeatureFlagValueProvider(flags map[string]bool) FeatureFlagValueProvider { + return &MockFeatureFlagValueProvider{ + flags: flags, + } +} + +func (ff *MockFeatureFlagValueProvider) GetFlagValue(key string) bool { + return ff.flags[key] +} diff --git a/internal/unleash/singleton.go b/internal/unleash/singleton.go new file mode 100644 index 00000000..a93222cf --- /dev/null +++ b/internal/unleash/singleton.go @@ -0,0 +1,26 @@ +package unleash + +import ( + "sync" + "testing" +) + +var ( + instance FeatureFlagValueProvider + syncOnce sync.Once +) + +func Init(provider FeatureFlagValueProvider) { + if testing.Testing() { + instance = provider + return + } + + syncOnce.Do(func() { + instance = provider + }) +} + +func Get() FeatureFlagValueProvider { + return instance +} diff --git a/internal/unleash/unleash.go b/internal/unleash/unleash.go index 0f07eacf..abb6978c 100644 --- a/internal/unleash/unleash.go +++ b/internal/unleash/unleash.go @@ -1,9 +1,7 @@ package unleash -import "github.com/ProtonMail/gluon/imap" - var CapabilityKillSwitchMap = map[string]string{ - string(imap.IDLE): `InboxBridgeImapIdleCapabilityDisabled`, + "IDLE": `InboxBridgeImapIdleCapabilityDisabled`, // maps to `imap.Idle`, removed dependency due to circular import } type FeatureFlagValueProvider interface { diff --git a/option.go b/option.go index 0b667b71..9459df8f 100644 --- a/option.go +++ b/option.go @@ -286,7 +286,8 @@ func WithConnectionRollingCounter(connectionLimitThreshold, observabilityThresho observabilityThreshold, numberOfBuckets, thresholdCheckInterval, - )} + ), + } } type withFeatureFlagProvider struct {