opencomputeproject / HWMgmt-OCP-Profiles

A place where all the OCP profiles are a placed. Issues can be submitted/resolved and modifications can be reviewed/merged
Apache License 2.0
16 stars 17 forks source link

HTTP attribute in OCPBaselineHardwareManagement #49

Open plmanik opened 1 year ago

plmanik commented 1 year ago

Hi, In https://github.com/opencomputeproject/HWMgmt-OCP-Profiles/blob/master/HWMgmt/OCPBaselineHardwareManagement.v1_0_2.json under ManagerNetworkProtocol

"HTTP": {
    "PropertyRequirements": {
        "ProtocolEnabled": {},
        "Port": {}
    }
},

HTTP is insecure and if implementation supports only https, how we can avoid this attribute getting error in report. In report we are getting error as fail.HTTP.ReadRequirement: 1

Is it possible to change as recommendation so that it won't expect in response?

Thanks, Mani

plmanik commented 1 year ago

Hi, Do we have any update for changing as recommendation or updating profile by removing HTTP PropertyRequirements?

Thanks, Mani

jautor commented 1 year ago

As Redfish requires HTTPS support, it certainly seems reasonable as a baseline requirement for any managed device (since we're expecting them to support the Redfish protocol anyway)

plmanik commented 1 year ago

@jautor Thanks for reply. We are asking to modify for http protocol ony and keep https protocol. In implementation we will support https only and not http due to security issues, like ssh port can we make http as recommended(Copied profile for reference)

        "ManagerNetworkProtocol": {
            "PropertyRequirements": {
                "HostName": {},
                "Status": {},
                "FQDN": {},
                **"HTTP": {
                    "PropertyRequirements": {
                        "ProtocolEnabled": {},
                        "Port": {}
                    }
                },**
                "HTTPS": {
                    "PropertyRequirements": {
                        "ProtocolEnabled": {},
                        "Port": {}
                    }
                },
                "SSH": {
                    **"ReadRequirement": "Recommended",**
                    "PropertyRequirements": {
                        "ProtocolEnabled": {},
                        "Port": {}
                    }
                }

Thanks, Mani

jcleung5549 commented 1 year ago

During the Profile WS, we agreed this would be a backward-compatibility breaking change and would change the profile revision to be 1.1. Then each platform can prescribe whether their platform needs to conform to v1.0 or v1.1. The server platform can do that within the Server Profile, the other platforms will need to prescribe it in text document.

plmanik commented 1 year ago

Thanks @jcleung5549 . Can you please provide link for v1.1 profile, I'm unable to find for 1.1 for OCPBaselineHardwareManagement ( https://github.com/opencomputeproject/HWMgmt-OCP-Profiles/blob/master/HWMgmt/OCPBaselineHardwareManagement.v1_0_2.json)

Thanks, Mani

jcleung5549 commented 1 year ago

The v1.1 profile will be posted this week. As soon as I resolve my 'git push' issue. Pull Request #50

plmanik commented 1 year ago

Thanks @jcleung5549. Power, thermal resources deprecated as per Redfish schema and using Power subsystem, thermal subsystem. So please consider Power, thermal, Power subsystem, thermal subsystem changes also. For servers using old redfish schema version we need to support for Power, thermal, new redfish schema version can support only Power subsystem, thermal subsystem

jcleung5549 commented 1 year ago

PR #50 has the changes for PowerSubsystem and ThermalSubsystem.

jautor commented 1 year ago

Reading the original issue the point is that the profile requires that the service report HTTP as a protocol. If the service doesn't support HTTP, only HTTPS (which is HTTP, after all), the service should include that information in the resource with a hard-coded "disabled" for that protocol. This allows clients to discover that distinction and making that reporting a mandatory requirement isn't a burden on those services...

For a service that uses HTTPS always, with no HTTP support, they would just include this hard-coded JSON in their ManagerNetworkProtocol payload:

{
   "HTTP": {
      "ProtocolEnabled": false,
      "Port": 80
   }
}
jcleung5549 commented 1 year ago

In baseline v1.0, the requirement is the ProtocolEnabled property be reported for HTTP and HTTPS. To address the security concern, baseline v1.1 would further required the HTTP.ProtocalEnabled have a value of false. Correct?

"ProtocolEnabled": {
    "Values": ["false"],
    "Comparison": "Equal"
}
jcleung5549 commented 1 year ago

10/2 - Add a purpose string to indicate why HTTP is required. ProtocolEnabled can be false and read-only. No other changes in profile needed.

plmanik commented 1 year ago

It may not be appropriate to hard code the values for the protocol not supported. Why don't we change that to recommended instead of mandatory. If we hard code the HTTP, then we need to change schema for protocol enabled as true for readonly, chances for user to patch, we have to consider negative scenario of patching HTTP protocol which is unnecessary from implementation view

jcleung5549 commented 1 year ago

11/7 - Decision to change 'mandatory' to 'if implemented' (can make properties mandatory)

jcleung5549 commented 11 months ago

@plmanik, does the 11/7 decision solve this issue? Need to disposition this issue before the Baseline v1.1 can be contributed?

plmanik commented 11 months ago

@jcleung5549 After adding if implemented, HTTP error is not coming "HTTP": { "ReadRequirement": "IfImplemented", "PropertyRequirements": { "ProtocolEnable": {}, "Port": {} } },

So we can include this change

Thanks, Mani

jcleung5549 commented 10 months ago

Thank you for the response. Included in pull request #50

jcleung5549 commented 8 months ago

Baseline v1.1 merged. Await HW Mgmt Project to release.