Redirect Loop Error with https store URLs and HTTPS reverse proxy

[matejak]

Describe the bug
I have a setup with a HTTPS nginx reverse proxy, and it works pretty well when the store URLs use the http:// URI. However, if I change the store URIs to https://, I keep getting redirect loops errors no matter what other settings I use. I think that I ran into a similar issue as #706, but I was not able to get it working. The user references that setting ForceSslForAllPages made it work for them, but I am not able to find that settings.
When reading https://smartstore.com/en/upgrade-for-https-configurations-in-smartstore-5/ I don't see the option that I want, which is https in the store URL, and no redirection if the X-Forwarded-Proto request header says https. I don't want to have HTTPS traffic between the proxy and the Smartstore container.

To Reproduce
Steps to reproduce the behavior:

1. Set up a SmartStore instance using a container served by a proxy nginx container.
   Configure the server according to the official documentation, set up a http->https redirect (although the SmartStore container now listens on port 80 rather than on port 5000)
2. Set up stores with HTTP URIs, and see that everything is working all right.
3. Change HTTP to HTTPS, and start experiencing redirect loops on the particular store, when the https://example.com would 301-redirect to https://example.com. This doesn't change at all if I flip the "Require HTTPS" in the store settings.

Expected behavior
There is no redirect loop, as the app should understand from the request headers that there is no need to redirect the client to the https port, because he already connects via https.

[muratcakir]

Please check, understand and modify the ReverseProxy section in appsettings.json to comply with your proxy configuration.

[matejak]

Thank you for your fast reply,
I have kept the default appsettings.json with this contents:

...
"ReverseProxy": {
  // "true" enables the ForwardedHeadersMiddleware.
  "Enabled": true,
  // Processes the X-Forwarded-For header, which identifies the originating IP address of the client.
  "ForwardForHeader": true,
  // Processes the X-Forwarded-Proto header, which identifies the protocol (HTTP or HTTPS) the client used to connect.
  "ForwardProtoHeader": true,
  // Processes the X-Forwarded-Host header, which identifies the original host requested by the client.
  "ForwardHostHeader": true,
  // Processes the X-Forwarded-Prefix header, which retrieves the value for the path base.
  "ForwardPrefixHeader": true,
  // Sets the header used to retrieve the originating client IP. Default: "X-Forwarded-For".
  "ForwardedForHeaderName": null,
  // Sets the header used to retrieve the value for the originating scheme (HTTP/HTTPS). Default: "X-Forwarded-Proto".
  "ForwardedProtoHeaderName": null,
  // Sets the header used to retrieve the original value of the Host header field. Default: "X-Forwarded-Host".
  "ForwardedHostHeaderName": null,
  // Sets the header used to retrieve the value for the path base. Default: "X-Forwarded-Prefix".
  "ForwardedPrefixHeaderName": null,
  // Array of known proxy IP addresses to accept forwarded headers from (as string array).
  "KnownProxies": null,
  // The allowed values from X-Forwarded-Host as string array. If the array is null or empty then all hosts are allowed.
  "AllowedHosts": null
},
...

I find them completely appropriate, perhaps except the "KnownProxies": null. Does the null mean that nothing is taken into the account, because no server is trusted as the proxy server? And, at the same time, is there a way to verify that those settings are truly effective in the deployment? The truth is that the default file isn't a valid JSON file due to the C#-like comments, so perhaps it is completely ignored.
And please what do you mean by "understand the ReverseProxy section"? Is there more to understand than to read the comments and the source code, or is there documentation or another explanation that I have missed?

[muratcakir]

The most important settings here are ForwardedForHeaderName, ForwardedProtoHeaderName and ForwardedHostHeaderName. Ensure that the header names sent by your proxy match those listed here. You can ignore KnownProxies.

You should also check the following settings in Configuration / Stores / [Store] for correctness: Require HTTPS, HTTPS port, and HOST values.

[gitekDev]

@matejak

i had the same Problem

What i have done in program.cs (Smartstore.Web):

    //Logger Test Headers
{
    var logger = context.RequestServices.GetRequiredService<ILogger<Program>>();

    var sb = new System.Text.StringBuilder();

    sb.AppendLine("=====DEBUG =====");
    sb.AppendLine($"Scheme        : {context.Request.Scheme}");
    sb.AppendLine($"IsHttps       : {context.Request.IsHttps}");
    sb.AppendLine($"Host          : {context.Request.Host}");
    sb.AppendLine($"Path          : {context.Request.Path}");
    sb.AppendLine($"Method        : {context.Request.Method}");
    sb.AppendLine($"RemoteIp      : {context.Connection.RemoteIpAddress}");
    sb.AppendLine($"RemotePort    : {context.Connection.RemotePort}");
    sb.AppendLine("");

    sb.AppendLine("---- Forwarded / Proxy Headers ----");
    sb.AppendLine($"X-Forwarded-Proto : {context.Request.Headers["X-Forwarded-Proto"]}");
    sb.AppendLine($"X-Forwarded-Host  : {context.Request.Headers["X-Forwarded-Host"]}");
    sb.AppendLine($"X-Forwarded-For   : {context.Request.Headers["X-Forwarded-For"]}");
    sb.AppendLine($"X-Forwarded-Port  : {context.Request.Headers["X-Forwarded-Port"]}");
    sb.AppendLine($"Forwarded         : {context.Request.Headers["Forwarded"]}");
    sb.AppendLine($"Via               : {context.Request.Headers["Via"]}");
    sb.AppendLine("");

    sb.AppendLine("---- ALL HEADERS RAW ----");
    foreach (var h in context.Request.Headers)
    {
        sb.AppendLine($"{h.Key}: {h.Value}");
    }

    sb.AppendLine("=====DEBUG =====");

    logger.LogWarning(sb.ToString());
    await next(context);
});
    //Logger Test Headers

[matejak]

@muratcakir I have put explicit values to the appsettings.json, and it didn't make a difference. Then, I have changed the ForwardedHostHeaderName to an invalid value, restarted the app, and I was still able to access different stores, which shouldn't have been possible if the app is mislead about where to get the hostname. Therefore, I think that the config file is completely ignored. I run Smartstore in the container, with the config file in the /app/appsettings.json path.

@gitekDev When I see your snippet, it seems that you don't change anything, and you just print messages to the console, which can be useful. Where have you added this debug code? What is the context? What about the }); at the end, that looks like unbalanced parentheses? And how were you able to fix the issue?

[muratcakir]

Therefore, I think that the config file is completely ignored

No, it isn't, trust me 😉 We use ForwardedHeadersMiddleware under the hood. See:

Smartstore/src/Smartstore.Web.Common/WebStarter.cs

Lines 126 to 148 in 6ef01ce

	var proxy = appContext.AppConfiguration.ReverseProxy;
	if (proxy != null && proxy.Enabled)
	{
	// Map the ForwaredHeadersMiddleware
	app.UseForwardedHeaders(MapForwardedHeadersOptions(proxy));
	if (proxy.ForwardPrefixHeader)
	{
	// The ForwardedHeadersMiddleware evaluates the X-Forwarded-Prefix correctly
	// by setting PathBase, but does not necessarily substitute the base path
	// from the current request path (which seems to be by design). In order for routing to
	// work correctly we have to deal with it programmatically.
	app.Use((context, next) =>
	{
	var req = context.Request;
	if (req.PathBase.HasValue && req.Path.StartsWithSegments(req.PathBase, out var remainingPath))
	{
	req.Path = remainingPath;
	}
	return next(context);
	});
	}

[gitekDev]

@matejak

What is your setup at this moment?
I had the same problem this weekend (my code is for more logs + debug mode in appsettings.json).

My Setup is
BunnyCDN (with hostnames) -> Hetzuner Load Balancer -> 2 Server

And after this debug + Logs i had to change Core files in Smartstore (because of loop)

In File = src/Smartstore.Web.Common/WebStarter.cs
From = ForwardLimit = 2 (hard coded)
To =
ForwardLimit = config.ForwardLimit ?? 2,

• add
  RequireHeaderSymmetry = config.RequireHeaderSymmetry

• From

            if (config.KnownProxies != null)
            {
                var addresses = config.KnownProxies
                    .Select(x =>
                    {
                        if (IPAddress.TryParse(x, out var ip))
                        {
                            return ip;
                        }

                        return null;
                    })
                    .Where(x => x != null)
                    .ToArray();

                options.KnownProxies.AddRange(addresses);
                if (addresses.Length > 0)
                {
                    // Disable limit, because at least one KnownProxy is configured.
                    options.ForwardLimit = null;
                }

To

            if (config.KnownProxies != null)
            {
                var addresses = config.KnownProxies
                    .Select(x =>
                    {
                        if (IPAddress.TryParse(x, out var ip))
                        {
                            return ip;
                        }

                        return null;
                    })
                    .Where(x => x != null)
                    .ToArray();

                options.KnownProxies.AddRange(addresses);
                if (addresses.Length > 0 && !config.ForwardLimit.HasValue)
                {
                    // Disable limit, because at least one KnownProxy is configured (unless explicitly set in config).
                    options.ForwardLimit = null;
                }

More Changes:
Add in appsettings.json
"ForwardLimit": 3,
"RequireHeaderSymmetry": false

Add in src/Smartstore/Engine/SmartConfiguration.cs

            /// <summary>
            /// Limits the number of entries processed from forwarded headers.
            /// Default is 2 if not specified.
            /// </summary>
            public int? ForwardLimit { get; set; }

            /// <summary>
            /// Indicates whether all forward headers must have the same number of values.
            /// Set to false when using multi-hop proxies where header counts may differ
            /// </summary>
            public bool RequireHeaderSymmetry { get; set; } = true;

Important and caution advised

If you update to a new Smartstore version, your changes will be lost and will need to be adjusted again, unless the update is integrated into the core and my method was correct. @muratcakir

As a side note, I'm trying to help you, but I'm not a .NET expert or Smartstore expert (i am new to Smartstore) and I'm still learning a lot myself (it took me a whole weekend :) )
If I've done something wrong, please correct me.

[muratcakir]

@gitekDev Thank you for contributing

[matejak]

I propose to repoen the issue, because the patch doesn't make any difference for my use case, when I have a simple docker-compose setup with a store container and an nginx proxy container. However, thanks to your involvement, I was able to nail the issue down further.
I can confirm that after I populate the KnownProxies with a correct IP address, the problem with the redirect loop goes away.
However, this is quite impractical, as it only works with an address, and not a range or a wildcard, and in the container environment, addresses are often dynamic. I am pretty sure that it is because of the middleware: https://github.com/dotnet/aspnetcore/blob/d3aba8fe1a0d0f5c145506f292b72ea9d28406fc/src/Middleware/HttpOverrides/src/ForwardedHeadersMiddleware.cs#L241
I thought that I could define KnownNetworks or KnownIPNetworks, but I wasn't able to get it working at all. Neither was I able to get any debug logs anywhere, so why that doesn't work is a mystery, because KnownNetworks should work in a similar way.
Anyway, I fixed it by defining ASPNETCORE_FORWARDEDHEADERS_ENABLED env variable to true - then, it doesn't require the KnownProxies either, and it should be fine as long as the proxy setup is not "buggy", whatever that means. I found it by following https://learn.microsoft.com/en-us/dotnet/core/compatibility/aspnet-core/8.0/forwarded-headers-unknown-proxies
I have outstanding questions:

• How to set up logging? I have edited the serilog section of appsettings.json to no avail - I wasn't able to see anything Debug in the console or in the App_data/Logs or even in the web interface.
• Any idea how to define KnownNetworks in appsettings.json? Some online resources say to use a list of CIDRs, but it didn't work for me, and I don't know what's going on without debug logging.

I would close this issue when it is documented somewhere that the middleware requires explicit listing of proxy IPs, or even better, if it is possible to define known networks in the appsettings. As I understand now, even the most trivial docker-compose deployment with a proxy will only work when stores have http-only base URLs, which doesn't work when it comes to setting up payment gateways.