[PATCH v2 11/13] bpftool: Add support for signing BPF programs

[Quentin Monnet]

2025-08-11 16:23 UTC+0200 ~ KP Singh <kpsingh at kernel.org>
> On Thu, Jul 24, 2025 at 7:07 PM KP Singh <kpsingh at kernel.org> wrote:
>>
>> On Tue, Jul 22, 2025 at 5:51 PM Quentin Monnet <qmo at kernel.org> wrote:
>>>
>>> 2025-07-21 23:19 UTC+0200 ~ KP Singh <kpsingh at kernel.org>

[...]

>>>> @@ -533,6 +547,11 @@ int main(int argc, char **argv)
>>>>       if (argc < 0)
>>>>               usage();
>>>>
>>>> +     if (sign_progs && (private_key_path == NULL || cert_path == NULL)) {
>>>> +             p_err("-i <identity_x509_cert> and -k <private> key must be supplied with -S for signing");
>>>> +             return -EINVAL;
>>>> +     }
>>>
>>>
>>> What if -i and/or -k are passed without -S?
>>
>> We can either print a warning or error out
>>
>> A) User does not want to sign removes --sign and forgets to remove -i
>> -k (better with warning)
>> B) User wants to sign but forgets to --sign (better with error)
>>
>> I'd say we print an error so that we don't accidentally not sign, WDYT?
>>
>> The reason why I think we should keep an explicit --sign is because we
>> can also extend this to have e.g. --verify.
> 
> if (!sign_progs && (private_key_path != NULL || cert_path != NULL)) {
> p_err("-i <identity_x509_cert> and -k <private> also need --sign to be
> used for sign programs");
> return -EINVAL;
> }
> 
> I will error out, I was waiting for Quentin's reply, we can fix it
> later if needed.

Hi KP, I meant to reply to your email but forgot, apologies.

Yes please, it makes sense to me to error out in that case. Let's make
sure that users have the right syntax rather than letting them
accidentally turn off signing.

Thanks for your other comments and clarification too, looks all good to
me :)

Thanks,
Quentin