iis-docs icon indicating copy to clipboard operation
iis-docs copied to clipboard

SFI images IIS-docs sev1-2 updates only

Open wadepickett opened this issue 1 year ago • 22 comments

Contributes to dotnet/AspNetCore.Docs#33750

All SFI image updates to fix sev 1-2 issues per SFI issue instruction.

There are also around 18 images (of the 61 total) that had SFI sev 1-2 issues that were orphaned, with no content referring to them that will be deleted, but those will be in a sperate PR.

wadepickett avatar Oct 28 '24 01:10 wadepickett

It seems extreme to blank out localhost IP addresses, but if that's what we're directed to do, this PR LGTM

I'd like to pushback and get some reason why. There's many tools and operations that rely on the loopback IP

Rick-Anderson avatar Oct 28 '24 16:10 Rick-Anderson

@John-Hart (or his delegate) approval needed before merging this

Rick-Anderson avatar Oct 28 '24 17:10 Rick-Anderson

@wadepickett take a look at how the Azure folks are handling similar tools. They give a tepid informational message at the top of the article.

Rick-Anderson avatar Oct 28 '24 17:10 Rick-Anderson

@wadepickett can you cite a reference to removing loop back IP?

Rick-Anderson avatar Oct 28 '24 17:10 Rick-Anderson

Thanks for the suggestions and info:

We were instructed to remove what was called out, as a top priority, and IP's were specifically called out in those cases, so that is what I was doing. To me, any example simply used as an fictional example, which would be 99% of all of the doc issues, would be an odd high bar to hit , but they were intentionally flagging fictional examples from what I could see. Almost all the flags are for fictional examples for all doc repos.

Rick, do you have a link to an example you like where the images are not addressed as requested, but instead they use an informational message at the top of the doc that you are referring to on Azure?

Who do we push back to? The SFI engineers?

The images were updated, not deleted and gone. For binaries, they are replaced when you update and there is no way around that. It is confusing when you refer to images as just "deleted" when they have been updated. It would be helpful if you communicate which you are referring to.

wadepickett avatar Oct 28 '24 17:10 wadepickett

We were instructed to remove what was called out,

Where were we instructed to blindly remove everything flagged? AFAIK, we were instructed to review everything flagged. Unless you can prove the loop back IP is a risk, it should be reviewed and exempted.

Almost all the flags are for fictional examples for all doc repos.

Loop back is not fictional, it's an IP needed to for loop-back.

You can replace fictional IP's with an IP from the list of approved IP's, just as with GUIDs

Rick-Anderson avatar Oct 28 '24 17:10 Rick-Anderson

@wadepickett

rick, do you have a link to an example

I don't think Rick Bradley cares See https://github.com/MicrosoftDocs/reusable-content/pull/213/files ROPC is significantly higher risk that IP's

they were intentionally flagging fictional examples from what I could see.

Loop-back is not fictional. IP's are different than GUIDs

Rick-Anderson avatar Oct 28 '24 17:10 Rick-Anderson

The first one is fine but I question why you need to make any changes. The PW is obscured. Admin is used through out the docs.

You 've left an obscured PW here, which is fine because it's obscured.

image

Rick-Anderson avatar Oct 28 '24 17:10 Rick-Anderson

You've left in the text, which can be copy/pasted. You can't do that easily with an image. It's inconsistent.

Specify FTP IP filtering options that allow access from

127.0.0.1

and deny access from the

169.254.0.0/255.255.0.0

range of IP addresses.

<location path="ftp.example.com">
  <system.ftpServer>
    <security>
      <authorization>
        <add accessType="Allow" roles="administrators" permissions="Read, Write" />
      </authorization>
      <requestFiltering>
        <fileExtensions allowUnlisted="true">
          <add fileExtension=".exe" allowed="false" />
          <add fileExtension=".bat" allowed="false" />
          <add fileExtension=".cmd" allowed="false" />
        </fileExtensions>
        <requestLimits maxAllowedContentLength="1000000" maxUrl="1024" />
        <hiddenSegments>
          <add segment="_vti_bin" />
        </hiddenSegments>
      </requestFiltering>
      <ipSecurity enableReverseDns="false" allowUnlisted="true">
        <add ipAddress="127.0.0.1" allowed="true" />
        <add ipAddress="169.254.0.0" subnetMask="255.255.0.0" allowed="false" />

Rick-Anderson avatar Oct 28 '24 18:10 Rick-Anderson

The first one is fine but I question why you need to make any changes. The PW is obscured. Admin is used through out the docs.

I was trying to make the best of the whole awkward situation in each case. The way the dialog was introduced, it felt like the dialog fields could be in a state of not filled out yet. We were asked to address the first few entered fields, user name etc, and it seemed out of state to leave in the obsured passwords at the bottom.

image

I can redo to leave the dialog half filled, with obscured passwords in there. Is that what you want if the other fields entries do need to be removed.

Since the loopback IP needs to stay there in every case, for which I will revert. It might be faster to call out the ones you feel actually need udpates which I will follow for any such cases.

As far as some message at the top of the doc, as you say, what would that be? I can't think of one that is appropriate either. If these are flags that simply should not be flagged I can not that, adjust the PR and update the spreadsheet to note they were reviewed and will not be changed.

wadepickett avatar Oct 28 '24 18:10 wadepickett

We were asked to address the first few entered fields, user name

Asked by whom? the user name Administrator is used throughout the docs. The security is in the PW, not in security by obscurity on the user name.

Rick-Anderson avatar Oct 28 '24 18:10 Rick-Anderson

Maybe we can break this down into some guidelines I can follow here. I am not that familier with the IIS repo.

  • Leave in the loopback address, typically 127.0.0.1.
  • Leave in "Administrator" when it is used as the user name even though flagged.
  • When details like a local path is left in and flagged, keep those anyway.
  • Leave in completely obscured passwords, (replaced with dots) even though they are called out.

any other guidelines you see here from the image list I could add before redoing?

wadepickett avatar Oct 28 '24 18:10 wadepickett

I don't care that image

was changed, I just question the need to change it. The loop back shouldn't be changed

Rick-Anderson avatar Oct 28 '24 18:10 Rick-Anderson

This walkthrough contains a series of steps in which you log in to your FTP site using an IIS Manager account. These steps should only be followed on the server itself using the loopback address or over SSL from a remote server.

IIS Manager is designed to manage websites and apps hosted on the local machine. By default, IIS creates bindings that listen on the loopback address, allowing access to websites on the local machine using http://localhost or http://127.0.0.1.

Rick-Anderson avatar Oct 28 '24 18:10 Rick-Anderson

https://github.com/rick/you-rang/issues/1

Ha ha, be sure to read that @wade

Rick-Anderson avatar Oct 28 '24 19:10 Rick-Anderson

@Rick-Anderson @tdykstra :

Maybe we can break this down into some guidelines I can follow here. I am not that familier with the IIS repo.

  • Leave in the loopback address, typically 127.0.0.1.
  • Leave in "Administrator" when it is used as the user name even though flagged.
  • When details like a local path is left in and flagged, keep those anyway.
  • Leave in completely obscured passwords, (replaced with dots) even though they are called out.

Any other guidelines you see here from the image list I could add before redoing?

wadepickett avatar Oct 28 '24 21:10 wadepickett

they were intentionally flagging fictional examples from what I could see.

My point being that the bar for what they would allow seemed very high, not that the loopback address is fictional, nor that an IP is the same as a GUID.

wadepickett avatar Oct 28 '24 21:10 wadepickett

Any other guidelines you see here from the image list I could add before redoing?

  • Leave in the name of a local database used for the article, like "School" or "Blogging" or "AdventureWorks."
  • Leave in localdb or SQLExpress used as server name. Consider leaving in localdb connection strings.

tdykstra avatar Oct 28 '24 21:10 tdykstra

Any other guidelines you see here from the image list I could add before redoing?

  • Leave in the name of a local database used for the article, like "School" or "Blogging" or "AdventureWorks."
  • Leave in localdb or SQLExpress used as server name. Consider leaving in localdb connection strings.

Thanks! That helps me get a better idea of where the line is compared to what is getting flagged. Much appreciated!

wadepickett avatar Oct 28 '24 21:10 wadepickett

My point being that the bar for what they would allow seemed very high

Where, that's not what I read.

Rick-Anderson avatar Oct 28 '24 22:10 Rick-Anderson

My point being that the bar for what they would allow seemed very high

Where, that's not what I read.

the bar for what they would allow seemed very high given what they were flagging.

wadepickett avatar Oct 28 '24 22:10 wadepickett

the bar for what they would allow seemed very high given what they were flagging.

You don't understand flagging. Flagging means take a look, it's not a directive to delete content.

The bar is very low for flagging.

Rick-Anderson avatar Oct 28 '24 22:10 Rick-Anderson