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 incor

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

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

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

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.

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 whi

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="webser

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.

Test

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 ...

account_service: redfish user Patch error handling

Modified doPatch method to populate redfish user update error codes.

Tested:
Tested user updates with below scenarios
1)Pr

account_service: redfish user Patch error handling

Modified doPatch method to populate redfish user update error codes.

Tested:
Tested user updates with below scenarios
1)Provided username is not exist
2)Replace username already user exists
3)Replace Username is NULL/Invalid
4)Replace username is not starting with alphabet
5)Replace username exceeds more than 16 characters
6)Password is not valid for Replace/existing username

Redfish validator test results:
1 failProp errors in /redfish/v1/Systems/system/LogServices/EventLog
1 problemResource errors in /redfish/v1/Systems/system/LogServices/
EventLog/Entries
Counter({'skipOptional': 17887, 'pass': 12133, 'passGet': 1285,
'metadataNamespaces': 1047, 'serviceNamespaces': 69, 'reflink': 9,
'passAction': 7, 'warningPresent': 6, 'optionalAction': 6,
'repeat': 3, 'invalidPropertyValue': 3, 'failErrorPresent': 1,
'err.LogEntryCollection.LogEntryCollection': 1, 'failProp': 1,
'unvalidated': 1, 'problemResource': 1,
'unverifiedComplexAdditional': 1, 'warnTrailingSlashLink': 1})
Validation has failed: 3 problems found

Signed-off-by: jayaprakash Mutyala <mutyalax.jayaprakash@intel.com>
Change-Id: Ibee448c5d5c4f38c5c4cacda757864593f6001fc

show more ...

Fix a bunch of warnings

using the list of warnings from here:
https://github.com/lefticus/cppbestpractices/blob/e73393f25a85f83fed7399d8b65cb117d00b2231/02-Use_the_Tools_Available.md#L10

Fix a bunch of warnings

using the list of warnings from here:
https://github.com/lefticus/cppbestpractices/blob/e73393f25a85f83fed7399d8b65cb117d00b2231/02-Use_the_Tools_Available.md#L100

Seems like a good place to start, and would improve things a bit
type-wise. This patchset attempts to correct all the issues in one
shot.

Tested:
It builds. Will test various subsystems that have been touched

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

show more ...

Revert "bmcweb: Fix a bunch of warnings"

This reverts commit 6ea007a2faec52ad62680015d2a3f00371a1e351.

Reason for revert: Reports of bmcweb seg faults.

Change-Id: I408f1bb2

Revert "bmcweb: Fix a bunch of warnings"

This reverts commit 6ea007a2faec52ad62680015d2a3f00371a1e351.

Reason for revert: Reports of bmcweb seg faults.

Change-Id: I408f1bb29c2f8e427a6621cdaac8c31b847ebf06

show more ...

bmcweb: Fix a bunch of warnings

bmcweb classically has not taken a strong opinion on warnings. With
this commit, that policy is changing, and bmcweb will invoke the best
warnings we

bmcweb: Fix a bunch of warnings

bmcweb classically has not taken a strong opinion on warnings. With
this commit, that policy is changing, and bmcweb will invoke the best
warnings we are able to enable, and turn on -Werror for all builds.

This is intended to reduce the likelihood of hard-to-debug situations
that the compiler coulve caught early on.

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

show more ...

bmcweb: /s/boost::string_view/std::string_view/g

With boost 1.69, we get the new option, BOOST_BEAST_USE_STD_STRING_VIEW
which allows us to use std::string for all beast interfaces, inst

bmcweb: /s/boost::string_view/std::string_view/g

With boost 1.69, we get the new option, BOOST_BEAST_USE_STD_STRING_VIEW
which allows us to use std::string for all beast interfaces, instead of
boost string_view. This was originally intended to try to reduce the
binary size, but the comparison shows only a minor improvement.

boost::string_view: 7420780 bytes
std::string_view: 7419948 bytes

832 bytes saved ! ! ! ! !

So instead, we will use the argument that it's more standard and easier
for people to grok.

Tested By:
Pulled down some bmcweb endpoints, and observed no change. Because the
two objects are essentially drop in replacements for one another, there
should be no change.

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

show more ...

Fix some static analysis bugs

A quick scan with infer, a static analysis package.
https://fbinfer.com/docs/getting-started.html

Revealed a couple of legitimate bugs. I'm attach

Fix some static analysis bugs

A quick scan with infer, a static analysis package.
https://fbinfer.com/docs/getting-started.html

Revealed a couple of legitimate bugs. I'm attaching the people on the
blame result to this review so they can look over the change. These are
unlikely to be exploitable in practice, but we should fix them anyway,
to clean up the analysis results.

Tested By:
Code still compiles, changes should be no-op.

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

show more ...

Implement AccountService PATCH method

This patchset implements the AccountService PATCH method, using PAM and
dbus in combination.

Change-Id: I754590f787fc84a21a9453e7e10726c56d

Implement AccountService PATCH method

This patchset implements the AccountService PATCH method, using PAM and
dbus in combination.

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

show more ...

Move to clang-format-6.0

This commit moves the codebase to the lastest clang-format file from
upstream, as well as clang-format-6.0.

Change-Id: Ice8313468097c0c42317fbb9e10ddf03

Move to clang-format-6.0

This commit moves the codebase to the lastest clang-format file from
upstream, as well as clang-format-6.0.

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

show more ...