mbox series

[v11,0/9] bpf: Add kfuncs for PKCS#7 signature verification

Message ID 20220812101902.2846182-1-roberto.sassu@huawei.com (mailing list archive)
Headers show
Series bpf: Add kfuncs for PKCS#7 signature verification | expand

Message

Roberto Sassu Aug. 12, 2022, 10:18 a.m. UTC 
One of the desirable features in security is the ability to restrict import
of data to a given system based on data authenticity. If data import can be
restricted, it would be possible to enforce a system-wide policy based on
the signing keys the system owner trusts.

This feature is widely used in the kernel. For example, if the restriction
is enabled, kernel modules can be plugged in only if they are signed with a
key whose public part is in the primary or secondary keyring.

For eBPF, it can be useful as well. For example, it might be useful to
authenticate data an eBPF program makes security decisions on.

After a discussion in the eBPF mailing list, it was decided that the stated
goal should be accomplished by introducing four new kfuncs:
bpf_lookup_user_key() and bpf_lookup_system_key(), for retrieving a keyring
with keys trusted for signature verification, respectively from its serial
and from a pre-determined ID; bpf_key_put(), to release the reference
obtained with the former two kfuncs, bpf_verify_pkcs7_signature(), for
verifying PKCS#7 signatures.

Other than the key serial, bpf_lookup_user_key() also accepts key lookup
flags, that influence the behavior of the lookup. bpf_lookup_system_key()
accepts pre-determined IDs defined in include/linux/verification.h.

bpf_key_put() accepts the new bpf_key structure, introduced to tell whether
the other structure member, a key pointer, is valid or not. The reason is
that verify_pkcs7_signature() also accepts invalid pointers, set with the
pre-determined ID, to select a system-defined keyring. key_put() must be
called only for valid key pointers.

Since the two key lookup functions allocate memory and one increments a key
reference count, they must be used in conjunction with bpf_key_put(). The
latter must be called only if the lookup functions returned a non-NULL
pointer. The verifier denies the execution of eBPF programs that don't
respect this rule.

The two key lookup functions should be used in alternative, depending on
the use case. While bpf_lookup_user_key() provides great flexibility, it
seems suboptimal in terms of security guarantees, as even if the eBPF
program is assumed to be trusted, the serial used to obtain the key pointer
might come from untrusted user space not choosing one that the system
administrator approves to enforce a mandatory policy.

bpf_lookup_system_key() instead provides much stronger guarantees,
especially if the pre-determined ID is not passed by user space but is
hardcoded in the eBPF program, and that program is signed. In this case,
bpf_verify_pkcs7_signature() will always perform signature verification
with a key that the system administrator approves, i.e. the primary,
secondary or platform keyring.

Nevertheless, key permission checks need to be done accurately. Since
bpf_lookup_user_key() cannot determine how a key will be used by other
kfuncs, it has to defer the permission check to the actual kfunc using the
key. It does it by calling lookup_user_key() with KEY_DEFER_PERM_CHECK as
needed permission. Later, bpf_verify_pkcs7_signature(), if called,
completes the permission check by calling key_validate(). It does not need
to call key_task_permission() with permission KEY_NEED_SEARCH, as it is
already done elsewhere by the key subsystem. Future kfuncs using the
bpf_key structure need to implement the proper checks as well.

Finally, the last kfunc, bpf_verify_pkcs7_signature(), accepts the data and
signature to verify as eBPF dynamic pointers, to minimize the number of
kfunc parameters, and the keyring with keys for signature verification as a
bpf_key structure, returned by one of the two key lookup functions.

bpf_lookup_user_key() and bpf_verify_pkcs7_signature() can be called only
from sleepable programs, because of memory allocation and crypto
operations. For example, the lsm.s/bpf attach point is suitable,
fexit/array_map_update_elem is not.

The correctness of implementation of the new kfuncs and of their usage is
checked with the introduced tests.

The patch set includes a patch from another author (dependency) for sake of
completeness. It is organized as follows.

Patch 1 from KP Singh allows kfuncs to be used by LSM programs. Patch 2
allows dynamic pointers to be used as kfunc parameters. Patch 3 exports
bpf_dynptr_get_size(), to obtain the real size of data carried by a dynamic
pointer. Patch 4 makes available for new eBPF kfuncs some key-related
definitions. Patch 5 introduces the bpf_lookup_*_key() and bpf_key_put()
kfuncs. Patch 6 introduces the bpf_verify_pkcs7_signature() kfunc. Finally,
patches 7-9 introduce the tests.

Changelog

v10:
 - Introduce key_lookup_flags_check() and system_keyring_id_check() inline
   functions to check parameters (suggested by KP)
 - Fix descriptions and comment of key-related kfuncs (suggested by KP)
 - Register kfunc set only once (suggested by Alexei)
 - Move needed kernel options to the architecture-independent configuration
   for testing

v9:
 - Drop patch to introduce KF_SLEEPABLE kfunc flag (already merged)
 - Rename valid_ptr member of bpf_key to has_ref (suggested by Daniel)
 - Check dynamic pointers in kfunc definition with bpf_dynptr_kern struct
   definition instead of string, to detect structure renames (suggested by
   Daniel)
 - Explicitly say that we permit initialized dynamic pointers in kfunc
   definition (suggested by Daniel)
 - Remove noinline __weak from kfuncs definition (reported by Daniel)
 - Simplify key lookup flags check in bpf_lookup_user_key() (suggested by
   Daniel)
 - Explain the reason for deferring key permission check (suggested by
   Daniel)
 - Allocate memory with GFP_ATOMIC in bpf_lookup_system_key(), and remove
   KF_SLEEPABLE kfunc flag from kfunc declaration (suggested by Daniel)
 - Define only one kfunc set and remove the loop for registration
   (suggested by Alexei)

v8:
 - Define the new bpf_key structure to carry the key pointer and whether
   that pointer is valid or not (suggested by Daniel)
 - Drop patch to mark a kfunc parameter with the __maybe_null suffix
 - Improve documentation of kfuncs
 - Introduce bpf_lookup_system_key() to obtain a key pointer suitable for
   verify_pkcs7_signature() (suggested by Daniel)
 - Use the new kfunc registration API
 - Drop patch to test the __maybe_null suffix
 - Add tests for bpf_lookup_system_key()

v7:
 - Add support for using dynamic and NULL pointers in kfunc (suggested by
   Alexei)
 - Add new kfunc-related tests

v6:
 - Switch back to key lookup helpers + signature verification (until v5),
   and defer permission check from bpf_lookup_user_key() to
   bpf_verify_pkcs7_signature()
 - Add additional key lookup test to illustrate the usage of the
   KEY_LOOKUP_CREATE flag and validate the flags (suggested by Daniel)
 - Make description of flags of bpf_lookup_user_key() more user-friendly
   (suggested by Daniel)
 - Fix validation of flags parameter in bpf_lookup_user_key() (reported by
   Daniel)
 - Rename bpf_verify_pkcs7_signature() keyring-related parameters to
   user_keyring and system_keyring to make their purpose more clear
 - Accept keyring-related parameters of bpf_verify_pkcs7_signature() as
   alternatives (suggested by KP)
 - Replace unsigned long type with u64 in helper declaration (suggested by
   Daniel)
 - Extend the bpf_verify_pkcs7_signature() test by calling the helper
   without data, by ensuring that the helper enforces the keyring-related
   parameters as alternatives, by ensuring that the helper rejects
   inaccessible and expired keyrings, and by checking all system keyrings
 - Move bpf_lookup_user_key() and bpf_key_put() usage tests to
   ref_tracking.c (suggested by John)
 - Call bpf_lookup_user_key() and bpf_key_put() only in sleepable programs

v5:
 - Move KEY_LOOKUP_ to include/linux/key.h
   for validation of bpf_verify_pkcs7_signature() parameter
 - Remove bpf_lookup_user_key() and bpf_key_put() helpers, and the
   corresponding tests
 - Replace struct key parameter of bpf_verify_pkcs7_signature() with the
   keyring serial and lookup flags
 - Call lookup_user_key() and key_put() in bpf_verify_pkcs7_signature()
   code, to ensure that the retrieved key is used according to the
   permission requested at lookup time
 - Clarified keyring precedence in the description of
   bpf_verify_pkcs7_signature() (suggested by John)
 - Remove newline in the second argument of ASSERT_
 - Fix helper prototype regular expression in bpf_doc.py

v4:
 - Remove bpf_request_key_by_id(), don't return an invalid pointer that
   other helpers can use
 - Pass the keyring ID (without ULONG_MAX, suggested by Alexei) to
   bpf_verify_pkcs7_signature()
 - Introduce bpf_lookup_user_key() and bpf_key_put() helpers (suggested by
   Alexei)
 - Add lookup_key_norelease test, to ensure that the verifier blocks eBPF
   programs which don't decrement the key reference count
 - Parse raw PKCS#7 signature instead of module-style signature in the
   verify_pkcs7_signature test (suggested by Alexei)
 - Parse kernel module in user space and pass raw PKCS#7 signature to the
   eBPF program for signature verification

v3:
 - Rename bpf_verify_signature() back to bpf_verify_pkcs7_signature() to
   avoid managing different parameters for each signature verification
   function in one helper (suggested by Daniel)
 - Use dynamic pointers and export bpf_dynptr_get_size() (suggested by
   Alexei)
 - Introduce bpf_request_key_by_id() to give more flexibility to the caller
   of bpf_verify_pkcs7_signature() to retrieve the appropriate keyring
   (suggested by Alexei)
 - Fix test by reordering the gcc command line, always compile sign-file
 - Improve helper support check mechanism in the test

v2:
 - Rename bpf_verify_pkcs7_signature() to a more generic
   bpf_verify_signature() and pass the signature type (suggested by KP)
 - Move the helper and prototype declaration under #ifdef so that user
   space can probe for support for the helper (suggested by Daniel)
 - Describe better the keyring types (suggested by Daniel)
 - Include linux/bpf.h instead of vmlinux.h to avoid implicit or
   redeclaration
 - Make the test selfcontained (suggested by Alexei)

v1:
 - Don't define new map flag but introduce simple wrapper of
   verify_pkcs7_signature() (suggested by Alexei and KP)

KP Singh (1):
  bpf: Allow kfuncs to be used in LSM programs

Roberto Sassu (8):
  btf: Handle dynamic pointer parameter in kfuncs
  bpf: Export bpf_dynptr_get_size()
  KEYS: Move KEY_LOOKUP_ to include/linux/key.h
  bpf: Add bpf_lookup_*_key() and bpf_key_put() kfuncs
  bpf: Add bpf_verify_pkcs7_signature() kfunc
  selftests/bpf: Add verifier tests for bpf_lookup_*_key() and
    bpf_key_put()
  selftests/bpf: Add additional tests for bpf_lookup_*_key()
  selftests/bpf: Add test for bpf_verify_pkcs7_signature() kfunc

 include/linux/bpf.h                           |   7 +
 include/linux/bpf_verifier.h                  |   3 +
 include/linux/key.h                           |  11 +
 include/linux/verification.h                  |   8 +
 kernel/bpf/btf.c                              |  23 +
 kernel/bpf/helpers.c                          |   2 +-
 kernel/bpf/verifier.c                         |   4 +-
 kernel/trace/bpf_trace.c                      | 180 ++++++++
 security/keys/internal.h                      |   2 -
 tools/testing/selftests/bpf/Makefile          |  14 +-
 tools/testing/selftests/bpf/config            |   6 +
 tools/testing/selftests/bpf/config.x86_64     |   5 -
 .../selftests/bpf/prog_tests/lookup_key.c     | 112 +++++
 .../bpf/prog_tests/verify_pkcs7_sig.c         | 399 ++++++++++++++++++
 .../selftests/bpf/progs/test_lookup_key.c     |  46 ++
 .../bpf/progs/test_verify_pkcs7_sig.c         | 100 +++++
 tools/testing/selftests/bpf/test_verifier.c   |   3 +-
 .../selftests/bpf/verifier/ref_tracking.c     | 139 ++++++
 .../testing/selftests/bpf/verify_sig_setup.sh | 104 +++++
 19 files changed, 1154 insertions(+), 14 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/lookup_key.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/verify_pkcs7_sig.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_lookup_key.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_verify_pkcs7_sig.c
 create mode 100755 tools/testing/selftests/bpf/verify_sig_setup.sh

Comments

Daniel Borkmann Aug. 15, 2022, 4:10 p.m. UTC | #1 
On 8/12/22 12:18 PM, Roberto Sassu wrote:
> One of the desirable features in security is the ability to restrict import
> of data to a given system based on data authenticity. If data import can be
> restricted, it would be possible to enforce a system-wide policy based on
> the signing keys the system owner trusts.
> 
[...]
> Changelog
> 
> v10:
>   - Introduce key_lookup_flags_check() and system_keyring_id_check() inline
>     functions to check parameters (suggested by KP)
>   - Fix descriptions and comment of key-related kfuncs (suggested by KP)
>   - Register kfunc set only once (suggested by Alexei)
>   - Move needed kernel options to the architecture-independent configuration
>     for testing

Looks like from BPF CI side, the selftest throws a WARN in test_progs / test_progs-no_alu32
and subsequently fails with error, ptal:

   https://github.com/kernel-patches/bpf/runs/7804422038?check_suite_focus=true

   [...]
   #235     verif_scale_xdp_loop:OK
   #236     verif_stats:OK
   #237     verif_twfw:OK
   [  760.448652] ------------[ cut here ]------------
   [  760.449506] WARNING: CPU: 3 PID: 930 at crypto/rsa-pkcs1pad.c:544 pkcs1pad_verify+0x184/0x190
   [  760.450806] Modules linked in: bpf_testmod(OE) [last unloaded: bpf_testmod]
   [  760.452340] CPU: 3 PID: 930 Comm: keyctl Tainted: G           OE      5.19.0-g9f0260338e31-dirty #1
   [  760.453626] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
   [  760.454801] RIP: 0010:pkcs1pad_verify+0x184/0x190
   [  760.455380] Code: 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc 48 89 df 89 c6 5b 41 5c 41 5d 41 5e 41 5f 5d e9 a5 04 00 00 0f 0b b8 ea ff ff ff eb d4 <0f> 0b b8 ea ff ff ff eb cb 0f 0b 90 0f 1f 44 00 00 53 48 89 fb c7
   [  760.456866] RSP: 0018:ffffad55478dbb58 EFLAGS: 00000246
   [  760.457684] RAX: ffff9b3c43c42458 RBX: ffff9b3c48975b00 RCX: 0000000000000000
   [  760.458672] RDX: ffffffffa7277438 RSI: ffffffffa5275510 RDI: 0000000000000000
   [  760.459670] RBP: ffffad55478dbcf8 R08: 0000000000000002 R09: 0000000000000000
   [  760.460688] R10: ffffad55478dbc20 R11: ffffffffa44dde10 R12: ffff9b3c43de2e80
   [  760.461695] R13: ffff9b3c58459ea0 R14: ffff9b3c44d59600 R15: ffffad55478dbc20
   [  760.462270] FS:  00007ff1ee0eb740(0000) GS:ffff9b3cf9cc0000(0000) knlGS:0000000000000000
   [  760.462722] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
   [  760.463026] CR2: 000055b9a4c17588 CR3: 0000000107bb2000 CR4: 00000000000006e0
   [  760.464039] Call Trace:
   [  760.464465]  <TASK>
   [  760.464749]  public_key_verify_signature+0x4a2/0x570
   [  760.465623]  x509_check_for_self_signed+0x4e/0xd0
   [  760.465937]  x509_cert_parse+0x193/0x220
   [  760.466656]  x509_key_preparse+0x20/0x1f0
   [  760.466975]  asymmetric_key_preparse+0x43/0x80
   [  760.467552]  key_create_or_update+0x24e/0x510
   [  760.468366]  __x64_sys_add_key+0x19b/0x220
   [  760.468704]  ? syscall_enter_from_user_mode+0x24/0x1f0
   [  760.469056]  do_syscall_64+0x43/0x90
   [  760.469657]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
   [  760.470413] RIP: 0033:0x7ff1edf0ba9d
   [  760.470832] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d cb e2 0e 00 f7 d8 64 89 01 48
   [  760.472742] RSP: 002b:00007ffe635e7a18 EFLAGS: 00000246 ORIG_RAX: 00000000000000f8
   [  760.473355] RAX: ffffffffffffffda RBX: 00007ffe635e7be0 RCX: 00007ff1edf0ba9d
   [  760.474523] RDX: 000055982fed80c0 RSI: 00007ffe635e7f17 RDI: 00007ffe635e7f0c
   [  760.475500] RBP: 00007ffe635e7a38 R08: 00000000fffffffd R09: 0000000000000000
   [  760.475913] R10: 0000000000000355 R11: 0000000000000246 R12: 0000000000000000
   [  760.476594] R13: 00007ffe635e7bd8 R14: 000055982fed48ae R15: 000055982fed76e8
   [  760.477579]  </TASK>
   [  760.477769] irq event stamp: 4727
   [  760.477963] hardirqs last  enabled at (4735): [<ffffffffa4101df5>] __up_console_sem+0x75/0xa0
   [  760.479036] hardirqs last disabled at (4744): [<ffffffffa4a31cca>] sysvec_apic_timer_interrupt+0xa/0xb0
   [  760.480403] softirqs last  enabled at (4762): [<ffffffffa4085172>] __irq_exit_rcu+0xb2/0x140
   [  760.480869] softirqs last disabled at (4755): [<ffffffffa4085172>] __irq_exit_rcu+0xb2/0x140
   [  760.481706] ---[ end trace 0000000000000000 ]---
   Generating a RSA private key
   .+++++
   ..................................................+++++
   writing new private key to '/tmp/verify_sigXdOL5V/signing_key.pem'
   -----
   add_key: Invalid argument
   test_verify_pkcs7_sig:PASS:mkdtemp 0 nsec
   test_verify_pkcs7_sig:FAIL:_run_setup_process unexpected error: 1 (errno 126)
   #238     verify_pkcs7_sig:FAIL
   #239     vmlinux:OK
   #240     xdp:OK
   #241/1   xdp_adjust_frags/xdp_adjust_frags:OK
   #241     xdp_adjust_frags:OK
   #242/1   xdp_adjust_tail/xdp_adjust_tail_shrink:OK
   #242/2   xdp_adjust_tail/xdp_adjust_tail_grow:OK
   [...]
Roberto Sassu Aug. 16, 2022, 7:12 a.m. UTC | #2 
> From: Daniel Borkmann [mailto:daniel@iogearbox.net]
> Sent: Monday, August 15, 2022 6:10 PM
> On 8/12/22 12:18 PM, Roberto Sassu wrote:
> > One of the desirable features in security is the ability to restrict import
> > of data to a given system based on data authenticity. If data import can be
> > restricted, it would be possible to enforce a system-wide policy based on
> > the signing keys the system owner trusts.
> >
> [...]
> > Changelog
> >
> > v10:
> >   - Introduce key_lookup_flags_check() and system_keyring_id_check() inline
> >     functions to check parameters (suggested by KP)
> >   - Fix descriptions and comment of key-related kfuncs (suggested by KP)
> >   - Register kfunc set only once (suggested by Alexei)
> >   - Move needed kernel options to the architecture-independent configuration
> >     for testing
> 
> Looks like from BPF CI side, the selftest throws a WARN in test_progs /
> test_progs-no_alu32
> and subsequently fails with error, ptal:
> 
>    https://github.com/kernel-
> patches/bpf/runs/7804422038?check_suite_focus=true

Hi Daniel

it is due to the missing SHA256 kernel module (not copied to
the virtual machine).

I made a small patch in libbpf/ci to change kernel options =m
into =y. With that patch, my instance of vmtest gives success
(except for z15, which requires adding openssl and keyctl
to the virtual machine image).

Roberto

>    [...]
>    #235     verif_scale_xdp_loop:OK
>    #236     verif_stats:OK
>    #237     verif_twfw:OK
>    [  760.448652] ------------[ cut here ]------------
>    [  760.449506] WARNING: CPU: 3 PID: 930 at crypto/rsa-pkcs1pad.c:544
> pkcs1pad_verify+0x184/0x190
>    [  760.450806] Modules linked in: bpf_testmod(OE) [last unloaded:
> bpf_testmod]
>    [  760.452340] CPU: 3 PID: 930 Comm: keyctl Tainted: G           OE      5.19.0-
> g9f0260338e31-dirty #1
>    [  760.453626] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.13.0-1ubuntu1.1 04/01/2014
>    [  760.454801] RIP: 0010:pkcs1pad_verify+0x184/0x190
>    [  760.455380] Code: 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc 48 89 df 89 c6 5b 41
> 5c 41 5d 41 5e 41 5f 5d e9 a5 04 00 00 0f 0b b8 ea ff ff ff eb d4 <0f> 0b b8 ea ff
> ff ff eb cb 0f 0b 90 0f 1f 44 00 00 53 48 89 fb c7
>    [  760.456866] RSP: 0018:ffffad55478dbb58 EFLAGS: 00000246
>    [  760.457684] RAX: ffff9b3c43c42458 RBX: ffff9b3c48975b00 RCX:
> 0000000000000000
>    [  760.458672] RDX: ffffffffa7277438 RSI: ffffffffa5275510 RDI:
> 0000000000000000
>    [  760.459670] RBP: ffffad55478dbcf8 R08: 0000000000000002 R09:
> 0000000000000000
>    [  760.460688] R10: ffffad55478dbc20 R11: ffffffffa44dde10 R12:
> ffff9b3c43de2e80
>    [  760.461695] R13: ffff9b3c58459ea0 R14: ffff9b3c44d59600 R15:
> ffffad55478dbc20
>    [  760.462270] FS:  00007ff1ee0eb740(0000) GS:ffff9b3cf9cc0000(0000)
> knlGS:0000000000000000
>    [  760.462722] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>    [  760.463026] CR2: 000055b9a4c17588 CR3: 0000000107bb2000 CR4:
> 00000000000006e0
>    [  760.464039] Call Trace:
>    [  760.464465]  <TASK>
>    [  760.464749]  public_key_verify_signature+0x4a2/0x570
>    [  760.465623]  x509_check_for_self_signed+0x4e/0xd0
>    [  760.465937]  x509_cert_parse+0x193/0x220
>    [  760.466656]  x509_key_preparse+0x20/0x1f0
>    [  760.466975]  asymmetric_key_preparse+0x43/0x80
>    [  760.467552]  key_create_or_update+0x24e/0x510
>    [  760.468366]  __x64_sys_add_key+0x19b/0x220
>    [  760.468704]  ? syscall_enter_from_user_mode+0x24/0x1f0
>    [  760.469056]  do_syscall_64+0x43/0x90
>    [  760.469657]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
>    [  760.470413] RIP: 0033:0x7ff1edf0ba9d
>    [  760.470832] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8
> 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> ff 73 01 c3 48 8b 0d cb e2 0e 00 f7 d8 64 89 01 48
>    [  760.472742] RSP: 002b:00007ffe635e7a18 EFLAGS: 00000246 ORIG_RAX:
> 00000000000000f8
>    [  760.473355] RAX: ffffffffffffffda RBX: 00007ffe635e7be0 RCX:
> 00007ff1edf0ba9d
>    [  760.474523] RDX: 000055982fed80c0 RSI: 00007ffe635e7f17 RDI:
> 00007ffe635e7f0c
>    [  760.475500] RBP: 00007ffe635e7a38 R08: 00000000fffffffd R09:
> 0000000000000000
>    [  760.475913] R10: 0000000000000355 R11: 0000000000000246 R12:
> 0000000000000000
>    [  760.476594] R13: 00007ffe635e7bd8 R14: 000055982fed48ae R15:
> 000055982fed76e8
>    [  760.477579]  </TASK>
>    [  760.477769] irq event stamp: 4727
>    [  760.477963] hardirqs last  enabled at (4735): [<ffffffffa4101df5>]
> __up_console_sem+0x75/0xa0
>    [  760.479036] hardirqs last disabled at (4744): [<ffffffffa4a31cca>]
> sysvec_apic_timer_interrupt+0xa/0xb0
>    [  760.480403] softirqs last  enabled at (4762): [<ffffffffa4085172>]
> __irq_exit_rcu+0xb2/0x140
>    [  760.480869] softirqs last disabled at (4755): [<ffffffffa4085172>]
> __irq_exit_rcu+0xb2/0x140
>    [  760.481706] ---[ end trace 0000000000000000 ]---
>    Generating a RSA private key
>    .+++++
>    ..................................................+++++
>    writing new private key to '/tmp/verify_sigXdOL5V/signing_key.pem'
>    -----
>    add_key: Invalid argument
>    test_verify_pkcs7_sig:PASS:mkdtemp 0 nsec
>    test_verify_pkcs7_sig:FAIL:_run_setup_process unexpected error: 1 (errno
> 126)
>    #238     verify_pkcs7_sig:FAIL
>    #239     vmlinux:OK
>    #240     xdp:OK
>    #241/1   xdp_adjust_frags/xdp_adjust_frags:OK
>    #241     xdp_adjust_frags:OK
>    #242/1   xdp_adjust_tail/xdp_adjust_tail_shrink:OK
>    #242/2   xdp_adjust_tail/xdp_adjust_tail_grow:OK
>    [...]
Daniel Borkmann Aug. 16, 2022, 10:05 a.m. UTC | #3 
On 8/16/22 9:12 AM, Roberto Sassu wrote:
>> From: Daniel Borkmann [mailto:daniel@iogearbox.net]
>> Sent: Monday, August 15, 2022 6:10 PM
>> On 8/12/22 12:18 PM, Roberto Sassu wrote:
>>> One of the desirable features in security is the ability to restrict import
>>> of data to a given system based on data authenticity. If data import can be
>>> restricted, it would be possible to enforce a system-wide policy based on
>>> the signing keys the system owner trusts.
>>>
>> [...]
>>> Changelog
>>>
>>> v10:
>>>    - Introduce key_lookup_flags_check() and system_keyring_id_check() inline
>>>      functions to check parameters (suggested by KP)
>>>    - Fix descriptions and comment of key-related kfuncs (suggested by KP)
>>>    - Register kfunc set only once (suggested by Alexei)
>>>    - Move needed kernel options to the architecture-independent configuration
>>>      for testing
>>
>> Looks like from BPF CI side, the selftest throws a WARN in test_progs /
>> test_progs-no_alu32
>> and subsequently fails with error, ptal:
>>
>>     https://github.com/kernel-
>> patches/bpf/runs/7804422038?check_suite_focus=true
> 
> it is due to the missing SHA256 kernel module (not copied to
> the virtual machine).
> 
> I made a small patch in libbpf/ci to change kernel options =m
> into =y. With that patch, my instance of vmtest gives success
> (except for z15, which requires adding openssl and keyctl
> to the virtual machine image).

The code in pkcs1pad_verify() triggering the warning is:

     [...]
         if (WARN_ON(req->dst) || WARN_ON(!digest_size) ||
             !ctx->key_size || sig_size != ctx->key_size)
                 return -EINVAL;
     [...]

It is not obvious at all to users that sha256 module is missing in their kernel,
how will they be able to figure it out?

Should the helper be gated if dependency is not available, or return a -EOPNOTSUPP
if the specific request cannot be satisfied (but others can..)?

>>     [...]
>>     #235     verif_scale_xdp_loop:OK
>>     #236     verif_stats:OK
>>     #237     verif_twfw:OK
>>     [  760.448652] ------------[ cut here ]------------
>>     [  760.449506] WARNING: CPU: 3 PID: 930 at crypto/rsa-pkcs1pad.c:544
>> pkcs1pad_verify+0x184/0x190
>>     [  760.450806] Modules linked in: bpf_testmod(OE) [last unloaded:
>> bpf_testmod]
>>     [  760.452340] CPU: 3 PID: 930 Comm: keyctl Tainted: G           OE      5.19.0-
>> g9f0260338e31-dirty #1
>>     [  760.453626] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
>> 1.13.0-1ubuntu1.1 04/01/2014
>>     [  760.454801] RIP: 0010:pkcs1pad_verify+0x184/0x190
>>     [  760.455380] Code: 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc 48 89 df 89 c6 5b 41
>> 5c 41 5d 41 5e 41 5f 5d e9 a5 04 00 00 0f 0b b8 ea ff ff ff eb d4 <0f> 0b b8 ea ff
>> ff ff eb cb 0f 0b 90 0f 1f 44 00 00 53 48 89 fb c7
>>     [  760.456866] RSP: 0018:ffffad55478dbb58 EFLAGS: 00000246
>>     [  760.457684] RAX: ffff9b3c43c42458 RBX: ffff9b3c48975b00 RCX:
>> 0000000000000000
>>     [  760.458672] RDX: ffffffffa7277438 RSI: ffffffffa5275510 RDI:
>> 0000000000000000
>>     [  760.459670] RBP: ffffad55478dbcf8 R08: 0000000000000002 R09:
>> 0000000000000000
>>     [  760.460688] R10: ffffad55478dbc20 R11: ffffffffa44dde10 R12:
>> ffff9b3c43de2e80
>>     [  760.461695] R13: ffff9b3c58459ea0 R14: ffff9b3c44d59600 R15:
>> ffffad55478dbc20
>>     [  760.462270] FS:  00007ff1ee0eb740(0000) GS:ffff9b3cf9cc0000(0000)
>> knlGS:0000000000000000
>>     [  760.462722] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>     [  760.463026] CR2: 000055b9a4c17588 CR3: 0000000107bb2000 CR4:
>> 00000000000006e0
>>     [  760.464039] Call Trace:
>>     [  760.464465]  <TASK>
>>     [  760.464749]  public_key_verify_signature+0x4a2/0x570
>>     [  760.465623]  x509_check_for_self_signed+0x4e/0xd0
>>     [  760.465937]  x509_cert_parse+0x193/0x220
>>     [  760.466656]  x509_key_preparse+0x20/0x1f0
>>     [  760.466975]  asymmetric_key_preparse+0x43/0x80
>>     [  760.467552]  key_create_or_update+0x24e/0x510
>>     [  760.468366]  __x64_sys_add_key+0x19b/0x220
>>     [  760.468704]  ? syscall_enter_from_user_mode+0x24/0x1f0
>>     [  760.469056]  do_syscall_64+0x43/0x90
>>     [  760.469657]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
>>     [  760.470413] RIP: 0033:0x7ff1edf0ba9d
>>     [  760.470832] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8
>> 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
>> ff 73 01 c3 48 8b 0d cb e2 0e 00 f7 d8 64 89 01 48
>>     [  760.472742] RSP: 002b:00007ffe635e7a18 EFLAGS: 00000246 ORIG_RAX:
>> 00000000000000f8
>>     [  760.473355] RAX: ffffffffffffffda RBX: 00007ffe635e7be0 RCX:
>> 00007ff1edf0ba9d
>>     [  760.474523] RDX: 000055982fed80c0 RSI: 00007ffe635e7f17 RDI:
>> 00007ffe635e7f0c
>>     [  760.475500] RBP: 00007ffe635e7a38 R08: 00000000fffffffd R09:
>> 0000000000000000
>>     [  760.475913] R10: 0000000000000355 R11: 0000000000000246 R12:
>> 0000000000000000
>>     [  760.476594] R13: 00007ffe635e7bd8 R14: 000055982fed48ae R15:
>> 000055982fed76e8
>>     [  760.477579]  </TASK>
>>     [  760.477769] irq event stamp: 4727
>>     [  760.477963] hardirqs last  enabled at (4735): [<ffffffffa4101df5>]
>> __up_console_sem+0x75/0xa0
>>     [  760.479036] hardirqs last disabled at (4744): [<ffffffffa4a31cca>]
>> sysvec_apic_timer_interrupt+0xa/0xb0
>>     [  760.480403] softirqs last  enabled at (4762): [<ffffffffa4085172>]
>> __irq_exit_rcu+0xb2/0x140
>>     [  760.480869] softirqs last disabled at (4755): [<ffffffffa4085172>]
>> __irq_exit_rcu+0xb2/0x140
>>     [  760.481706] ---[ end trace 0000000000000000 ]---
>>     Generating a RSA private key
>>     .+++++
>>     ..................................................+++++
>>     writing new private key to '/tmp/verify_sigXdOL5V/signing_key.pem'
>>     -----
>>     add_key: Invalid argument
>>     test_verify_pkcs7_sig:PASS:mkdtemp 0 nsec
>>     test_verify_pkcs7_sig:FAIL:_run_setup_process unexpected error: 1 (errno
>> 126)
>>     #238     verify_pkcs7_sig:FAIL
>>     #239     vmlinux:OK
>>     #240     xdp:OK
>>     #241/1   xdp_adjust_frags/xdp_adjust_frags:OK
>>     #241     xdp_adjust_frags:OK
>>     #242/1   xdp_adjust_tail/xdp_adjust_tail_shrink:OK
>>     #242/2   xdp_adjust_tail/xdp_adjust_tail_grow:OK
>>     [...]
Roberto Sassu Aug. 16, 2022, 12:15 p.m. UTC | #4 
> From: Daniel Borkmann [mailto:daniel@iogearbox.net]
> Sent: Tuesday, August 16, 2022 12:05 PM
> On 8/16/22 9:12 AM, Roberto Sassu wrote:
> >> From: Daniel Borkmann [mailto:daniel@iogearbox.net]
> >> Sent: Monday, August 15, 2022 6:10 PM
> >> On 8/12/22 12:18 PM, Roberto Sassu wrote:
> >>> One of the desirable features in security is the ability to restrict import
> >>> of data to a given system based on data authenticity. If data import can be
> >>> restricted, it would be possible to enforce a system-wide policy based on
> >>> the signing keys the system owner trusts.
> >>>
> >> [...]
> >>> Changelog
> >>>
> >>> v10:
> >>>    - Introduce key_lookup_flags_check() and system_keyring_id_check()
> inline
> >>>      functions to check parameters (suggested by KP)
> >>>    - Fix descriptions and comment of key-related kfuncs (suggested by KP)
> >>>    - Register kfunc set only once (suggested by Alexei)
> >>>    - Move needed kernel options to the architecture-independent
> configuration
> >>>      for testing
> >>
> >> Looks like from BPF CI side, the selftest throws a WARN in test_progs /
> >> test_progs-no_alu32
> >> and subsequently fails with error, ptal:
> >>
> >>     https://github.com/kernel-
> >> patches/bpf/runs/7804422038?check_suite_focus=true
> >
> > it is due to the missing SHA256 kernel module (not copied to
> > the virtual machine).
> >
> > I made a small patch in libbpf/ci to change kernel options =m
> > into =y. With that patch, my instance of vmtest gives success
> > (except for z15, which requires adding openssl and keyctl
> > to the virtual machine image).
> 
> The code in pkcs1pad_verify() triggering the warning is:
> 
>      [...]
>          if (WARN_ON(req->dst) || WARN_ON(!digest_size) ||
>              !ctx->key_size || sig_size != ctx->key_size)
>                  return -EINVAL;
>      [...]
> 
> It is not obvious at all to users that sha256 module is missing in their kernel,
> how will they be able to figure it out?

Yes, it was not trivial to find.

> Should the helper be gated if dependency is not available, or return a -
> EOPNOTSUPP
> if the specific request cannot be satisfied (but others can..)?

Uhm, the failure is not related to the kfuncs I introduced. The add_key()
system call failed.

Also, it seems not easy to determine if dependencies are satisfied.
If SHA512 is supported, that might be sufficient. It depends on how
the certificate is generated.

What is your opinion on the solution, change all config options
to =y, or use sed like I did?

Thanks

Roberto

> >>     [...]
> >>     #235     verif_scale_xdp_loop:OK
> >>     #236     verif_stats:OK
> >>     #237     verif_twfw:OK
> >>     [  760.448652] ------------[ cut here ]------------
> >>     [  760.449506] WARNING: CPU: 3 PID: 930 at crypto/rsa-pkcs1pad.c:544
> >> pkcs1pad_verify+0x184/0x190
> >>     [  760.450806] Modules linked in: bpf_testmod(OE) [last unloaded:
> >> bpf_testmod]
> >>     [  760.452340] CPU: 3 PID: 930 Comm: keyctl Tainted: G           OE      5.19.0-
> >> g9f0260338e31-dirty #1
> >>     [  760.453626] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS
> >> 1.13.0-1ubuntu1.1 04/01/2014
> >>     [  760.454801] RIP: 0010:pkcs1pad_verify+0x184/0x190
> >>     [  760.455380] Code: 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc 48 89 df 89 c6
> 5b 41
> >> 5c 41 5d 41 5e 41 5f 5d e9 a5 04 00 00 0f 0b b8 ea ff ff ff eb d4 <0f> 0b b8 ea
> ff
> >> ff ff eb cb 0f 0b 90 0f 1f 44 00 00 53 48 89 fb c7
> >>     [  760.456866] RSP: 0018:ffffad55478dbb58 EFLAGS: 00000246
> >>     [  760.457684] RAX: ffff9b3c43c42458 RBX: ffff9b3c48975b00 RCX:
> >> 0000000000000000
> >>     [  760.458672] RDX: ffffffffa7277438 RSI: ffffffffa5275510 RDI:
> >> 0000000000000000
> >>     [  760.459670] RBP: ffffad55478dbcf8 R08: 0000000000000002 R09:
> >> 0000000000000000
> >>     [  760.460688] R10: ffffad55478dbc20 R11: ffffffffa44dde10 R12:
> >> ffff9b3c43de2e80
> >>     [  760.461695] R13: ffff9b3c58459ea0 R14: ffff9b3c44d59600 R15:
> >> ffffad55478dbc20
> >>     [  760.462270] FS:  00007ff1ee0eb740(0000) GS:ffff9b3cf9cc0000(0000)
> >> knlGS:0000000000000000
> >>     [  760.462722] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >>     [  760.463026] CR2: 000055b9a4c17588 CR3: 0000000107bb2000 CR4:
> >> 00000000000006e0
> >>     [  760.464039] Call Trace:
> >>     [  760.464465]  <TASK>
> >>     [  760.464749]  public_key_verify_signature+0x4a2/0x570
> >>     [  760.465623]  x509_check_for_self_signed+0x4e/0xd0
> >>     [  760.465937]  x509_cert_parse+0x193/0x220
> >>     [  760.466656]  x509_key_preparse+0x20/0x1f0
> >>     [  760.466975]  asymmetric_key_preparse+0x43/0x80
> >>     [  760.467552]  key_create_or_update+0x24e/0x510
> >>     [  760.468366]  __x64_sys_add_key+0x19b/0x220
> >>     [  760.468704]  ? syscall_enter_from_user_mode+0x24/0x1f0
> >>     [  760.469056]  do_syscall_64+0x43/0x90
> >>     [  760.469657]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
> >>     [  760.470413] RIP: 0033:0x7ff1edf0ba9d
> >>     [  760.470832] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48
> 89 f8
> >> 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01
> f0 ff
> >> ff 73 01 c3 48 8b 0d cb e2 0e 00 f7 d8 64 89 01 48
> >>     [  760.472742] RSP: 002b:00007ffe635e7a18 EFLAGS: 00000246 ORIG_RAX:
> >> 00000000000000f8
> >>     [  760.473355] RAX: ffffffffffffffda RBX: 00007ffe635e7be0 RCX:
> >> 00007ff1edf0ba9d
> >>     [  760.474523] RDX: 000055982fed80c0 RSI: 00007ffe635e7f17 RDI:
> >> 00007ffe635e7f0c
> >>     [  760.475500] RBP: 00007ffe635e7a38 R08: 00000000fffffffd R09:
> >> 0000000000000000
> >>     [  760.475913] R10: 0000000000000355 R11: 0000000000000246 R12:
> >> 0000000000000000
> >>     [  760.476594] R13: 00007ffe635e7bd8 R14: 000055982fed48ae R15:
> >> 000055982fed76e8
> >>     [  760.477579]  </TASK>
> >>     [  760.477769] irq event stamp: 4727
> >>     [  760.477963] hardirqs last  enabled at (4735): [<ffffffffa4101df5>]
> >> __up_console_sem+0x75/0xa0
> >>     [  760.479036] hardirqs last disabled at (4744): [<ffffffffa4a31cca>]
> >> sysvec_apic_timer_interrupt+0xa/0xb0
> >>     [  760.480403] softirqs last  enabled at (4762): [<ffffffffa4085172>]
> >> __irq_exit_rcu+0xb2/0x140
> >>     [  760.480869] softirqs last disabled at (4755): [<ffffffffa4085172>]
> >> __irq_exit_rcu+0xb2/0x140
> >>     [  760.481706] ---[ end trace 0000000000000000 ]---
> >>     Generating a RSA private key
> >>     .+++++
> >>     ..................................................+++++
> >>     writing new private key to '/tmp/verify_sigXdOL5V/signing_key.pem'
> >>     -----
> >>     add_key: Invalid argument
> >>     test_verify_pkcs7_sig:PASS:mkdtemp 0 nsec
> >>     test_verify_pkcs7_sig:FAIL:_run_setup_process unexpected error: 1 (errno
> >> 126)
> >>     #238     verify_pkcs7_sig:FAIL
> >>     #239     vmlinux:OK
> >>     #240     xdp:OK
> >>     #241/1   xdp_adjust_frags/xdp_adjust_frags:OK
> >>     #241     xdp_adjust_frags:OK
> >>     #242/1   xdp_adjust_tail/xdp_adjust_tail_shrink:OK
> >>     #242/2   xdp_adjust_tail/xdp_adjust_tail_grow:OK
> >>     [...]
Alexei Starovoitov Aug. 16, 2022, 4:39 p.m. UTC | #5 
Roberto,

please use a different email provider,
since gmail classifies all of your emails as spam.
I've seen other huawei folks use huaweicloud.com.