Re: [pve-devel] [PATCH network v2 1/5] sdn: add global lock for configuration

[Stefan Hanreich <s.hanreich@proxmox.com>]

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]


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel