[Bug 2085444] Review Request: sgx-sdk - Software Guard eXtension software development kit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #69 from Daniel Berrangé <berrange@xxxxxxxxxx> ---
(In reply to Yunying Sun from comment #68)
> About the bundle thing Cole suggested in comment 63, we don't quite
> understand. 
> Are you suggesting to add all third party tools that linux-sgx is using
> their old versions instead of the up-to-date system-provided versions, to
> spec file with "Provides: bundled(xxx) = <the old versions>"? 
> 
> Package guidance says:
> "All packages whose upstreams have no mechanism to build against system
> libraries MAY opt to carry bundled libraries"
> does that mean bundled libraries are not necessarily listed in spec file?

Normally you have to link against the system libraries. This guideline allows
you to skip that and instead rely on bundled libraries. When doing that though,
you need to add 

   Provides: bundled(xxx) = <the version you bundle>"

for each library you have bundled.

> > > 5. Removed subpackage "sgxsdk-samples" stuff, which will be added later after sgx-aesm-service available Fedora.
> > 
> > I'm not happy with that. As mentioned in
> > https://bugzilla.redhat.com/show_bug.cgi?id=2030595#c36 since they are
> > mutually dependant, 
> > I think sgx-sdk and sgx-aesm-service need to be proposed together as a pair
> > and reviewed at the same time.
> > 
> > I don't think we should be approving this with bits cut out, which will be
> > re-added just after acceptance, but without review.
> We removed sub-package sgxsdk-samples because as Cestmir stated in comment
> 59:
> "Perhaps sgxsdk-samples subpackage can be added later if you don't want to
> address these now -- after sgx-aesm-service (that provides both
> libsgx-enclave-common and libsgx-urt [and require many of the above
> discussed fixes, too]) is in Fedora; until then its dependencies will not be
> satisfiable anyway."
> 
> Since sgx-aesm-service depends on SGX SDK package reversely, could you
> confirm if reviewing sgx-sdk and sgx-aesm-service in pair could satisfy the
> cross-dependency for compiling?

Having a circular dependency at package build time is a problem, because
packages
are built individually. IIUC, that's not the case with the samples though. They
are just source code, and user can build themselves against the
sgex-aesm-service
package(s).

> If yes, I can add the sgxsdk-samples back(and rename it to sgxsdk-examples
> as Cole suggested in comment 63).

I'd prefer to see it kept, depsite what Cestmir said.

> > > # To build SGX SDK from linux-sgx source, first download the prebuilt
> > > # binaries by "make preparation", then run script
> > > # ./linux/installer/rpm/sdk/build.sh to update spec and repack tarball.
> > > # The repacked tarball is shared on 01.org for generating target RPMs.
> > > Source0:        https://download.01.org/intel-sgx/sgx_repo/rpm_onespec/%{name}-%{version}.tar.gz
> > 
> > These instructions don't appear to work when I try them
> > 
> > $ git clone https://github.com/intel/linux-sgx
> > $ cd linux-sgx
> > $ make preparation
> > ...snip...
> > $ ./linux/installer/rpm/sdk/build.sh 
> > ~/tmp/linux-sgx/linux/installer/rpm/sdk/sgxsdk-2.20.100.4 ~/tmp/linux-sgx
> > ~/tmp/linux-sgx
> > readelf:
> > ./linux/installer/rpm/sdk/../../../..//linux/installer/common/sdk/../../../..
> > //build/linux/sgx_sign: Error: No such file
> > ./linux/installer/rpm/sdk/../../../..//linux/installer/common/sdk/
> > createTarball.sh: line 51: test: =: unary operator expected
> > Create the dest dir
> > Error !!!: src file not exist
> > /home/berrange/tmp/linux-sgx/build/linux/libsample_libcrypto.so
> > 
> > Seems like there is some intermediate step missing
> There are some unpublished fixes(needed for compiling source on Fedora) not
> available on public github linux-sgx repo yet.
> The fixes are available only in the tarball contained in SRPM @
> https://yunyings.fedorapeople.org/sgxsdk-2.20.100.0-1.fc39.src.rpm.
> 


> > > %{_libdir}/*.so
> > > %{_libdir}/pkgconfig/*.pc
> > > %{_libdir}/sgx-gdb-plugin/
> > > %{_prefix}/lib/sgxsdk/
> > > %{_datadir}/sgxsdk/
> > 
> > I'd like to see all of these expand the list of files being packaged. This
> > serves two purposes - it causes a build failure if a file unexpectedly goes
> > missing  on a rebase, and it identifies when new stuff is added on a rebase.
> > Given that this package is bundling so much 3rd party code, I think the
> > identifying newly added libraries is especially important as it would likely
> > have implications for the license declaration.
> > 
> > > %{_includedir}/sgxsdk/
> > 
> > I don't want each .h file listed separately, but I'd like to see this list
> > the top level as again this clearly identifies what is being bundled:
> > 
> > %dir %{_includedir}/sgxsdk/
> > %{_includedir}/sgxsdk/sgx_*.h
> > %{_includedir}/sgxsdk/stdc++/
> > %{_includedir}/sgxsdk/libcxx/
> > %{_includedir}/sgxsdk/ipp/
> > %{_includedir}/sgxsdk/tlibc/
> > %{_includedir}/sgxsdk/tprotobuf/
> We will expand the list of %{_includedir}/sgxsdk/ as you suggested above. 
> Do we need to expand %{_datadir}/sgxsdk/ and %{_libdir}/sgx-gdb-plugin/ to
> its next level too?

I don't think it is needed to list every .py file in the sgx-gdb-plugin dir,
just the dir ittself.

For %{_datadir}/sgxsdk/  there are only two files, so might as well just list
them directly.

> > Furthermore, code under the CC0 license is now broadly forbidden in new packages addd to Fedora
> > https://lists.fedoraproject.org/archives/list/legal@xxxxxxxxxxxxxxxxxxxxxxx/thread/RRYM3CLYJYW64VSQIXY6IF3TCDZGS6LM/
> Latest post on this thread says: 
> "Realistically, we may also want to continue to allow CC0 covered code for a
> short period of time for new packages that are just getting into Fedora
> until some percolating has happened in those upstream communities".
> 
> Can we ship this package still with CC0 license, meanwhile we notify
> upstream about the need for a license change?
> We can refresh package license after upstream license changes. Is this
> acceptable?

I looked for other examples of package reviews since this CC0 license change,
and
the recent additino os wasi-libc had to replace the dlmalloc impl with a
different 
one under MIT license. They could have chosen to use an older versin of
dlmalloc
under the previous license, don't know why they chose a completely new malloc
impl.  So I think the precedent is that after 1 year since the announcement,
adding new packages with CC0 content is no longer permitted.

> > At least the ISC license appears relevant and isn't listed.
> It seems the ISC license only appears in the "OpenBSD Copyright Policy" part
> in License.txt. Not related to any source code?

I wouldn't trust the License.txt file summary to be accurate. What matters is
the
individual files' declared licenses, and I see lots of source files with ISC
license 
reported by the license scanner, and its identification appears to be correct
in 
these cases:

./sdk/pthread/pthread_cond.cpp: ISC License
./sdk/pthread/pthread_mutex.cpp: ISC License
./sdk/pthread/pthread_once.cpp: ISC License
./sdk/pthread/pthread_rwlock.cpp: ISC License
./sdk/pthread/pthread_tls.cpp: ISC License
./common/inc/tlibc/complex.h: ISC License
./common/inc/tlibc/inttypes.h: ISC License
./common/inc/tlibc/stdint.h: ISC License
./sdk/tlibc/gen/fpclassify.c: ISC License
./sdk/tlibc/gen/fpclassifyl.c: ISC License
./sdk/tlibc/gen/isfinite.c: ISC License
./sdk/tlibc/gen/isfinitel.c: ISC License
...snip...
./sdk/tlibc/math/e_expl.c: ISC License
./sdk/tlibc/math/e_log10l.c: ISC License
./sdk/tlibc/math/e_log2l.c: ISC License
./sdk/tlibc/math/e_logl.c: ISC License
...snip...


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2085444

Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202085444%23c69
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux