| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
| |
$ find * -type f |xargs perl contrib/scripts/spdx.pl
$ git rm contrib/scripts/spdx.pl
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
NMManager and NMSettings both may have multiple authorization requests
ongoing. They need to keep track of them, at the very least to be able
to cancel them on shutdown.
Since NMAuthChain is not ref-countable, it always has only one clear
user/owner. It makes little sense otherwise. Since most callers already
want to track their NMAuthChain instances, let NMAuthChain help with that.
Embed a "parent" CList field inside NMAuthChain. This avoids requiring
an additional GSList allocation to track the element. Also, it allows to
link and append an element without iterating the list.
This ties the caller and the NMAuthChain a bit tighter together (making them
less indepdendent). Generally that is not desirable. But here it seems the
logic (of tracking the NMAuthChain) is still trivial and well separated.
It's just that NMAuthChain instances now can be linked in a CList.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We no longer add these. If you use Emacs, configure it yourself.
Also, due to our "smart-tab" usage the editor anyway does a subpar
job handling our tabs. However, on the upside every user can choose
whatever tab-width he/she prefers. If "smart-tabs" are used properly
(like we do), every tab-width will work.
No manual changes, just ran commands:
F=($(git grep -l -e '-\*-'))
sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}"
sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}"
Check remaining lines with:
git grep -e '-\*-'
The ultimate purpose of this is to cleanup our files and eventually use
SPDX license identifiers. For that, first get rid of the boilerplate lines.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
All callers pass a C string literal as the user-data tag of NMAuthChain.
It makes little sense otherwise because you usually know which user data
you need in advance.
So don't bother with copying the string. Just reference the defacto
static string.
Rename nm_auth_chain_set_data() to nm_auth_chain_set_data_unsafe() to indicate
that the lifetime of the tag string is now the caller's responsibility.
The nm_auth_chain_set_data() macro now ensures that the tag argument is
a C string literal. In fact, all callers that we had did that already.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Out of the 33 callers of nm_auth_chain_add_call(), the permission
argument is:
- 29 times a C string literal like NM_AUTH_PERMISSION_NETWORK_CONTROL.
- 3 times assign a string that is in fact a static string (it's just
not a string literal)
- only NMManager's device_auth_request_cb() passes a permission of
(possibly) non static origin. But it already duplicates the string
for it's own purposes and attaches it as user-data to the
NMAuthChain.
There really is no need to duplicate the string.
Replace nm_auth_chain_add_call() by a macro that ensures that the
permission string is a C literal.
Rename nm_auth_chain_add_call() to nm_auth_chain_add_call_unsafe() to
indicate that the lifetime of the permission argument is now the
responsibility of the caller.
|
| |
|
|
|
|
|
|
|
|
|
| |
We track the user-data in a linked list. Hence, when setting
a user data we would need to search the list whether the
tag already exists.
This has an overhead and makes set-data() O(n). But we really don't need
this. The NMAuthChain allows a simple way to attach user-data to the
request. We don't need a full-blown g_object_set_data() (which btw is
also just implemented as a linked list).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
NMAuthChain tracks arbitrary user-data pointers for convenience of the
caller.
Previously, it would also track the permission request result as such
user-data. That is not optimal.
- for one, we should keep the namespaces for user data (tags) and
permissions separate. When the user requests a permission result with
nm_auth_chain_get_result() then clearly no user-data is requested.
- we already track permissions in a separate linked list. Previously, when the
auth-call completes, we would destroy the entry in the linked list for
requests and create an entry in the linked list for user-data.
Possibly this was done in the past, because the user-data list was
indexed by a hash table. So, in that case, we only would need to
lookup the permission results in the indexed user-data hash, but never
do any search of a linked list. That is no longer the case, because
in practice the number of tracked user-data/permissions is fixed and
small (at which point it's just more efficient to search a short
linked list).
- There is already distrinct API for getting user-data and permissions.
By keeping the lists separate we don't need to search the list for
entries of the wrong type (it's faster).
- also assert that nm_auth_chain_get_result() indeed asks for a result
that was previously requested. It makes no sense otherwise.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
NMAuthChain usually requests several permissions at once. Hence, an error
argument in the overall callback does not make sense, because you
wouldn't know which request failed.
If at all, it could only mean that the overall request failed (like an
D-Bus failure communicating to D-Bus *for all permisssions*),
but we don't need to handle that specially. In fact, we don't really care
why permission was not granted, whether it's due to an error or legitimate
reasons.
The error in the callback was always set to %NULL. Remove it.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
NMAuthChain
The number of user-datas attached to the NMAuthChain is generally fixed
and small.
For example, in current code impl_manager_get_permissions() will be the
instance that ends up with the largest number of data items. It
performs zero calls of nm_auth_chain_set_data() but 16 times
nm_auth_chain_add_call(). So currently the maximum number is 16.
With such a fixed, small number of elements it is expected to be more
efficient to just track the elements in a CList instead of a GHashTable.
|
| |
|
|
|
|
|
|
| |
- consistently name the ChainData variable "chain_data"
- return the ChainData element from _get_data(). This way it
also can be used by nm_auth_chain_steal_data(), which needs
the ChainData element.
|
| |
|
|
|
|
| |
Since the allocated AuthCall struct is small, we can just copy the
permission string inside the struct. No need to strdup() the string
separately.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
NMAuthChain is not ref-counted.
You may call nm_auth_chain_destroy() once before the callback
gets invoked. This destroys the auth-chain instance right away.
You may call nm_auth_chain_destroy() once from inside the callback.
This basically has no effect but is allowed for convenince.
All this does is remembering that destroy was called and asserts that
destroy gets called at most once.
After the callback returns, the auth-chain will always be destroyed.
That means, generally there is no need to call nm_auth_chain_destroy()
from inside the callback.
Remove that code, and refactor some code to return early (where it makes
sense).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
NMAuthChain is not really ref-counted, there is no need for that additional
complexity.
But it is graceful towards calling nm_auth_chain_destroy() from inside the
callback. The caller may do so.
But we don't need a "ref_count" to track that. Two flags suffice: one to
say whether destroy was called and one to indicate that we are in the
process of finishing (to delay deallocating the NMAuthChain struct).
We already had the "done" flag that we used to indicate that the chain
is finished. So, we just need one more flag instead.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
From the files under "shared/nm-utils" we build an internal library
that provides glib-based helper utilities.
Move the files of that basic library to a new subdirectory
"shared/nm-glib-aux" and rename the helper library "libnm-core-base.la"
to "libnm-glib-aux.la".
Reasons:
- the name "utils" is overused in our code-base. Everything's an
"utils". Give this thing a more distinct name.
- there were additional files under "shared/nm-utils", which are not
part of this internal library "libnm-utils-base.la". All the files
that are part of this library should be together in the same
directory, but files that are not, should not be there.
- the new name should better convey what this library is and what is isn't:
it's a set of utilities and helper functions that extend glib with
funcitonality that we commonly need.
There are still some files left under "shared/nm-utils". They have less
a unifying propose to be in their own directory, so I leave them there
for now. But at least they are separate from "shared/nm-glib-aux",
which has a very clear purpose.
(cherry picked from commit 80db06f768e47541eae7d66ef48fbe47bf1a69ce)
|
| |
|
|
|
| |
"nm-macros-interal.h" already includes <errno.h> and <string.h>.
No need to include it everywhere else too.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Remainder of typos found using `codespell -q 3 --skip="./shared,./src/systemd,*.po" -I ../NetworkManager-word-whitelist.txt` whereby whitelist consists of:
```
ans
busses
cace
cna
conexant
crasher
iff
liftime
creat
nd
sav
technik
uint
```
https://github.com/NetworkManager/NetworkManager/pull/205
|
| | |
|
| |
|
|
|
|
|
|
| |
It makes sense to use NMAuthChain also when not attaching any user-data to
the chain. The main reason would be, the ability to schedule multiple permission
checks in parallel, and wait for them to complete together.
Only allocate the hash-table, when we really need it.
|
| |
|
|
|
| |
We already use "data" for other places. Let's use unique names
that can be searched within one file.
|
| |
|
|
|
|
|
| |
For one, we already do <trace> level logging inside NMAuthManager.
So, at trace level we have everything.
If a request fails, it's not up to NMAuthChain to log a warning.
|
| |
|
|
|
|
|
| |
This makes NMAuthCallResult not only usable from within a NMAuthChain.
It makes sense to just call nm-auth-manager directly, but then we need
a way to convert the more detailed result into an NMAuthCallResult
value.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
NMAuthChain's nm_auth_chain_add_call() used to add special handling for
the NMAuthSubject. This handling really belongs to NMAuthManager for two
reasons:
- NMAuthManager already goes through the effort of scheduling an idle
handler to handle the case where no GDBusProxy is present. It can
just as well handle the special cases where polkit-auth is disabled
or when we have internal requests.
- by NMAuthChain doing special handling, it makes it more complicated
to call nm_auth_manager_check_authorization() directly. Previously,
the NMAuthChain had additional logic, which means you either were
forced to create an NMAuthChain, or you had to reimplement special
handling like nm_auth_chain_add_call().
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Supporting PolicyKit required no additional library, just extra code
to handle the D-Bus calls. For that, there was a compile time option
to even stip out that code. Note, that you could (and still can)
configure the system not to use policy-kit. The point was to reduce
the binary size in case you don't need it.
Remove this. I guess, we we aim for such aggressive optimization of
the binary size, we should instead make all device types disablable
at configuration time. We don't do that either and other low hanging
fruits, because it's better to always enable features, unless they
require external dependencies.
Also, the next commit will make more use of NMAuthManager. So, having
it disabled at compile time, makes even less sense.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Don't use the GAsyncResult pattern for internal API of auth-manager. Instead,
use a simpler API that has a more strict API and simpler use.
- return a call-id handle when scheduling the authorization request.
The request is always scheduled asynchronsously and thus call-id
is never %NULL.
- the call-id can be used to cancel the request. It can be used exactly
once, and only before the callback is invoked.
- the async keeps the auth-manager alive. It needs to do so, because
when cancelling the request we might not yet be done: instead we
might still need to issue a CancelCheckAuthorization call (which
we need to handle as well).
- the callback is always invoked exactly once.
Currently NMAuthManager's API effectivly is only called by NMAuthChain.
The point of this is to make NMAuthManager's API more consumable, and
thus let users use it directly (instead of using the NMAuthChain layer).
As well known, we don't do a good job during shutdown of NetworkManager
to release all resources and cancel pending requests. This rework also
makes it possible to actually get this right. See the comment in
nm_auth_manager_force_shutdown(). But yes, it is still a bit complicated
to do a controlled shutdown, because we cannot just synchronously
complete. We need to issue CancelCheckAuthorization D-Bus calls, and
give these requests time to complete. The new API introduced by this patch
would make that easier.
|
| |
|
|
|
|
|
|
|
|
|
| |
NMAuthChain schedules (possibly) multiple authentication requests.
When they all complete, it will once invoke the result-callback.
There is no need to schedule this result-callback on another idle-handler,
because nm_auth_manager_polkit_authority_check_authorization() should guarantee
to invoke the callback never-synchronously and on a clean call-stack (to avoid
problems with re-entrancy). At that point, NMAuthChain does not need to
delay this further.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
NMAuthChain is not really ref-counted. True, we have an internal ref-counter
to ensure that the instance stays alive while the callback is invoked. However,
the user cannot take additional references as there is no nm_auth_chain_ref().
When the user wants to get rid of the auth-chain, with the current API it
is important that the callback won't be called after that point. From the
name nm_auth_chain_unref(), it sounds like that there could be multiple references
to the auth-chain, and merely unreferencing the object might not guarantee that
the callback is canceled. However, that is luckily not the case, because
there is no real ref-counting involved here.
Just rename the destroy function to make this clearer.
|
| | |
|
| |
|
|
|
|
|
| |
- instead of allocating memory separately for the @tag (key)
and ChainData (data), store the tag also inside ChainData.
- instead of adding two separate key and value items to GHashTable,
use g_hash_table_add(), which is optimized for this case.
|
| | |
|
| |
|
|
|
|
|
| |
auth_call_complete() had two callers: once from the idle handler, and
once from pk_call_cb(). The conditions are slightly different, split
the function in two. For one, this allows to unset the obsolete
call_idle_id.
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
| |
Next we will use siphash24() instead of the glib version g_direct_hash() or
g_str_hash(). Hence, the "nm-utils/nm-hash-utils.h" header becomes very
fundamental and will be needed basically everywhere.
Instead of requiring the users to include them, let it be included via
"nm-default.h" header.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Replace the usage of g_str_hash() with our own nm_str_hash().
GLib's g_str_hash() uses djb2 hashing function, just like we
do at the moment. The only difference is, that we use a diffrent
seed value.
Note, that we initialize the hash seed with random data (by calling
getrandom() or reading /dev/urandom). That is a change compared to
before.
This change of the hashing function and accessing the random pool
might be undesired for libnm/libnm-core. Hence, the change is not
done there as it possibly changes behavior for public API. Maybe
we should do that later though.
At this point, there isn't much of a change. This patch becomes
interesting, if we decide to use a different hashing algorithm.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
src/nm-auth-utils.c:343:6: error: 'is_authorized' may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (is_authorized) {
^
src/nm-auth-utils.c:320:11: note: 'is_authorized' was declared here
gboolean is_authorized, is_challenge;
^
src/nm-auth-utils.c:346:13: error: 'is_challenge' may be used uninitialized in this function [-Werror=maybe-uninitialized]
} else if (is_challenge) {
^
src/nm-auth-utils.c:320:26: note: 'is_challenge' was declared here
gboolean is_authorized, is_challenge;
^
(cherry picked from commit 24ab2a4945f8daf53e40bbfb03e647df31337786)
|
| | |
|
| |
|
|
|
|
| |
It could be that the client is just newer and it's just too harsh to
fail the whole request. Leave the unknown permission in unknown and
possibly proceed filling in the rest.
|
| |
|
|
|
|
|
|
|
|
| |
Fix the following build error:
nm-auth-utils.c: In function ‘nm_auth_chain_add_call’:
nm-auth-utils.c:402:46: error: ‘DBUS_GERROR’ undeclared (first use in this function)
call->chain->error = g_error_new_literal (DBUS_GERROR,
Fixes: 1cf35cb26b6cc04f8b2c51c3cde4bc08ef311062
|
| | |
|
| |
|
|
|
|
|
|
|
| |
GError codes are only unique per domain, so logging the code without
also indicating the domain is not helpful. And anyway, if the error
messages are not distinctive enough to tell the whole story then we
should fix the error messages.
Based-on-patch-by: Dan Winship <danw@gnome.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
- All internal source files (except "examples", which are not internal)
should include "config.h" first. As also all internal source
files should include "nm-default.h", let "config.h" be included
by "nm-default.h" and include "nm-default.h" as first in every
source file.
We already wanted to include "nm-default.h" before other headers
because it might contains some fixes (like "nm-glib.h" compatibility)
that is required first.
- After including "nm-default.h", we optinally allow for including the
corresponding header file for the source file at hand. The idea
is to ensure that each header file is self contained.
- Don't include "config.h" or "nm-default.h" in any header file
(except "nm-sd-adapt.h"). Public headers anyway must not include
these headers, and internal headers are never included after
"nm-default.h", as of the first previous point.
- Include all internal headers with quotes instead of angle brackets.
In practice it doesn't matter, because in our public headers we must
include other headers with angle brackets. As we use our public
headers also to compile our interal source files, effectively the
result must be the same. Still do it for consistency.
- Except for <config.h> itself. Include it with angle brackets as suggested by
https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
|
| |
|
|
|
|
|
|
|
|
|
|
| |
- move nm_auth_chain_check_done() and nm_auth_chain_remove_call()
into the only caller auth_call_complete().
- take a ref of the "context" argument.
- in nm_auth_chain_add_call(), assert that we didn't yet invoke the
done-callback. The auth-chain should not be reusued.
- use slice allocator for ChainData, AuthCall and NMAuthChain
|
| |
|
|
|
|
|
|
|
|
| |
Port remaining bits to gdbus and remove stray dbus-glib references
Drop the dbus-glib version check from configure, since nothing depends
on new dbus-glib any more.
Move nm-dbus-glib-types.h and nm-gvaluearray-compat.h from include/ to
libnm-util/ since they are now only used by libnm-util and libnm-glib.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
| |
Move D-Bus export/unexport handling into NMExportedObject and remove
type-specific export/get_path methods (export paths are now specified
at the class level, and NMExportedObject handles the counters for all
exported types automatically).
Since all exportable objects now use the same get_path() method, we
can also add some helper methods to simplify get_property()
implementations for object-path and object-path-array properties.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Rather than randomly including one or more of <glib.h>,
<glib-object.h>, and <gio/gio.h> everywhere (and forgetting to include
"nm-glib-compat.h" most of the time), rename nm-glib-compat.h to
nm-glib.h, include <gio/gio.h> from there, and then change all .c
files in NM to include "nm-glib.h" rather than including the glib
headers directly.
(Public headers files still have to include the real glib headers,
since nm-glib.h isn't installed...)
Also, remove glib includes from header files that are already
including a base object header file (which must itself already include
the glib headers).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
==5177== 6 (+6) bytes in 1 (+1) blocks are definitely lost in loss record 118 of 6,581
==5177== at 0x4C29BCF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==5177== by 0x7F4A6F5: g_malloc (gmem.c:97)
==5177== by 0x7F6301E: g_strdup (gstrfuncs.c:356)
==5177== by 0x4AD902: nm_auth_chain_set_data (nm-auth-utils.c:194)
==5177== by 0x50919E: impl_agent_manager_register_with_capabilities (nm-agent-manager.c:323)
==5177== by 0x62649BE: invoke_object_method (dbus-gobject.c:1899)
==5177== by 0x62649BE: object_registration_message (dbus-gobject.c:2161)
==5177== by 0x649D5CE: _dbus_object_tree_dispatch_and_unlock (dbus-object-tree.c:1018)
==5177== by 0x648F193: dbus_connection_dispatch (dbus-connection.c:4718)
==5177== by 0x6261DB4: message_queue_dispatch (dbus-gmain.c:90)
==5177== by 0x7F44AEA: g_main_dispatch (gmain.c:3111)
==5177== by 0x7F44AEA: g_main_context_dispatch (gmain.c:3710)
==5177== by 0x7F44E87: g_main_context_iterate.isra.29 (gmain.c:3781)
==5177== by 0x7F451B1: g_main_loop_run (gmain.c:3975)
|
