-
Notifications
You must be signed in to change notification settings - Fork 58
Discussion for: Publish for IIS changes to web.config location #158
Comments
Does this mean |
I'm a bit concerned about this change. I was pretty happy seeing that IIS points directly to
Now to be 100% sure that iis will under no circumstances serve these files, we need to go back to blocking various path segments or extensions in IIS or go deeper with the structure to something like
Which makes me feel like the main point of |
The module for ASP.NET core completely takes the request from IIS so there is no chance it would be served unless you configured IIS to specifically serve them. |
Yes |
@tuespetre I understand how it works in "well configured" scenarios, but imagine the deployment fails somewhere and for some reason deletes web.config. Now you have IIS with default configuration pointed in your directory with configs, logs and possibly other private data and IIS will be happy to serve them. I agree, that this is scenario that's just not supposed to happen, but in real life these things >can< happen and when it comes to this sensitive information, one has to be extra careful. What I dislike about this change is that it decreases the convenience of achieving such security from the default back to the old asp.net where you have to rely on IIS being configured properly on machine level and not serve files from certain 'magic' paths or extensions or alter the project structure accordingly and do extra steps that are different on local vs staging/production environments (like locating configuration files). |
@Kukkimonsuta this is our new default because of the technology and ecosystem we've built around web hosting in IIS. If you control everything, you can certainly move the |
Yeah ... This is kind'a serious (logs prob wouldn't have the right extension to pass, but JSON and XML is cool with IIS). If you don't have a Just to demo this, I removed ... well ... this is a bit 💀 scary 💀 ... you know things are going to end badly for someone down the road. cc/ @blowdart @danroth27 @Rick-Anderson ... at the very least I'll add a warning in the |
Oh believe me I've already expressed my reservations about this change. However a malformed web.config will just error out, and nothing gets past that. |
@blowdart Almost true .... The schema will match and pass if the <?xml version="1.0" encoding="utf-8"?>
<configuration>
<system.webServer>
<handlers>
</handlers>
<aspNetCore processPath=".\teststandalone.exe" stdoutLogEnabled="true" stdoutLogFile=".\logs\teststandalone" />
</system.webServer>
</configuration> It will not error out and IIS will serve all of that content. Of course, that's an unlikely scenario. It's much more likely that the file will just be absent or accidentally renamed. Once, I saw a I can at the very least caution folks in the guidance about this, but this doesn't have any easy, obvious fix AFAICT. The second one makes the IIS website physical path the application base path containing JSON, XML, TXT, and other files that IIS will be happy to serve, this risk will present itself. @Kukkimonsuta is right on point. |
I think we could go further in documentation, and at least walk people through how to configure IIS not to serve .json files |
It could complicate matters if the app is supposed to serve dev-sanctioned JSON files to enforce a blanket extension filter; as you know, IIS would stop all such requests dead in their tracks. It feels like you almost end up having to tell server admins to make an IIS server-wide change and add these sensitive known filenames of known config files to the Even that leaves a hole where devs may come along and not realize what they're doing by adding a custom-named JSON or XML config file anywhere in the app that they didn't explicitly tell IIS not to serve. If the Anywho ... I'll try to craft some good language on this one. I'll ping you back on it when I get some draft language in the draft PR, which I might have by the end of the day. |
|
3 words: lazy, stupid and non-sense. |
Nope, it is the wwwroot folder as IIS still isn't responsible for servicing static files.
It was a hack and didn't work well with most hosters configuration. With the new layout things were being dumped one folder up from the wwwroot. We looked at a few options but ultimately this one had the best set of trade offs and on the plus side required no server side changes to web deploy. |
@davidfowl but this way we can't really ensure the hosting provider's IIS config, therefore it's more possible to serve the code or the config as static files than the old way. It is fairly possible to a broken IIS extension, that would try to serve our private files to public, could be installed to the hosting. I agree with your point, but every option has some advantages and disadvantages in computer field, why wouldn't we just consider two approaches and find the best one here (or better, two approaches, but what developer choices for)? |
Referencing other issue for moving the |
If you control the hosting environment, then as @guardrex mentions, you'll be able to go back to the former model which is safer as IIS never sees anything but the wwwroot folder.
Yep, we chose the best one given the trade offs. This is what we do everyday (as you mentioned). |
why the @guardrex approach (if it works) cannot be the default? |
@Bartmax because we can't change every hoster and IIS server that exists on the planet. When you host node on IIS it's the same idea (and it sucks) but that's just how IIS runs. |
The |
@giggio I did a search, and it looks like that particular design decision was made in ancient times. It would be interesting to hear the history and reasoning on |
@davidfowl That was should to be said when HttpPlatformHandler was introduced (and AspNetCoreModule). |
@danroth27 @davidfowl @blowdart What about the 'bin' folder? Chatting with @guardrex, we're seeing that it appears to be a default 'hidden segment' configured in IIS -- and while static files and view templates technically aren't binaries, that could offer a more secure 'easy default' for IIS deployments. What do you think? Publish outputs into a bin folder with a sibling web.config that points into it? |
Let's also consider the guidance to remove the IIS static file module from @danroth27 https://github.com/danroth27 @davidfowl What about the 'bin' folder? Chatting with @guardrex — |
Wait, why are views being served anyway? .cshtml should be in the unsafe list. |
Removing the IIS module sounds like a good idea, if the hoster supports it, and that section isn't locked :) |
@blowdart right ... if they can do it. Otherwise, they will prob only have one option .... the workaround that moves web.config back to wwwroot. |
@blowdart I was commenting more about "those other files that aren't binaries" being deployed into the bin folder. |
For the unofficial Slack #iis channel "IIS Hotline! Links, Notes & Tips," I say ...
|
What about URL rewrite?
|
@AR1ES I don't have too much experience with needing to mod the default behavior with regard to trailing slashes; but having said that, I take it you aren't interested in going with route options ... using Microsoft.AspNetCore.Routing;
services.Configure<RouteOptions>(options =>
{
options.AppendTrailingSlash = true;
}); -- OR -- services.AddRouting(options => {
options.AppendTrailingSlash = true;
}); [EDIT] dotnet/aspnetcore#1459 |
@guardrex These options are responsible only for the links on the site but not for real redirects. Of course I can write a few lines of code for this purpose. But I'm sure it's not the only problem, which is caused by the change of the app root folder. And I would like to use the standard IIS capabilities. |
@AR1ES I haven't had to achieve the type of redirects that you're going for here ... I don't even understand what you're trying to achieve. My recommendation is to open an issue over in https://github.com/aspnet/IISIntegration/issues and hit up The Honorable Mr. Chris @Tratcher, Esq. 😎, who can provide more assistance than I on this one. |
We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have. |
Discussion for aspnet/Announcements#173
The text was updated successfully, but these errors were encountered: