Age | Commit message (Collapse) | Author | Files | Lines |
|
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>
|
|
This extra quote snuch into the comment somehow. Fix it.
Change-Id: I5aa14e1f43b1de9cabda006f7f9727d611c5aea3
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
```
../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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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>
|
|
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
|
|
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>
|
|
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
|
|
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
|
|
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
|
|
This reverts commit 6ea007a2faec52ad62680015d2a3f00371a1e351.
Reason for revert: Reports of bmcweb seg faults.
Change-Id: I408f1bb29c2f8e427a6621cdaac8c31b847ebf06
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
This patchset moves bmcweb over to the upstream style naming
conventions for variables, classes, and functions, as well as imposes
the latest clang-format file.
This changeset was mostly built automatically by the included
.clang-tidy file, which has the ability to autoformat and auto rename
variables. At some point in the future I would like to see this in
greater use, but for now, we will impose it on bmcweb, and see how it
goes.
Tested: Code still compiles, and appears to run, although other issues
are possible and likely.
Change-Id: If422a2e36df924e897736b3feffa89f411d9dac1
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
|
|
Signed-off-by: Jennifer Lee <jennifer1.lee@intel.com>
Change-Id: I18850e82e116683b1c56e3a0eb23511b09aeed51
|
|
This commit is the beginings of attempting to transition away from
crow, and toward boost::beast. Unit tests are passing, and
implementation appears to be slightly faster than crow.
Change-Id: Ic8d946dc7a04f514c67b1098f181eee1ced69171
|
|
Change-Id: If1e194009cec31c593e4a0f89c875a3e3bd84110
|
|
Upate get_routes to use the correct constness for its use case
crow to set json_mode if json value is populated
Delete std::array bytes API due to major efficiency issues. To be
replaced with span API in near future
Implement a catch block for handlers that can throw exceptions
Implement direct handling of routes that end with / to better support
redfish.
/foo and /foo/ now invoke the same handler insead of issuing a 301
redirect
Update nlohmann to latest version
Implement one nlohmann endpoint with exceptions disabled
Implement first pass at a IBM style rest-dbus interface
Fix pam authentication to call dropbear auth methods
Implements first pass at redfish interface. Shemas avaialble pass
redfish validation 100%
Use response json object rather than request json object.
Update authorization middleware to be redfish compliant
UPdate random token generation to be more efficient, and not base64
bytes, generate bytes directly
Change-Id: I63cc2005c1a21f5c2f5168777a4e09f3c965a34f
|
|
Do not merge yet
Change-Id: I38c56844c1b0e3e8e5493c2705e62e6db7ee2102
|
|
|