BMCWeb changes for expired password design

[Ed Tanous]

On 8/13/19 1:09 PM, Joseph Reynolds wrote:
> Ed,
> 
> Please review the "expired password" design:
> https://gerrit.openbmc-project.xyz/c/openbmc/docs/+/23849
> 

Richard would likely be a better person to poke, as he wrote the user
management design doc originally.

My main concerns are around the requirements.  I would like to see
mentioned that we:
1. Intend to implement something in compliance with California bill
SB-327 Information privacy: connected devices, and document the relevant
requirements from that bill.
2. We intend to implement the Redfish initial password mechanism as the
spec has defined, to ensure interop with other Redfish devices.

I think the details on the second one seem to be lacking at bit.  I see
a lot of design elements and flows, but I would've expected most of it
to simply be "we intend to implement the Redfish flow for default
passwords" then only provide details where it's missing or optional in
the spec.

If we're building something that's redfish, but unique to OpenBMC , or
counter to the spec, I'm going to have a tough time supporting it, as it
will cause confusion, and somewhat defeat the purpose of having an
industry standard.

> Implementing this requires a few BMCWeb changes:
> - For the `/login` URI: when a correct but expired password is given,
> indicate the password was expired via HTTP response body:
> "Unauthorized.  Password expired.  Use Redfish APIs to change the
> password.", and do not create a session.
> - For Basic Auth (https://user:password@host): when a correct but
> expired password is given, give HTTP response code 403 or similar.
With what content in the response?  This will need to differ for
Redfish, the rest dbus apis, and HTML responses, as they all expect
different error handling.  Simply mentioning that you intend to support
the correct error codes for all 3 would be fine.

> - For Redfish sessions: when a correct password is given, create the
> session as usual, but set the PasswordChangeRequired field (based on
> PAM_NEW_AUTHTOK_REQD).
> - Limit access from sessions which have PasswordChangeRequired=True as
> follows:
>     + The session can only be used to GET its own account and session
> information, PATCH its own account's password, and log out.
>     + Successfully changing the password terminates the session. That
> is, the session does not change from PasswordChangeRequired=True to
> PasswordChangeRequired=False.

I'm interested to see how this will be executed in practice, as we only
have a fixed-at-compile-time privilege to url map.  This would need to
be adjusted, and we would either need to define a new OEM privilege type
(that we may or may not hide) or come up with a way to dynamically
adjust privileges on the fly.

>     + Other uses get HTTP response code 403 (or similar).
> - The existing password changing mechanism would be used, with the
> additional behavior that when ((PasswordChangeRequired=True) and (the
> password was successfully changed)), the session will terminate.
> 
> These changes are based on the design and the Redfish
> PasswordChangeRequired handling specifications (referenced by the
> design).  Would you take a BMCWeb patch to implement this?

I typed the above having looked at an old version of your document.  You
seem to have adjusted it a bit to cover the redfish stuff, which is
excellen.

Yes, I would have no problem with a patchset to do this.  Some important
design points to consider.

1. Please do not recreate the existing privileges module to do the
above.  Your design should be able to fit into what's there.
2. Please do not have a fixed "go/no-go" url list separate from the
other 2 we already have.  Please keep your changes inside the existing
url routing infrastructure by tagging your "ok without auth" routes
appropriately.

I suspect overall, this is going to be difficult, but worthwhile, and
I'd be happy to help review your changes when they're ready.