🐛 [Bug]: setting a Logger that access TLSConnectionState() will break when `app.Server().MaxConnsPerIP` is set to a value

[rabarar]

Bug Description

There is a condition where the TLSConnectionState from Context( ) is nil - only when app.Server( ).MaxConnPerIP is set to a value.

How to Reproduce

1. Create a new app
2. Set the app.Server( ).MaxConnPerIP = 1
3. use a custom logger

        DisableColors: false,
        Format:        "${date} ${time} ${ip} ${status} - ${method} ${path} ${yellow}SerialNo[${serialNumber}]\n",
        TimeFormat:    "2006-01-02 15:04:05",
        TimeZone:      "Local",
        CustomTags: map[string]logger.LogFunc{
            "serialNumber": func(output logger.Buffer, c *fiber.Ctx, data *logger.Data, extraParam string) (int, error) {
                return output.WriteString(outputSerialFromContext(c))

            },
        },
    }))

4. receive an incoming route ...
5. boom

Expected Behavior

no sigsegv

Fiber Version

v2@v2.52.4

Code Snippet (optional)

package main

import "github.com/gofiber/fiber/v3"
import "log"

func main() {
  app := fiber.New()

  // Steps to reproduce

  log.Fatal(app.Listen(":3000"))
}

Checklist:

• I agree to follow Fiber's Code of Conduct.
• I have checked for existing issues that describe my problem prior to opening this one.
• I understand that improperly formatted bug reports may be closed without explanation.

[ReneWerner87]

Pls share the code for outputSerialFromContext

And the error message

[rabarar]

func outputSerialFromContext(c *fiber.Ctx) string { return c.Context().TLSConnectionState().PeerCertificates[0].SerialNumber.String() }

…

On Apr 27, 2024, at 6:44 PM, RW ***@***.***> wrote: Pls share rhs code for outputSerialFromContext — Reply to this email directly, view it on GitHub <#2990 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABB2J6D6VVZA6KGFCOEG3UTY7QS3RAVCNFSM6AAAAABG4MVTPKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBRGIZDAOBXHA>. You are receiving this because you authored the thread.

[ReneWerner87]

You need to make a copy https://docs.gofiber.io/#zero-allocation

[rabarar]

Nothing is persisting outside of the context call. The logger function prints the value from the tls connection state in the logger.Everything works as expected until you set the Server MaxConnsPerIP and then the TLSConnectionState returns nilOn Apr 28, 2024, at 2:13 AM, RW ***@***.***> wrote: You need to make a copy https://docs.gofiber.io/#zero-allocation —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>

[rabarar]

I think the bug/issue is in the fasthttp package - The issue only occurs when the `MaxConnsPerIP` is greater than zero. Looking at the code in the package, they’re creating the connection differently in that case and perhaps that’s why the TLSConnectionState() is nil - but for it to be nil would mean that the connection isn’t a TLS connection - which isn’t the case for me…

…

On Apr 28, 2024, at 2:13 AM, RW ***@***.***> wrote: ch You need to make a copy https://docs.gofiber.io/#zero-allocation — Reply to this email directly, view it on GitHub <#2990 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABB2J6FXL3TSQMX6X24G4WDY7SHQ3AVCNFSM6AAAAABG4MVTPKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBRGM2DSOJQGM>. You are receiving this because you authored the thread.

[rabarar]

here's a small example that demonstrates the bug:

package main

import (
        "crypto"
        "crypto/tls"
        "fmt"
        "os"

        "github.com/gofiber/fiber/v2"
        //"golang.org/x/crypto/pkcs12"

        "github.com/gofiber/fiber/v2/log"
        "github.com/gofiber/fiber/v2/middleware/logger"
        "software.sslmate.com/src/go-pkcs12"
)

func initFiberApp() *fiber.App {
        app := fiber.New()
        app.Server().MaxConnsPerIP = 1

        app.Use(logger.New(logger.Config{
                DisableColors: false,
                Format:        "${date} ${time} ${ip} ${status} - ${method} ${path} ${yellow}SerialNo[${serialNumber}]\n",
                TimeFormat:    "2006-01-02 15:04:05",
                TimeZone:      "Local",
                CustomTags: map[string]logger.LogFunc{
                        "serialNumber": func(output logger.Buffer, c *fiber.Ctx, data *logger.Data, extraParam string) (int, error) {
                                return output.WriteString(c.Context().TLSConnectionState().PeerCertificates[0].SerialNumber.String())

                        },
                },
        }))

        app.Get("/", func(c *fiber.Ctx) error {
                if c.Context().TLSConnectionState() != nil && len(c.Context().TLSConnectionState().PeerCertificates) == 0 {
                        log.Warn("no peer certs")
                } else {
                        log.Info("peer certs exist")
                }
                return c.SendString(fmt.Sprintf("This page is being served over TLS using a PKCS12 store type: %s",
                        c.Context().TLSConnectionState().PeerCertificates[0].SerialNumber.String()))
        })

        return app
}

func initTLSConfig(path string, password string) (*tls.Certificate, error) {
        pkcs12Data, err := os.ReadFile(path)
        if err != nil {
                return nil, err
        }

        key, cert, _, err := pkcs12.DecodeChain(pkcs12Data, password)

        if err != nil {
                return nil, err
        }

        tlsCert := tls.Certificate{
                Certificate: [][]byte{cert.Raw},
                PrivateKey:  key.(crypto.PrivateKey),
                Leaf:        cert,
        }

        return &tlsCert, nil
}

func main() {
        path := "./security/server.p12"
        password := "foobar"

        tlsCert, error := initTLSConfig(path, password)

        if error != nil {
                fmt.Printf("Unable to initialize TLS configuration object. Check your configuration and try again. Program will STOP: %s", error)
        } else {
                config := &tls.Config{
                        Certificates: []tls.Certificate{*tlsCert},
                        ClientAuth:   tls.RequireAndVerifyClientCert,
                }

                app := initFiberApp()
                ln, err := tls.Listen("tcp", ":5443", config)
                if err != nil {
                        panic(err)
                }

                log.Fatal(app.Listener(ln))
        }
}

if you set MaxConnsPerIP > 0 you'll get a sigseg violation - the issue is in the worker pool implementation for fasthttps...

I think you can close this out and I should open it with their package...

[rabarar]

FYI - from valyala/fasthttp "I have pushed a fix. I'll tag a release next week probably."

[gaby]

Fixed by FastHTTP via: valyala/fasthttp@105eb3b

Fixed in Fiber via: #3010

Related: valyala/fasthttp#1770

@rabarar While this is fixed, the fix only applies to Fiber v3, the latest releases from fasthttp are no longer compatible with Fiber v2.

[rabarar]

Is there a timeline for the v3 production release?

…

On May 27, 2024, at 8:35 AM, Juan Calderon-Perez ***@***.***> wrote: Closed #2990 <#2990> as completed. — Reply to this email directly, view it on GitHub <#2990 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABB2J6C6XJL36S6G4XEZFHLZEMR7ZAVCNFSM6AAAAABG4MVTPKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSHE2DKNBVHEZTINY>. You are receiving this because you were mentioned.