Use specific misc-include-cleaner statement

There are a few places that which clang-tidy seems reporting
false-positives and which can be suppressed either via using
`modernize-deprecated-headers`

Use specific misc-include-cleaner statement

There are a few places that which clang-tidy seems reporting
false-positives and which can be suppressed either via using
`modernize-deprecated-headers` or more targeted inline
`misc-include-cleaner` statement.

Tested: Compiles

Change-Id: Ib609adbe8619f4b9a84e08388eea1e7cee58aa54
Signed-off-by: Myung Bae <myungbae@us.ibm.com>

show more ...

Fix includes

Clang-tidy misc-include-cleaner appears to now be enforcing
significantly more headers than previously. That is overall a good
thing, but forces us to fix some issues. This commit is

Fix includes

Clang-tidy misc-include-cleaner appears to now be enforcing
significantly more headers than previously. That is overall a good
thing, but forces us to fix some issues. This commit is largely just
taking the clang-recommended fixes and checking them in. Subsequent
patches will fix the more unique issues.

Note, that a number of new ignores are added into the .clang-tidy file.
These can be cleaned up over time as they're understood. The majority
are places where boost includes a impl/x.hpp and x.hpp, but expects you
to use the later. include-cleaner opts for the impl, but it isn't clear
why.

Change-Id: Id3fdd7ee6df6c33b2fd35626898523048dd51bfb
Signed-off-by: Ed Tanous <etanous@nvidia.com>
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

Use SPDX identifiers

SPDX identifiers are simpler, and reduce the amount of cruft we have in
code files. They are recommended by linux foundation, and therefore we
should do as they allow.

This pa

Use SPDX identifiers

SPDX identifiers are simpler, and reduce the amount of cruft we have in
code files. They are recommended by linux foundation, and therefore we
should do as they allow.

This patchset does not intend to modify any intent on any existing
copyrights or licenses, only to standardize their inclusion.

[1] https://www.linuxfoundation.org/blog/blog/copyright-notices-in-open-source-software-projects

Change-Id: I935c7c0156caa78fc368c929cebd0f068031e830
Signed-off-by: Ed Tanous <etanous@nvidia.com>

show more ...

Pam refactoring: To support multiple prompts

The commit refactors pam_authenticated.cpp to support newer prompts
which may come in future for various MFA options. Now the support
restricted to unix

Pam refactoring: To support multiple prompts

The commit refactors pam_authenticated.cpp to support newer prompts
which may come in future for various MFA options. Now the support
restricted to unix Password and google authenticator Verification Code.

Tested by:

1: Successful session creation
2: Successful patch operation for password change using below
curl -k -H "Content-Type: application/json" -H
"X-Auth-Token: $bmc_token" -X
PATCH https://${bmc}/redfish/v1/AccountService/Accounts/root
-d '{"Password":"xxxxxxxx"}'

Change-Id: Iea8696c8a28adefcd5bf62e22978010f38ce8084
Signed-off-by: Abhilash Raju <abhilash.kollam@gmail.com>

show more ...

Move response creation into PasswordData class

No functional changes.

Tested: WIP

Change-Id: Ia306322e08690375f170ae82a82cde2aa8ce850d
Signed-off-by: Ed Tanous <etanous@nvidia.com>

Fix password update

There is a regression issue found with this commit in password update
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/73605
This commit fixes PAM conversation for password update

Fix password update

There is a regression issue found with this commit in password update
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/73605
This commit fixes PAM conversation for password update

Tested by:
PATCH https://${bmc}/redfish/v1/AccountService/Accounts/root -d
'{"Password":"0penBmc1"}'

Change-Id: Ifcede67364c35ced899a3f726f67253cdb51002e
Signed-off-by: Ed Tanous <etanous@nvidia.com>
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>

show more ...

Redfish Session: Implement MFA "Token" property

This commit implements multi-factor authentication "Token" property to
create redfish sessions when multi-factor token authentication enabled.

Tested

Redfish Session: Implement MFA "Token" property

This commit implements multi-factor authentication "Token" property to
create redfish sessions when multi-factor token authentication enabled.

Tested by:

Verified redfish session and login redfish commands with or without
TOTP token for MFA enabled/disabled users.

User authentication with MFA token:
POST https://${bmc}/redfish/v1/SessionService/Sessions -d '{"UserName"
:"root", "Password": "0penBmc","Token":"510760"}'

User authentication without MFA token:
POST https://${bmc}/login -d '{"username" : "newuser", "password"
:"0penBmc"}'

POST https://${bmc}/redfish/v1/SessionService/Sessions -d '{"UserName"
:"newuser", "Password": "0penBmc"}'

In case of invalid MFA token or password then authentication fails and
returns "ResourceAtUriUnauthorized" error message.

Change-Id: I639163dd3d49ff8ed886f72c99ad264317d59c34
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>

show more ...

Fix NOLINT in pam module

There's a number of places in the pam module where we do pointer
manipulation by hand. This is because pam relies on passing pointers.

This commit updates to at least usin

Fix NOLINT in pam module

There's a number of places in the pam module where we do pointer
manipulation by hand. This is because pam relies on passing pointers.

This commit updates to at least using unqiue_ptr with release(), as well
as std::span, rather than using raw pointers.

Tested: Tested in token commit. Will merge at same time.

Change-Id: Ie49f7e6eeaa5c7ac1798b9a123e3ab5439a4ab28
Signed-off-by: Ed Tanous <etanous@nvidia.com>

show more ...

Add missing headers

Most of these were found by breaking every redfish class handler into
its own compile unit:

When that's done, these missing headers become compile errors. We
should just fix the

Add missing headers

Most of these were found by breaking every redfish class handler into
its own compile unit:

When that's done, these missing headers become compile errors. We
should just fix them.

In addition, this allows us to enable automatic header checking in
clang-tidy using misc-header-cleaner. Because the compiler can now
"see" all the defines, it no longer tries to remove headers that it
thinks are unused.

[1] https://github.com/openbmc/bmcweb/commit/4fdee9e39e9f03122ee16a6fb251a380681f56ac

Tested: Code compiles.

Change-Id: Ifa27ac4a512362b7ded7cc3068648dc4aea6ad7b
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Remove unused const-cast

std::string::data now has a non-const variation in c++20. This allows
us to remove a NOLINT and follow the standard.

Tested: Login succeeds.

Change-Id: Ie49932fae8efa90af

Remove unused const-cast

std::string::data now has a non-const variation in c++20. This allows
us to remove a NOLINT and follow the standard.

Tested: Login succeeds.

Change-Id: Ie49932fae8efa90afe1a238f7059924747300521
Signed-off-by: Ed Tanous <edtanous@google.com>

show more ...

Remove extra character

This extra quote snuch into the comment somehow. Fix it.

Change-Id: I5aa14e1f43b1de9cabda006f7f9727d611c5aea3
Signed-off-by: Ed Tanous <edtanous@google.com>

pam-authenticate: fix clang-tidy warning

```
../include/pam_authenticate.hpp:11:75: error: 'msg' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage]
inline int pamFunctionCon

pam-authenticate: fix clang-tidy warning

```
../include/pam_authenticate.hpp:11:75: error: 'msg' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage]
inline int pamFunctionConversation(int numMsg, const struct pam_message** msg,
```

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: Ic0e6d63b01eea78cac54407246363177cb208f8b

show more ...

Pass string views by value

string_view should always be passed by value; This commit is a sed
replace of the code to make all string_views pass by value, per general
coding guidelines[1].

[1] http

Pass string views by value

string_view should always be passed by value; This commit is a sed
replace of the code to make all string_views pass by value, per general
coding guidelines[1].

[1] https://quuxplusone.github.io/blog/2021/11/09/pass-string-view-by-value/

Tested: Code compiles.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I55b342a29a0fbfce0a4ed9ea63db6014d03b134c

show more ...

Enable malloc checks

We only use malloc in one place, when we hand a pointer off to PAM.
Ignore that one issue, and enable the check.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I37c4

Enable malloc checks

We only use malloc in one place, when we hand a pointer off to PAM.
Ignore that one issue, and enable the check.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I37c41c193bae1bab370b03944617c642df0179fc

show more ...

Enable const_cast checks

const_cast is an anti pattern. There are a few places we need to do it
for interacting with C APIs, so enable the checks, and ignore the
existing uses.

Signed-off-by: Ed T

Enable const_cast checks

const_cast is an anti pattern. There are a few places we need to do it
for interacting with C APIs, so enable the checks, and ignore the
existing uses.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If1748213992b97f5e3e04cf9b86a6fcafbb7cf06

show more ...

Enable checks for pointer arithmetic

Quite a few places we've disobeyed this rule, so simply ignore them for
now to avoid new issues popping up.

Signed-off-by: Ed Tanous <edtanous@google.com>
Chang

Enable checks for pointer arithmetic

Quite a few places we've disobeyed this rule, so simply ignore them for
now to avoid new issues popping up.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I3e518a8e8742279afb3ad1a9dad54006ed109fb1

show more ...

Enable reinterpre_cast checks

We seem to use reinterpret cast in a few cases unfortunately. For the
moment, simply ignore most of them, and make it so we don't get more.

Signed-off-by: Ed Tanous <

Enable reinterpre_cast checks

We seem to use reinterpret cast in a few cases unfortunately. For the
moment, simply ignore most of them, and make it so we don't get more.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ic860cf922576b18cdc8d51d6132f5a9cbcc1d9dc

show more ...

Restructure pam conversation function

Altered return values form the function. With the earlier
implementation, the function returned PAM_AUTH_ERR on failure scenarios
which is incorrect. Replaced P

Restructure pam conversation function

Altered return values form the function. With the earlier
implementation, the function returned PAM_AUTH_ERR on failure scenarios
which is incorrect. Replaced PAM_AUTH_ERR with PAM_CONV_ERR and
PAM_BUF_ERR at respetive places.

Added a check for number of messages received by the conversation
function capped at PAM_MAX_NUM_MSG.

Added a check for password size, which is capped at PAM_MAX_RESP_SIZE
as the bytes in the password greater than this limit would be discarded
by PAM.

Though pam_response structure and response, which are dynamically
allocated by the pam conversation function are the responsibility of the
caller to free them, with the current implemention, there is a possibility of
memory leak when numMsg would be zero or if PAM_PROMPT_ECHO_OFF
message never arrived.
This commit fixes the possible memory leak by allocating only on
receiving PAM_PROMPT_ECHO_OFF message.

Tested:
- Basic Authencation is functional.
- POST on /redfish/v1/SessionService/Sessions was succesfull with
the right credentials
- POST on /redfish/v1/AccountService was successfull when the password
was within the limit, and returned a failure when password length
exceeded the limit.

Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com>
Change-Id: Idfa41d94d5a01b62aec119f88cbdaab1523ad936

show more ...

Turn on ALL perf checks

1st, alphabetize the tidy-list for good housekeeping.

Next, enable all the clang-tidy performance checks, and resolve all the
issues. most of the issues boil down to:
1. Us

Turn on ALL perf checks

1st, alphabetize the tidy-list for good housekeeping.

Next, enable all the clang-tidy performance checks, and resolve all the
issues. most of the issues boil down to:
1. Using std::move on const variables. This does nothing.
2. Passing big variables (like std::string) by value.
3. Using double quotes on a find call, which constructs an intermediate
string, rather than using the character overload.

Tested
Loaded on system, logged in successfully and pulled down webui-vue. No
new errors.

Walked the Redfish tree a bit, and observed no new problems.

Ran redfish service validator. Got no new failures (although there are
a lot of log service deprecation warnings that we should look at).

Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I2238958c4b22c1e554e09a0a1787c744bdbca43e

show more ...

Deallocate memory during failed case

Free memory during failed case inside
pamFunctionConversation() function.

Tested:
- Pam authentication works as normal.

Signed-off-by: AppaRao Puli <apparao.p

Deallocate memory during failed case

Free memory during failed case inside
pamFunctionConversation() function.

Tested:
- Pam authentication works as normal.

Signed-off-by: AppaRao Puli <apparao.puli@linux.intel.com>
Change-Id: I81c06a3d674b0806c96e5847cda6f208795bd02c

show more ...

Swap strcpy for strncpy

strncpy has range checks, which reduce the possibility of overrunning
the buffer in the case of a bug.

Tested: clang-tidy cert check now passes. Needs functional testing.

Swap strcpy for strncpy

strncpy has range checks, which reduce the possibility of overrunning
the buffer in the case of a bug.

Tested: clang-tidy cert check now passes. Needs functional testing.

Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I2fab19ca40f97cc0574146883ee19b573285a59c

show more ...

clang-format: update to latest from docs repo

This is from openbmc/docs/style/cpp/.clang-format

Other OpenBMC repos are doing the same.

Tested: Built and validator passed.
Change-Id: Ief26c755c9ce

clang-format: update to latest from docs repo

This is from openbmc/docs/style/cpp/.clang-format

Other OpenBMC repos are doing the same.

Tested: Built and validator passed.
Change-Id: Ief26c755c9ce012823e16a506342b0547a53517a
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

Enhance return value from pamAuthenticateUser

This enhances the return value from the pamAuthenticateUser function so
callers can articulate PAM error codes like PAM_NEW_AUTHTOK_REQD which
means the

Enhance return value from pamAuthenticateUser

This enhances the return value from the pamAuthenticateUser function so
callers can articulate PAM error codes like PAM_NEW_AUTHTOK_REQD which
means the credentials are correct, but the password must be changed.

Tested: Yes, scenarios via both Redfish login and Basic Auth:
- correct username and password, password is not expired
- correct username and password, password is expired
- correct username and incorrect password, password is not expired
- correct username and incorrect password, password is expired
- non-existent user (passsword is not relevant)

Signed-off-by: Joseph Reynolds <joseph-reynolds@charter.net>
Change-Id: I1114d6c9cc591fb0a1853cb4edea32ad22f7b015

show more ...

Change PAM service_name for password changes

BMCWeb was using the "passwd" command PAM configuration settings when changing
the password. This changes it to use PAM service_name="webserver", the sa

Change PAM service_name for password changes

BMCWeb was using the "passwd" command PAM configuration settings when changing
the password. This changes it to use PAM service_name="webserver", the same
name it already uses for PAM authentication and account management functions.

Doing so allows the BMC system administrator to provide separate PAM config
files for the "passwd" command and BMCWeb's password changing function, and
for these activities to be correctly identified in the system logs.

Here is a sample incorrect log message (truncated & wrapped):
bmcweb[353]: pam_warn(passwd:chauthtok): function=[pam_sm_chauthtok]
flags=0xc000 service=[passwd] terminal=[<unknown>] user=[testuser]
ruser=[<unknown>] rhost=[<unknown>]
The "service" should be "webserver".

Tested: yes, password change successful

Change-Id: I593f42c8f0494df967fb498fb4dd66fa4168596b
Signed-off-by: Joseph Reynolds <joseph-reynolds@charter.net>

show more ...

Replace all uses of NULL with nullptr

This was an automatic change made by clang-tidy. It moves all uses of
NULL to nullptr, which are equivalent, but nullptr is prefered.

Tested: Code compiles.

Replace all uses of NULL with nullptr

This was an automatic change made by clang-tidy. It moves all uses of
NULL to nullptr, which are equivalent, but nullptr is prefered.

Tested: Code compiles.

Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I9526599b222693c9723a69934b599c7a5b5d1fbf

show more ...