[pve-devel] [PATCH network v2 1/5] sdn: add global lock for configuration
Stefan Hanreich
s.hanreich at proxmox.com
Tue Jul 29 09:59:35 CEST 2025
On 7/29/25 9:28 AM, Thomas Lamprecht wrote:
[snip]
>> +my $LOCK_SECRET_FILE = "/etc/pve/sdn/.lock";
>> +
>> # improve me : move status code inside plugins ?
>>
>> sub ifquery_check {
>> @@ -197,14 +199,57 @@ sub commit_config {
>> cfs_write_file($running_cfg, $cfg);
>> }
>>
>> +sub generate_lock_secret {
>
> nit: might be better to avoid the "secret" terminology here? As this is not really
> a secret but rather something like a token, handle or maybe even cookie.
>
> I.e., this hasn't to stay secret, as it does not provide access on it's own, it's
> just for ensuring orderly locking by identifying the locker.
>
> I'm mostly mentioning this as such method and variable names tend to leak into
> docs and other communications, and especially secrets are a bit delicate topic,
> for me that's the biggest reason why it would be better to avoid the term here.
>
> Could be fixed up though, if you agree with changing this and have an opinion
> on what variant (handle, token, cookie, ...?) would be best.
Makes sense, I'm gravitating towards token then - although handle would
be fine by me as well. Cookie has the same issues with pre-existing
sentiment / connotations imo?
>> + my $min = ord('!'); # first printable ascii
>> +
>> + my $rand_bytes = Crypt::OpenSSL::Random::random_bytes(32);
>> + die "failed to generate lock secret!\n" if !$rand_bytes;
>> +
>> + my $str = join('', map { chr((ord($_) & 0x3F) + $min) } split('', $rand_bytes));
>
> hmm, might have overlooked when checking the v1, but would it be a better option
> to decode the $rand_bytes as base64? That keeps the full entropy and ensures we
> got an easy to handle character-set.
>
> Another option might be to use a UUIDv7 [0], as that version includes the
> milliseconds since the unix expoch in the first 48 bits, that would also give
> some hints for when the handle was created, that info could be even used for
> expiring a lock handle.
>
> [0]: https://www.rfc-editor.org/rfc/rfc9562.html#name-uuid-version-7
>
> As the users of this should not really expect any specific format, we could still
> change that after applying though, so just tell me what you think/prefer.
Gabriel mentioned something similar about the used characters, because
the current character set is also inconvenient for running CLI commands.
UUIDv7 sounds sensible for this use-case and since we already use the
UUID module in our stack we could just opt for that?
[snip]
More information about the pve-devel
mailing list