Bug 2342978 - Review Request: linux-sgx - Intel Linux SGX SDK and Platform Software
Summary: Review Request: linux-sgx - Intel Linux SGX SDK and Platform Software
Keywords:
Status: RELEASE_PENDING
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Richard W.M. Jones
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/intel/linux-sgx
Whiteboard:
Depends On: 2344173 2311762 2338150
Blocks:
TreeView+ depends on / blocked
 
Reported: 2025-01-30 15:01 UTC by Daniel Berrangé
Modified: 2025-02-14 15:54 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:
rjones: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 8587790 to 8625878 (7.92 KB, patch)
2025-02-07 14:09 UTC, Fedora Review Service
no flags Details | Diff
Full review with rpmlint output (95.79 KB, text/plain)
2025-02-14 11:24 UTC, Richard W.M. Jones
no flags Details
The .spec file difference from Copr build 8625878 to 8654414 (7.22 KB, patch)
2025-02-14 12:26 UTC, Fedora Review Service
no flags Details | Diff

Description Daniel Berrangé 2025-01-30 15:01:58 UTC
Spec URL: https://berrange.fedorapeople.org/review/linux-sgx/linux-sgx.spec
SRPM URL: https://berrange.fedorapeople.org/review/linux-sgx/linux-sgx-2.25-1.fc41.src.rpm
Description: The Intel SGX SDK is a collection of APIs, libraries, documentations and tools that allow software developers to create and debug Intel SGX enabled applications in C/C++.
Fedora Account System Username: berrange

Comment 1 Fedora Review Service 2025-01-30 18:01:56 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8587790
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2342978-linux-sgx/fedora-rawhide-x86_64/08587790-linux-sgx/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 2 Richard W.M. Jones 2025-02-06 06:57:26 UTC
Well this is complicated ...

You can add repack as a source line if you want.

What's the reason for doing this?
%{?_smp_mflags} -j1

Build currently fails because of missing dependencies, but I think we're adding the
deps to Rawhide now / soon so I'll try to do a build when those are there.

Comment 3 Richard W.M. Jones 2025-02-06 08:17:24 UTC
My scratch build failed: https://koji.fedoraproject.org/koji/taskinfo?taskID=128901722

Comment 4 Daniel Berrangé 2025-02-06 09:05:31 UTC
Oh fun, we've been broken by GCC switching to the gnu23  C standard

In file included from crypto/pcl_sha256.c:94:
./crypto/pcl_crypto_internal.h:41:22: error: ‘bool’ cannot be defined via ‘typedef’
   41 | typedef unsigned int bool;
      |                      ^~~~
./crypto/pcl_crypto_internal.h:41:22: note: ‘bool’ is a keyword with ‘-std=c23’ onwards

Comment 5 Daniel Berrangé 2025-02-06 14:50:13 UTC
Fixing the 'bool' compat problem was easy, but that just uncovered a further GCC 15 compat issue that is impossible to workaround.

SGX was relying on a (questionably supported) GCC C++ extension, and GCC 15 is now rejecting this.

https://bugzilla.redhat.com/show_bug.cgi?id=2344173

I'm blocked waiting to see what GCC decide to do about this before we can progress SGX. If GCC decides to keep the rejection then we're in a hard place, as I'm unsure how to change the code to avoid the now rejected behaviour.

Comment 6 Daniel Berrangé 2025-02-07 12:31:47 UTC
Refreshed at:

Spec URL: https://berrange.fedorapeople.org/review/linux-sgx/linux-sgx.spec
SRPM URL: https://berrange.fedorapeople.org/review/linux-sgx/linux-sgx-2.25-1.fc41.src.rpm


Note, while I solved one GCC15 compat problem, this still won't build in rawhide, pending bug 2344173, but figure it can be reviewed while we wait for that GCC fix. 

> You can add repack as a source line if you want.

Added that

> What's the reason for doing this?
> %{?_smp_mflags} -j1

The intention was that we honour %{?_smp_mflags} always. The build system has bugs in places which force me to (temporarily) add -j1 to serialize it.

Since the use of -j1 was temporary, I left %{?_smp_mflags} so we could just drop the '-j1' bit and return to parallel builds at a later date.

A few of the -j1 bits are obsolete so I've removed them, and added comments in the other cases that these are temporary hacks

Comment 7 Fedora Review Service 2025-02-07 14:09:23 UTC
Created attachment 2075464 [details]
The .spec file difference from Copr build 8587790 to 8625878

Comment 8 Fedora Review Service 2025-02-07 14:09:25 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8625878
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2342978-linux-sgx/fedora-rawhide-x86_64/08625878-linux-sgx/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 9 Daniel Berrangé 2025-02-14 10:05:06 UTC
Refreshed again:

Spec URL: https://berrange.fedorapeople.org/review/linux-sgx/linux-sgx.spec
SRPM URL: https://berrange.fedorapeople.org/review/linux-sgx/linux-sgx-2.25-1.fc43.src.rpm

Includes a temporary hack to cull the code that tickles the GCC 15 bug, so it now builds in rawhide.

Comment 10 Richard W.M. Jones 2025-02-14 11:23:36 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- The License field must be a valid SPDX expression.
  Note: Not a valid SPDX expression 'Apache-2.0 AND BSD-2-Clause AND
  BSD-3-Clause AND BSD-4-Clause AND BSD-4-Clause-UC AND GPL-2.0-only AND
  ISC AND MIT AND MIT-0 AND NCSA AND OpenSSL AND SMLNJ AND SunPro AND
  LicenseRef-Public-Domain'.
  See: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1

This seems to be a problem.

- Static libraries in -static or -devel subpackage, providing -devel if
  present.
  Note: Package has .a files: sgx-enclave-devel. Does not provide -static:
  sgx-enclave-devel.
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/#packaging-static-libraries

The .a files in -devel are a bit special.  They provide the "SGX
enclave runtime".  When making an enclave you have to link with them,
and they include everything needed (even the C runtime?).  So they're
not really Fedora static libraries in the ordinary sense.

- systemd_post is invoked in %post, systemd_preun in %preun, and
  systemd_postun in %postun for Systemd service files.
  Note: Systemd service file(s) in sgx-aesm, sgx-mpa, tdx-qgs
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/Scriptlets/#_scriptlets

Maybe a problem?


===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.

These are part of the SGX SDK, so not really used in the normal
sense of *.so files.

[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Package contains no static executables.
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated", "BSD 2-Clause License and/or BSD
     3-Clause License and/or Eclipse Public License 1.0", "BSD 3-Clause
     License", "Eclipse Public License 1.0", "BSD 2-Clause License and/or
     BSD 2-clause NetBSD License", "*No copyright* Apache License 2.0",
     "Apache License 2.0", "FSF Unlimited License [generated file]", "BSD
     3-Clause License and/or OpenSSL License", "University of Illinois/NCSA
     Open Source License", "*No copyright* MIT License", "ISC License",
     "MIT License", "BSD 3-Clause License and/or MIT License", "BSD
     2-Clause License", "BSD 2-clause FreeBSD License", "BSD 3-Clause
     License and/or GNU General Public License, Version 2", "Standard ML of
     New Jersey License", "MIT No Attribution", "BSD 4-Clause License",
     "*No copyright* Public domain", "BSD 3-Clause License and/or BSD
     4-Clause License", "OpenSSL License", "BSD 3-Clause License and/or GNU
     General Public License", "BSD-4-Clause (University of California-
     Specific)", "BSD 3-Clause License and/or Microsoft Public License",
     "*No copyright* BSD 3-Clause License", "FSF All Permissive License",
     "*No copyright* Eclipse Public License 1.0", "*No copyright* ISC
     License", "BSD 3-Clause License and/or Public domain", "GNU General
     Public License, Version 2", "Apache License 2.0 and/or BSD 3-Clause
     License", "BSD 2-Clause License and/or BSD 3-Clause License", "Apache
     License 2.0 and/or BSD 2-Clause License", "SSLeay", "Apache License
     1.0 and/or OpenSSL License", "zlib License", "ISC License and/or MIT
     License", "BSD 2-Clause License and/or BSD-4-Clause (University of
     California-Specific)", "BSD 2-Clause License and/or BSD 2-clause
     FreeBSD License", "*No copyright* Creative Commons CC0 1.0". 2501
     files have unknown license. Detailed output of licensecheck in
     /var/tmp/2342978-linux-sgx/licensecheck.txt

The license analysis in the spec looks fine.

[x]: License file installed when any subpackage combination is installed.

Yes, since the other packages require sgx-common, and sgx-common
includes the licenses.

[x]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[!]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/share/aesmd, /usr/lib64/aesmd,
     /usr/lib64/aesmd/bundles

Seems like an issue.

[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib/udev,
     /usr/x86_64-intel-sgx/lib64, /usr/lib/.build-id/5b,
     /usr/lib64/aesmd/bundles, /usr/share/aesmd, /usr/x86_64-intel-sgx,
     /usr/lib/.build-id/47, /usr/lib/udev/rules.d, /usr/lib64/aesmd

That .build-id directory also seems wrong.  Note this is from a
mock-built package on my machine.

[ ]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/x86_64-intel-
     sgx/lib64(sgx-enclave-prebuilt-common)

Unclear.  As far as I'm aware it's OK for multiple RPMs to own a
directory.

[x]: %build honors applicable compiler flags or justifies otherwise.

It doesn't use the compiler flags, but justifies this in the spec.

[x]: Package contains no bundled libraries without FPC exception.

We believe that current policy allows bundling with an exception,
provided that the Provides: bundled() dependencies are added, which
they are here.

[x]: Changelog in prescribed format.

Uses autochangelog.

[x]: Sources contain only permissible code or content.

Eventually after a lot of repacking, yes.

[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
     Note: Package contains Conflicts: tag(s) needing fix or justification.

I'm not sure what it's complaining about here.  There's a
BuildConflicts line, but it is justified.

[ ]: Package obeys FHS, except libexecdir and /usr/target.

(See paths problems above)

[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[x]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.

It has justified ExclusiveArch.

[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: %config files are marked noreplace or the reason is justified.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 4346 bytes in 1 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).

Complicated, but seems correct.

[ ]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in sgx-
     common , sgx-enclave-latest-pce-unsigned , sgx-enclave-latest-ide-
     unsigned , sgx-enclave-latest-qe3-unsigned , sgx-enclave-latest-tdqe-
     unsigned , sgx-enclave-devel , sgx-devel , sgx-libs , sgx-aesm , sgx-
     pccs-admin , sgx-pckid-tool , sgx-mpa , tdx-qgs , tdx-attest-libs ,
     tdx-attest-devel

Unclear.

[-]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[x]: Scriptlets must be sane, if used.
[x]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments

This is fine, as tarball is repacked.

[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[-]: %check is present and all tests pass.

No tests.

[-]: Packages should try to preserve timestamps of original installed
     files.
[-]: Files in /run, var/run and /var/lock uses tmpfiles.d when appropriate
[ ]: Spec use %global instead of %define unless justified.
     Note: %define requiring justification: %define linux_sgx_version 2.25,
     %define dcap_version 1.22, %define dcap_qvl_version 1.21, %define
     dcap_qvs_version 1.1.0-2885, %define sgx_ssl_version 3.0_Rev4, %define
     ipp_crypto_version 2021.12.1, %define sgx_emm_version 1.0.3, %define
     openssl_version 3.0.14, %define libcbor_version 0.10.2, %define
     abseil_cpp_version 20230125.3, %define jwt_cpp_version 0.6.0, %define
     wamr_version 1.3.3, %define epid_version 6.0.0, %define rdrand_version
     1.1, %define vtune_version 2018, %define enclave_pce_version 2.25,
     %define enclave_ide_version 1.22, %define enclave_qe3_version 1.22,
     %define enclave_tdqe_version 1.22, %define enclave_qve_version 1.22,
     %define with_enclaves 1, %define with_enclave_pce 1, %define
     with_enclave_ide 1, %define with_enclave_qe3 1, %define
     with_enclave_tdqe 1, %define with_enclave_qve 0, %define
     _with_enclave_pce %{expr:%{with_enclaves} ? %{with_enclave_pce} : 0},
     %define _with_enclave_ide %{expr:%{with_enclaves} ?
     %{with_enclave_ide} : 0}, %define _with_enclave_qe3
     %{expr:%{with_enclaves} ? %{with_enclave_qe3} : 0}, %define
     _with_enclave_tdqe %{expr:%{with_enclaves} ? %{with_enclave_tdqe} :
     0}, %define _with_enclave_qve %{expr:%{with_enclaves} ?
     %{with_enclave_qve} : 0}, %define vroot build/vroot

I believe this requirement is wrong, %define is fine with modern RPM.

[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Package should compile and build into binary rpms on all supported
     architectures.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: There are rpmlint messages (see attachment).
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Package should not use obsolete m4 macros
[x]: Spec file according to URL is the same as in SRPM.

(will attach rpmlint output separately as it's too large for a bugzilla comment)

Comment 11 Richard W.M. Jones 2025-02-14 11:24:16 UTC
Created attachment 2076455 [details]
Full review with rpmlint output

Comment 12 Fedora Review Service 2025-02-14 12:26:25 UTC
Created attachment 2076458 [details]
The .spec file difference from Copr build 8625878 to 8654414

Comment 13 Fedora Review Service 2025-02-14 12:26:28 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8654414
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2342978-linux-sgx/fedora-rawhide-x86_64/08654414-linux-sgx/fedora-review/review.txt

Found issues:

- Not a valid SPDX expression 'Apache-2.0 AND BSD-2-Clause AND BSD-3-Clause AND BSD-4-Clause AND BSD-4-Clause-UC AND GPL-2.0-only AND ISC AND MIT AND MIT-0 AND NCSA AND OpenSSL AND SMLNJ AND SunPro AND LicenseRef-Public-Domain'.
  Read more: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1
- Package has .a files: sgx-enclave-devel. Does not provide -static: sgx-enclave-devel.
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries
- Systemd service file(s) in sgx-aesm, sgx-mpa, tdx-qgs
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 14 Daniel Berrangé 2025-02-14 14:15:44 UTC
(In reply to Richard W.M. Jones from comment #10)
> Issues:
> =======
> - The License field must be a valid SPDX expression.
>   Note: Not a valid SPDX expression 'Apache-2.0 AND BSD-2-Clause AND
>   BSD-3-Clause AND BSD-4-Clause AND BSD-4-Clause-UC AND GPL-2.0-only AND
>   ISC AND MIT AND MIT-0 AND NCSA AND OpenSSL AND SMLNJ AND SunPro AND
>   LicenseRef-Public-Domain'.
>   See: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1
> 
> This seems to be a problem.

Yes, same as my other package, it should be LicenseRef-Fedora-Public-Domain

> - systemd_post is invoked in %post, systemd_preun in %preun, and
>   systemd_postun in %postun for Systemd service files.
>   Note: Systemd service file(s) in sgx-aesm, sgx-mpa, tdx-qgs
>   See: https://docs.fedoraproject.org/en-US/packaging-
>   guidelines/Scriptlets/#_scriptlets
> 
> Maybe a problem?

I had scripts for aesm, but forgot scripts for mpa/qgs, so have
added those now.

I also added scripts for sysusers, but conditionalized for RHEL only given Fedora 42 relies on RPM magic now.


> [!]: Package requires other packages for directories it uses.
>      Note: No known owner of /usr/share/aesmd, /usr/lib64/aesmd,
>      /usr/lib64/aesmd/bundles
>
> Seems like an issue.

Those dirs are now owned by sgx-aesm.


> 
> [!]: Package must own all directories that it creates.
>      Note: Directories without known owners: /usr/lib/udev,
>      /usr/x86_64-intel-sgx/lib64, /usr/lib/.build-id/5b,
>      /usr/lib64/aesmd/bundles, /usr/share/aesmd, /usr/x86_64-intel-sgx,
>      /usr/lib/.build-id/47, /usr/lib/udev/rules.d, /usr/lib64/aesmd
> 
> That .build-id directory also seems wrong.  Note this is from a
> mock-built package on my machine.

I've added ownership of those dirs except the wierd build-id ones.

> [ ]: Package does not own files or directories owned by other packages.
>      Note: Dirs in package are owned also by: /usr/x86_64-intel-
>      sgx/lib64(sgx-enclave-prebuilt-common)
> 
> Unclear.  As far as I'm aware it's OK for multiple RPMs to own a
> directory.

Yep, multiple packages can own a dir provided they agree on permissions, which is trivial if relying no default permissions

> [x]: Package does not generate any conflict.
>      Note: Package contains Conflicts: tag(s) needing fix or justification.
> 
> I'm not sure what it's complaining about here.  There's a
> BuildConflicts line, but it is justified.

Yeah, I think it is the BuildConflicts it is reporting, which is ok.

> 
> [ ]: Fully versioned dependency in subpackages if applicable.
>      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in sgx-
>      common , sgx-enclave-latest-pce-unsigned , sgx-enclave-latest-ide-
>      unsigned , sgx-enclave-latest-qe3-unsigned , sgx-enclave-latest-tdqe-
>      unsigned , sgx-enclave-devel , sgx-devel , sgx-libs , sgx-aesm , sgx-
>      pccs-admin , sgx-pckid-tool , sgx-mpa , tdx-qgs , tdx-attest-libs ,
>      tdx-attest-devel
> 
> Unclear.

There is no "%{name}" package emitted at all, so it is a false positive to add such a dep

Several of the subpackages do have deps on the -libs sub-RPM, which fully set the
version + release, but omit %{_?isa} which is fine since this is ExclusiveArch to a single target

> [ ]: Spec use %global instead of %define unless justified.
>      Note: %define requiring justification: %define linux_sgx_version 2.25,
>      %define dcap_version 1.22, %define dcap_qvl_version 1.21, %define
>      dcap_qvs_version 1.1.0-2885, %define sgx_ssl_version 3.0_Rev4, %define
>      ipp_crypto_version 2021.12.1, %define sgx_emm_version 1.0.3, %define
>      openssl_version 3.0.14, %define libcbor_version 0.10.2, %define
>      abseil_cpp_version 20230125.3, %define jwt_cpp_version 0.6.0, %define
>      wamr_version 1.3.3, %define epid_version 6.0.0, %define rdrand_version
>      1.1, %define vtune_version 2018, %define enclave_pce_version 2.25,
>      %define enclave_ide_version 1.22, %define enclave_qe3_version 1.22,
>      %define enclave_tdqe_version 1.22, %define enclave_qve_version 1.22,
>      %define with_enclaves 1, %define with_enclave_pce 1, %define
>      with_enclave_ide 1, %define with_enclave_qe3 1, %define
>      with_enclave_tdqe 1, %define with_enclave_qve 0, %define
>      _with_enclave_pce %{expr:%{with_enclaves} ? %{with_enclave_pce} : 0},
>      %define _with_enclave_ide %{expr:%{with_enclaves} ?
>      %{with_enclave_ide} : 0}, %define _with_enclave_qe3
>      %{expr:%{with_enclaves} ? %{with_enclave_qe3} : 0}, %define
>      _with_enclave_tdqe %{expr:%{with_enclaves} ? %{with_enclave_tdqe} :
>      0}, %define _with_enclave_qve %{expr:%{with_enclaves} ?
>      %{with_enclave_qve} : 0}, %define vroot build/vroot
> 
> I believe this requirement is wrong, %define is fine with modern RPM.

I switched to %global regardless, just to be consistent within the spec as I had mixed both

Comment 15 Daniel Berrangé 2025-02-14 14:49:40 UTC
  sgx-enclave-latest-ide-unsigned.x86_64: E: statically-linked-binary /usr/x86_64-intel-sgx/lib64/libsgx_id_enclave.so
  sgx-enclave-latest-pce-unsigned.x86_64: E: statically-linked-binary /usr/x86_64-intel-sgx/lib64/libsgx_pce.so
  sgx-enclave-latest-qe3-unsigned.x86_64: E: statically-linked-binary /usr/x86_64-intel-sgx/lib64/libsgx_qe3.so
  sgx-enclave-latest-tdqe-unsigned.x86_64: E: statically-linked-binary /usr/x86_64-intel-sgx/lib64/libsgx_tdqe.so

False positive. these are the static SGX enclaves, masquerading as shared object due to intel's wierd file ext choice.


  sgx-enclave-devel.x86_64: E: static-library-without-debuginfo /usr/x86_64-intel-sgx/lib64/libsgx_capable.a
  sgx-enclave-devel.x86_64: E: static-library-without-debuginfo /usr/x86_64-intel-sgx/lib64/libsgx_dcap_tvl.a
  ..snip..
  sgx-enclave-devel.x86_64: E: static-library-without-debuginfo /usr/x86_64-intel-sgx/lib64/libsgx_utls.a
  sgx-enclave-devel.x86_64: E: static-library-without-debuginfo /usr/x86_64-intel-sgx/lib64/libtdx_tls.a

False positive. These are all static libs that provide the SGX enclave runtime library. Including debug symbols is not relevant, as you cannot attach a debugger to an SGX enclave.


  sgx-enclave-latest-ide-unsigned.x86_64: E: spelling-error ('toolchain', 'Summary(en_US) toolchain -> tool chain, tool-chain, blockchain')
  sgx-enclave-latest-ide-unsigned.x86_64: E: spelling-error ('toolchain', '%description -l en_US toolchain -> tool chain, tool-chain, blockchain')
  ...snip...
  sgx-enclave-latest-tdqe-unsigned.x86_64: E: spelling-error ('toolchain', 'Summary(en_US) toolchain -> tool chain, tool-chain, blockchain')
  sgx-enclave-latest-tdqe-unsigned.x86_64: E: spelling-error ('toolchain', '%description -l en_US toolchain -> tool chain, tool-chain, blockchain')

Will change to 'tool-chain'


  linux-sgx.spec:511: W: setup-not-quiet

Will add -q


  sgx-aesm.x86_64: W: position-independent-executable-suggested /usr/lib64/aesmd/aesm_service
  sgx-mpa.x86_64: W: position-independent-executable-suggested /usr/bin/mpa_manage
  sgx-mpa.x86_64: W: position-independent-executable-suggested /usr/bin/mpa_registration
  sgx-pckid-tool.x86_64: W: position-independent-executable-suggested /usr/bin/PCKIDRetrievalTool
  tdx-qgs.x86_64: W: position-independent-executable-suggested /usr/bin/qgs

Valid complaint. These should be built as PIE binaries, but the SGX build system is horrendous so thus far I've not been able to solve this, and don't propose fixing it for review. Will leave it on my TODO list though, to feed back to upstream.


  sgx-aesm.x86_64: W: non-standard-uid /run/aesmd aesmd
  sgx-aesm.x86_64: W: non-standard-uid /var/lib/aesmd aesmd
  tdx-qgs.x86_64: W: non-standard-uid /run/tdx-qgs qgs
  tdx-qgs.x86_64: W: non-standard-uid /var/lib/qgs qgs
  sgx-aesm.x86_64: W: non-standard-gid /run/aesmd aesmd
  sgx-aesm.x86_64: W: non-standard-gid /var/lib/aesmd aesmd
  tdx-qgs.x86_64: W: non-standard-gid /run/tdx-qgs qgs
  tdx-qgs.x86_64: W: non-standard-gid /var/lib/qgs qgs

False positive, these user accounts are created by the sysusers files


  sgx-aesm.x86_64: E: non-standard-dir-perm /run/aesmd 700
  tdx-qgs.x86_64: E: non-standard-dir-perm /run/tdx-qgs 700

False positive, and IMHO bug in rpmlint that it only accepts 755 and calls it an error, not warning, as there are plenty of reasons to want other permissions.


  sgx-enclave-devel.x86_64: W: no-soname /usr/lib64/libsgx_epid_sim.so
  sgx-enclave-devel.x86_64: W: no-soname /usr/lib64/libsgx_launch_sim.so
  sgx-enclave-devel.x86_64: W: no-soname /usr/lib64/libsgx_ptrace.so
  sgx-enclave-devel.x86_64: W: no-soname /usr/lib64/libsgx_quote_ex_sim.so
  sgx-enclave-devel.x86_64: W: no-soname /usr/lib64/libsgx_uae_service_sim.so
  sgx-enclave-devel.x86_64: E: invalid-soname /usr/lib64/libsgx_capable.so libsgx_capable.so
  sgx-enclave-devel.x86_64: E: invalid-soname /usr/lib64/libsgx_urts_sim.so libsgx_urts_sim.so

Sigh yes, but not something we should unilaterally fix downstream. Another item to take to upstream


  sgx-aesm.x86_64: W: no-manual-page-for-binary aesmd
  sgx-enclave-devel.x86_64: W: no-manual-page-for-binary sgx-gdb
  sgx-enclave-devel.x86_64: W: no-manual-page-for-binary sgx_config_cpusvn
  sgx-enclave-devel.x86_64: W: no-manual-page-for-binary sgx_edger8r
  sgx-enclave-devel.x86_64: W: no-manual-page-for-binary sgx_encrypt
  sgx-enclave-devel.x86_64: W: no-manual-page-for-binary sgx_sign
  sgx-mpa.x86_64: W: no-manual-page-for-binary mpa_manage
  sgx-mpa.x86_64: W: no-manual-page-for-binary mpa_registration
  sgx-pccs-admin.x86_64: W: no-manual-page-for-binary pccsadmin
  sgx-pckid-tool.x86_64: W: no-manual-page-for-binary PCKIDRetrievalTool
  tdx-qgs.x86_64: W: no-manual-page-for-binary qgs

Valid, but not to be fixed. Upstream provides docs in PDFs (sic)


  sgx-enclave-latest-ide-unsigned.x86_64: E: no-ldconfig-symlink /usr/x86_64-intel-sgx/lib64/libsgx_id_enclave.so
  sgx-enclave-latest-pce-unsigned.x86_64: E: no-ldconfig-symlink /usr/x86_64-intel-sgx/lib64/libsgx_pce.so
  sgx-enclave-latest-qe3-unsigned.x86_64: E: no-ldconfig-symlink /usr/x86_64-intel-sgx/lib64/libsgx_qe3.so
  sgx-enclave-latest-tdqe-unsigned.x86_64: E: no-ldconfig-symlink /usr/x86_64-intel-sgx/lib64/libsgx_tdqe.so

False positive, again these are SGX enclaves not normal shared libraries, despite the file ext

sgx-aesm.x86_64: W: no-documentation
sgx-common.x86_64: W: no-documentation
sgx-devel.x86_64: W: no-documentation
sgx-enclave-devel.x86_64: W: no-documentation
sgx-enclave-latest-ide-unsigned.x86_64: W: no-documentation
sgx-enclave-latest-pce-unsigned.x86_64: W: no-documentation
sgx-enclave-latest-qe3-unsigned.x86_64: W: no-documentation
sgx-enclave-latest-tdqe-unsigned.x86_64: W: no-documentation
sgx-mpa.x86_64: W: no-documentation
sgx-pccs-admin.x86_64: W: no-documentation
tdx-attest-devel.x86_64: W: no-documentation
tdx-qgs.x86_64: W: no-documentation

  sgx-common.x86_64: E: no-binary
  sgx-pccs-admin.x86_64: E: no-binary

False positive, since the package is ExclusiveArch x86_64, there's no point making these noarch.


  linux-sgx.spec: W: no-%check-section

No practical tests to run


  linux-sgx.spec:203: W: macro-in-comment %{dcap_version}
  linux-sgx.spec:203: W: macro-in-comment %{dcap_version}
  linux-sgx.spec:1044: W: macro-in-comment %{sgx_includedir}
  linux-sgx.spec:1046: W: macro-in-comment %{_includedir}

False positive, harmless & intentional.


  sgx-enclave-devel.x86_64: E: lto-no-text-in-archive /usr/x86_64-intel-sgx/lib64/libsgx_pcl.a
  sgx-enclave-devel.x86_64: E: lto-no-text-in-archive /usr/x86_64-intel-sgx/lib64/libsgx_pclsim.a

False positive, Not normal libraries, this is SGX enclave code


  linux-sgx.spec: W: invalid-url Source3: prebuilt_dcap_1.22-repacked.tar.gz

False positive, required due to need to strip forbidden source files.


  linux-sgx.src: W: invalid-license LicenseRef-Public-Domain
  sgx-aesm.x86_64: W: invalid-license LicenseRef-Public-Domain
  ...snip...
  tdx-attest-libs.x86_64: W: invalid-license LicenseRef-Public-Domain
  tdx-qgs.x86_64: W: invalid-license LicenseRef-Public-Domain

Should be LicenseRef-Fedora-Public-Domain


  sgx-common.x86_64: W: files-duplicate /usr/share/licenses/sgx-common/licenses/external/dcap_source/tools/SGXPlatformRegistration/inf/MPA_UEFI_Components/License.txt /usr/share/licenses/sgx-common/licenses/external/dcap_source/tools/PCKRetrievalTool/License.txt:/usr/share/licenses/sgx-common/licenses/external/dcap_source/tools/SGXPlatformRegistration/inf/MPA_Network_Components/License.txt

Not desirable to change, because while they may currently have matching text, this can change on new releases.


  sgx-enclave-devel.x86_64: W: binary-or-shlib-calls-gethostbyname /usr/bin/sgx_edger8r

Valid, but harmless in this context, so won't change

Comment 17 Richard W.M. Jones 2025-02-14 15:13:02 UTC
This one looks good now.  FYI I believe that macro-in-comment *is* actually
a problem, if the macro contains multiple lines (which these obviously don't).

Comment 18 Richard W.M. Jones 2025-02-14 15:13:27 UTC
** This package is APPROVED for Fedora by rjones **

Comment 19 Fedora Admin user for bugzilla script actions 2025-02-14 15:54:13 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/linux-sgx


Note You need to log in before you can comment on or make changes to this bug.