estevaocm / bitkey

A self-contained read-only CD/USB stick with everything you need to perform highly secure air-gapped Bitcoin transactions. Offline cold storage made (slightly more) practical.
https://bitkey.io
23 stars 9 forks source link

Discussion: Conventions in /overlay/usr/local/src/ folder #21

Closed emartec closed 6 years ago

emartec commented 6 years ago

Has I was implementing Exodus integration I stepped on this directory which didn't seems to have any convention concerning the files in it.

The common base of this directory is that it contains the sources of all the apps installed during the build. But I found many non-consistent points.

Here are the inconsistencies that I can see:

If we would make some convention in this, Here are my suggestions:

For all the "checksum" stuff, I actually wonder if it really have it's usefulness... Here's my concerns about it :

If you think that the checksums are an absolute necessity , Here would be my recommandations:

Please note that I don't want you to take personally the points I've wrote up here. This is absolutely not a assessment of "bad stuff", I just want to address points that I think it would help future contribuer to get around in the project. I really appreciate all the work you've done, and I can say that the qrcode scanner is yet the absolute main feature of this bitkey fork. I work like a charm.

estevaocm commented 6 years ago

Well, I would never take it personally as most of those points were in the original Bitkey before my fork, and all are valid points. Thank you for reviewing.

The missing hashes are my fault, I have tried to keep them up to date but it seems that with the fast updates I have missed many of them.

The ASC files have been put as provided by each project, I have not really bothered with them.

The purpose of the checksum stuff in my view is to facilitate comparison with the files provided by the respective projects. I have tried to only use official releases whenever possible but in some cases the releases are very old and riddled with bugs so I've had to download specific commits wich solve such issues. This is the case for electrum-ltc-2017-12-14-4546320.zip for instance. Because the source code does not have its dependencies and I didn't want the package to change from Git, I placed code to copy the dependencies from the main Electrum without changes to the package.

Mostly I tried following the original Bitkey's designs whenever possible, but had to make some calls like running Electrum directly without installing with Python due to compilation problems.

estevaocm commented 6 years ago

Also I'm under the impression that while the checksums are not required, when both the hash and the file are available, they do get checked during compilation. I should test this again, but I specifically remember at least one build failing due to a file's checksum not matching.

emartec commented 6 years ago

Just made a test by modifying the checksum of one app (exodus: Changed the first "8" for a "9") in SHA256SUM file, and I didn't got any errors. I've deleted the "build" directory first and then proceeded to build to ensure a clean build. Everything proceeded as expected.

From time to time, I have errors building this project, (that actually happened on my first build with the original bitkey project), and most of the time I just needed to call make again or reboot and flush the "build" folder to make it work. That behavior may have falsely made you think that the checksum was the reason...

estevaocm commented 6 years ago

Yes, I get those occasional build failures as well. But I remember a failed checksum message one time. No idea how to reproduce it. Maybe @JedMeister can give us some tips.

estevaocm commented 6 years ago

It would really be nice for these to be automatically checked. But I guess the main value in it is to facilitate for users to validate the packages that are included in the build. Towards that end, your suggestion of adding the URLs also makes a lot of sense.

JedMeister commented 6 years ago

From the FHS 3.0:

/usr/local hierarchy is for use by the system administrator when installing software locally.

And more specifically and succinctly from the Debian wiki:

/usr/local/ : Tertiary hierarchy for local data installed by the system administrator /usr/local/bin : locally compiled binaries, local shell script, etc. /usr/local/src : Source code (place where to extract and build non debian'ized stuffs)

As for the contents of /usr/local/src: It should contain unadulterated source code direct from upstream. I.e. the files exactly as they would be if you downloaded them yourself nad did not make any changes or adjustments.

If you wanted to tidy up a little and introduce some consistency, I'm not totally against that, but it is imperative that the original files are not changed in any way.

The point of that, as well as the inclusion of the checksums and signature files, is to maximise transparency and eliminate the need to trust BitKey. That is likely why the checksums and signature files are inconsistent. They have just been copied direct from upstream. So any checksum and/or signature file we have, is whatever upstream provide (and if that's not currently the case, then they need updating). Because different developers do things differently and there is no consistency across upstream, we end up with inconsistent naming, hash and signature conventions. I don't see any way around that, whilst still providing the value that keeping things unchanged across all upstream software (i.e. not consistent with one another, but consistent with each project's upstream).

To explicitly respond to your points @emartec:

The suggestion on checking the checksums at buildtime is quite a good one. For all the "checksum" stuff, I actually wonder if it really have it's usefulness... Here's my concerns about it :

  • If anyone can compromise the source app, it can probably also compromise the checksum file, and every files in the bitkey project... much more of a security issue here.

True. Which is exactly the point of including them, and keeping them exactly consistent with upstream. A BitKey user (if they wish) can check the software against the checksum; then check the checksum against upstream. Hence why it's important to keep the files & checksums exactly as upstream provide them.

  • The checksum value is not validated at compile time. Probably no one would think to check that all checksums are consistent with actual apps packages before building the ISO.

We always manually check before we do an official release as part of the code review process. Although an automated check at build time is a great idea. Not only is it a confirmation, but it would also raise the alarm if something got forgotten! FWIW, whilst it's not exactly the same, this is more-or-less the BitKey version of a feature request against TurnKey.

  • Updating the checksum on upgraded versions seems to be a task easily forgotten.

True, see above.

@estevaocm - It's been a long time since I've build Bitkey, so I'm not sure what might have caused a build to fail due to a checksum. Sorry...

emartec commented 6 years ago

@JedMeister Thank for your feedback and explanation !

So good, I agree with the idea to keep everything intact from the upstream in this folder. Maybe then only create sub-directories to separate the different apps, since we could fall on a file name conflict. What you guys think of that "convention" ?

So if we keep the checksums exactly as they were provided by the upstream, hence, a 1 checksum file per program/package (the one provided by the program source) and let go the "SHA256SUMS" file. Any objections ?

As for the /usr/local/share/ directory I found this documentation.

It refer to:

Any program or package which contains or requires data that doesn't need to be modified.

So if we consider icons as "data required by the program" we could leave the icon in this directory. But the shortcut created to start an application can also not be seen as "data required by the program". IMO would propose the leave the icons with the app source. ( /usr/local/src/ )

Any comments ?

estevaocm commented 6 years ago

TBH, I like the way it is because I can see all the source packages in a single directory view, with their versions and all. I understand your point of view but to me personally it is convenient for maintainence as it is.

There was one package that had a SHA1 hash and this was inconvenient, but since I've updated it, it now uses SHA256 as well. My phylosophy ATM is to compute everything's SHA256 and compare it to the provided one, so I garantee my download is legitimate, and include it in the consolidated file for convenience. And if the signature is also provided, I keep it in the same directory as the package, with the same name that was provided. It could be renamed to the same name as the package, I think it's fine either way.

estevaocm commented 6 years ago

Following that reference documentation, I'm thinking perhaps the Javascript and Python apps should be moved to /usr/local/lib actually. What do you guys think?

estevaocm commented 6 years ago

Or maybe it should all be in /usr/local/share.

The /usr/share hierarchy is for all read-only architecture independent data files. [30]

This hierarchy is intended to be shareable among all architecture platforms of a given OS; thus, for example, a site with i386, Alpha, and PPC platforms might maintain a single /usr/share directory that is centrally-mounted. Note, however, that /usr/share is generally not intended to be shared by different OSes or by different releases of the same OS.

Any program or package which contains or requires data that doesn't need to be modified should store that data in /usr/share (or /usr/local/share, if installed locally). It is recommended that a subdirectory be used in /usr/share for this purpose.

The JavaScript apps are not modified, and I think the same is true for the Python apps. And I don't think the Python packages have architecture-dependent code, since it is interpreted and we are not using setup.py install, just running from source. There are no .so files, mostly all are .py.

emartec commented 6 years ago

Ok, nice, let's leave it like this. But here are the remaining inconsistencies :

For the ' /usr/local/share/' , my view of this folder is that it is ment to contain static "data" (icons, fonts, etc..). The web apps can be seen as "apps" in their own, hence leave them in the /usr/local/src/ folder. Or you can see web apps as "static data" for the "firefox" or "chronium" application, hence place them in the /usr/local/share folder.

But all this is subject to interpretation, I don't think that all theses standard were designed with web apps in mind but more for the binaries apps, that were dominating at that time.

My opinion is that it is more convenient to hold all bitkey apps in one place, to make easier to maintain.

estevaocm commented 6 years ago

Yeah, I think both JavaScript/HTML and Python qualify to both "share" and "src" directories, because they are static source code that is interpreted during execution. However, I read that "src" is used mostly for compilation or future reference, so I tend to think the "share" directory would be more appropriate. But I'm fine either way.

estevaocm commented 6 years ago

I've been doing some reading in the file system hierarchy definitions for Linux and Debian to figure out the proper organization of the application files. All our applications are either Web (HTML+JavaScript) or Python (which do not need to be compiled and may be interpreted, since it may be troublesome to get the dependencies from the outdated Jessie distribution), with the exception of Exodus, which is pre-compiled for x64.

**

/opt : Add-on application software packages. Debian: Pre-compiled, non ".deb" binary distribution (tar'ed..) goes here.

So this is not what we are looking for.

**

/usr is the second major section of the filesystem. /usr is shareable, read-only data. That means that /usr should be shareable between various FHS-compliant hosts and must not be written to. Any information that is host-specific or varies with time is stored elsewhere.

Large software packages must not use a direct subdirectory under the /usr hierarchy.

Debian: Secondary hierarchy for shareable, read-only data (formerly from UNIX source repository, now from UNIX system resources) (files that are not-required to boot or rescue the system)

**

/usr/local : Local hierarchy

The /usr/local hierarchy is for use by the system administrator when installing software locally. It needs to be safe from being overwritten when the system software is updated. It may be used for programs and data that are shareable amongst a group of hosts, but not found in /usr.

Locally installed software must be placed within /usr/local rather than /usr unless it is being installed to replace or upgrade software in /usr. [27]

Debian: Tertiary hierarchy for local data installed by the system administrator

Corroborating what we have previously established, we should only use the /usr/local hiearchy for our application-specific files. But which subdirectory exactly? Candidates:

lib: Local libraries share: Local architecture-independent hierarchy src: Local source code

**

/usr/local/lib has no specific definitions, so we take a look at /usr/lib for reference.

/usr/lib : Libraries for programming and packages

/usr/lib includes object files, libraries, and internal binaries that are not intended to be executed directly by users or shell scripts.

Applications may use a single subdirectory under /usr/lib. If an application uses a subdirectory, all architecture-dependent data exclusively used by the application must be placed within that subdirectory.

Despite the architecture-dependent definition, in Ubuntu we can find in /usr/lib hundreds of py scripts from the repository packages. This could be simply a quirk, however. We also find hundreds of application packages, including the full installation with executables for browsers, libreoffice, thunderbird, and virtualbox. /usr/local/lib has Python egg packages and codec libraries. However, most files in /usr/local/lib are compiled to the specific system architecture. This is a proper location for Exodus and compiled Python applications. Currently we are not compiling Python apps, so we should not use this directory for them unless we go back to running "python setup.py install".

**

/usr/local/share : The requirements for the contents of this directory are the same as /usr/share.

usr/share : Architecture-independent data

The /usr/share hierarchy is for all read-only architecture independent data files.

This hierarchy is intended to be shareable among all architecture platforms of a given OS; thus, for example, a site with i386, Alpha, and PPC platforms might maintain a single /usr/share directory that is centrally-mounted. Note, however, that /usr/share is generally not intended to be shared by different OSes or by different releases of the same OS.

Any program or package which contains or requires data that doesn't need to be modified should store that data in /usr/share (or /usr/local/share, if installed locally). It is recommended that a subdirectory be used in /usr/share for this purpose.

It is recommended that application-specific, architecture-independent directories be placed here. Such directories include groff, perl, ghostscript, texmf, and kbd (Linux) or syscons (BSD). They may, however, be placed in /usr/lib for backwards compatibility, at the distributor's discretion.

In Ubuntu, /usr/local/share has only documents, web documents and fonts. /usr/share has hundreds of application directories, with files like fonts, background images, java 3rd-party packages, py scripts, authority certificates, ffmpeg presets, and virtualbox additions disk image.

With the above definitions and examples, I believe the interpreted python scrypts and Web applications should be installed to application-specific subdirectories in /usr/local/share. We may package the TKLDev plan with them in /usr/local/src but not leave them there.

**

/usr/src : Source code (optional)

Source code may be place placed in this subdirectory, only for reference purposes.

Debian: to build debian packages. see also /usr/local/src/

In Ubuntu, /usr/src has only bbswitch, linux-headers, nvidia-updates, virtualbox, and virtualbox-guest, while /usr/local/src is empty.

So I understand we should only leave here the sources for compiled applications, which we do not have (Exodus is already compiled so should be removed from this directory).

estevaocm commented 6 years ago

I have modified the build to do as follows: