Code Review auf Sicherheit

Nur weil Du eine reprozierbare und vertrauenswürdige Buildumgebung hast, bedeutet das doch noch lagne nicht, dass die Qualität des Codes steigt.
Ich sehe auch ehrlich gesagt nicht, wie „reproduzierbare Builds“ überhaupt den Codereview untersützten könnten.

Eventuell bringt man damit natürlich mehr Leute ins Boot, klar. Aber sonst?

Das ist richtig, aber hierum ging es bei der Debatte die ich zuletzt mit fuzzle hat auch nicht. Hierbei ging es darum die resultierenden Images vergleichen zu können. Dafür ist eine solche Buildumgebung die richtige Variante.

Die Codequalität kann nun mit den gängigen Mitteln gesteigert werden. Reviews, Refactoring, noch mehr reviews und nicht zuletzt reviews. Diese macht man basierend auf den Commits und ggf. in einem üblichen Refacotoring-Schritt den man ohnehin macht.

Es muss darum gehen, mehr Leute ins Projekt zu bringen.
Gluon (und auch LEDE) sind viel zu schmal aufgestellt, um der Forderung nach „sicherem und gutem Code“ gerecht zu werden.

Irgendwelches Nitpicking in Richtung „die GITs und Buildumgebungen sind inkonsistent“, meinetwegen auch von @fuzzle (oder @rubo77) bringt uns in diesem Problem nicht weiter.
Was wir hier nun besprechen ist doch eher ein Sicherheitsaudit der jeweiligen Build-Umgebungen und der von den Community genutzten Repositories und Toolchain, würde am eigentlich Gluon/LEDE-Code wenig ändern (hoffentlich…)

na, @rubo77 hat den thread hier ausgegraben, und unwichtig ist das thema an sich ja nicht. Er fing die Diskussion um code und commits und deren Verfügbarkeit an.
… darauf eingehend fing der kleine Exkurs dazu an.

nur um das nochmal in Erinnerung zu rufen - bzw zusammenzufassen.

@adorfer - deinen letzten beitrag versteh ich semantisch nicht, da fehlt nen komma oder irgendwas

Sinnvoll halte ich auch das mehr Menschen sich proaktiv mit Code und Netzwerken auseinander setzen. Das dient der Diversität, verteilt Last auf Schultern. Das erfindet das Rad ggf mehrfach neu und führt zu „evolutionärer“ codequalität(sunterschieden). Survival of the fittest - mit ohne bugs.

Danke, ich hab’s mal editiert.

Wir brauchen in jedem Fall mehr Leute, die testen und sich einbringen. (Und sich auch nicht von den bisweilen rustikalen Umgangsformen auf der Gluon-Mailingliste abschrecken lassen.)

Zusammenfassung:

1.

Ein externer Prüfer sieht, welche firmware auf einem Router installiert ist, dabei sieht er im Moment nur in der internen config files in /lib/gluon/gluon-version, release und site.json. In den ersten beiden steht z.b.

  gluon-version:v2016.1.6
  release:2016.1.6

Die gluon-version anthält die commit ID nur, wenn man nicht genau auf einem annotated tag gebaut hat, dann steht dort sehr genau z.b.

2016.2~exp1611131432 / gluon-v2016.2.1-3-g0f9a1a9

also commit-id 0f9a1a9 genau 3 commits nach dem tag gluon-v2016.2.1

Außerdem enthält die site.json die exact benutzte site config, anhand der könnte ein Prüfer die Firmware genau so nachbauen.

2.

Hier kommt die Docker Diskussion ins spiel: Wenn man mit einem vorgegebenen Docker image gebaut hätte, dann könnte der Prüfer jetzt das selbe Docker Image nehmen und anhand der site.json und dem gluon-commit die Firmware selbst auch bauen und dann die resultierenden images binär vergleichen.

Damit sieht der Prüfer, dass die Firmware wirklich mit diesem Docker image gebaut worden ist.

3.

Das verschiebt aber natürlich den Angriffsvektor auf das Docker-Image, das jetzt überprüft werden müsste auf Hintertüren:

Dazu könnte man eine Anleitung, wie man das Docker Image aus Standard-Paketen erstellt hat veröffentlichen, nachbauen und dann die Docker-Images überprüfen.

Ich hab das wichtigste hier ergänzt: https://wiki.freifunk.net/Sicherheit

Ehrlich gesagt halte ich das für overpowered.

Sinnvoller wäre es wenn die commits der genutzten Grundlagen nachvollziehbar wären. Sonst könnte man auch einfach den build Ordner 7zippen, hashen und zum DL anbieten. Das hätte den selben Effekt wie Docker.

Ergänzung zum Wiki:
Jede Community die den AU nutzt bietet Hash Werte zum überprüfen an. Sie stehen im Manifest.

1 Like

Zuerst einmal sollte der gesamte site-Ordner in der Firmware landen, sodass man sie einfacher nachbauen kann. Alternativ auch gerne der git-commit im site-Ordner.
Ich habe nicht den Anspruch, dass die binaries, die rausfallen, identisch sind. Mir reicht es, wenn nach Entpacken, (evtl. strings) und rekursivem diff nur so Dinge wie Datum, Pfad zum build-Verzeichnis und so weiter rauskommen.

Wenn man dem Firmware-Bäcker vertraut, reicht es aber, seine Signatur auf dem Manifest und die Hashsumme zu prüfen.

1 Like

Ich glaube du gehst davon aus, dass das gleiche Environment und der gleiche Code zu einer bitweisen identischen Binary führen.

Das ist aber nicht zwangsweise der Fall. Debian gibt sich seit einiger Zeit Mühe damit und hat trotzdem immer noch 20% Pakete die nicht reproducible bauen.

Ist das also überhaupt bei Gluon schon so? Sonst ist die Diskussion imo erstmal müßig. (Wobei es in dem Fall natürlich schön wäre wenn jemand™ sich des Themas annimmt und Gluon entsprechend patcht)

Meiner Meinung nach hat das trotzdem nichts mit „Codereview“ zu tun.
Sondern eben mit Reproduzierbarkeits von Builds (und evtl. dem Signaturprozess in Communities).

Oder anders: Ich halte das Thema für so relevant, dass es einen eigenen Thread verdient hätte.

2 Likes

ja gerne einen „Reproduzierbarkeits von Builds“ Thread abspalten.

Zu code-Review gehört nur, wie man rausfindet, welche commit_id benutzt wurde, damit man sich das gluon repo zu diesem commit anschauen kann. Dazu gehört auch die Erkenntniss, dass anscheinend dazu die information fehlt, welche site-conf commit-id benuttz wurde.
dazu sollten wir mal ein issue im gluon machen, wenns das noch nicht gibt

@rubo77 du meinst sowas wie das hier ?

0 root@fffr-rauberhaus2:~# cat /lib/gluon/gluon-version 
v2016.2-28-g5ef3d88

gluon version +28 commits mit id g5ef3d88 - wenn man auf einem tag baut steht da auch „nur“ v2016.2.1 bspw.

@fuzzle: das ist der commit aus dem gluon repo. Es fehlt die URL und commit ID, welches site-repo benutzt wurde.

Ich hab das mal hier als Pull Request erstellt:

https://github.com/freifunk-gluon/gluon/pull/937 - Add remote URLs and commit-IDs of gluon and site to config files.

Hab das getestet, funktioniert, er baut dann images, wo die URL zum git repo und site-repo mit in der config stehen

1 Like

Im Master ist jetzt drin, welcher Commit aus dem site-Repository benutzt wurde

/lib/gluon/gluon-version 
/lib/gluon/site-version 

Die Communities müssen jetzt allerdings Kommunizieren, wo Ihr benutztes site-Repository liegt und ob sie das original Gluon-Repository benutzt haben oder wo der eingene Fork liegt.

Man kann im Router jetzt erkennen, mit welchen Gluon- und Site-Repository Commits die Firmware gebaut wurde, aber nicht die Repository-URLs.

1 Like

Ich habe einen PR erstellt, die neue site-version im config mode auf der Info page anzuzeigen:

Wenn das so angenommen wird, dann können wir dort auch einen Link zu unserem Site Repository plazieren

1 Like

Wie kleben an jeden Build dies dran: http://firmware.4830.org/rawhide/factory/ReleaseNotes-1.0.0~34

Wahrscheinlich sollten die Repository-URLs noch mit rein und bei der Gelegenheit der verwendete Checkout für das site-Repo …

2 Likes

Alternativ, man klebt parallel zu „sysupgrade“ und „factory“ noch ein Directory „site“
in das man neben der site.conf und der site.mk auch die modules-Datei und language-Datei und das gepackte Buildlog kopiert.
Und das Buildscript ist auch drin. So dass nun prinzipiell jedeR die Firmware nachbauen können sollte.

Beispiel:

https://images.ffdus.de/stable/site/

1 Like

Super Ideen. Wir werden beides umsetzen. Die Datei von Wusel kann man auch einfach README.md nennen, dann wird die in GitHub auch gleich schon angezeigt.

Da könnte man dann eigentlich auch gleich eine generierte Index.HTML mit rein packen mit einem schönen Download selector

Das finde ich nun wiederum oversized; der Gluon-Build-Prozeß ist dokumentiert und basiert auf Commit-IDs, mit öffentlichen Repos und Commit-IDs ist nicht nur formal der GPL genüge getan.

Der Hintergrund für unsere »ReleaseNotes«-Datei ist schlicht der, das auch wir in der Lage sein wollen, zu einem Stand X, den wir ggf. aus Platzgründen schon wieder gelöscht haben, zurückkehren können wollen.

Hmm, die wird beim Build erzeugt, man müßte sie dann nach dem Build neu einchecken, was einen neuen Commit bedeutete, der automatisch einen neuen Build triggerte …

warum nicht. Wir haben das jetzt umgesetzt: https://freifunk.in-kiel.de/firmware/nightly/2018.1~ngly-245/

481K Jul 21 14:29 site.tgz

das ist glaube ich vertretbar und man kann sich das gleich lokal entpacken und loslegen :wink:

Ihr könntet in dem Readme ja einen Platzhalter anlegen mit <!-- hier kommt die release note rein -->, der dann im buildprozess nur ausgetauscht wird, bevor die datei hochgeladen wird.

auch fertig:

https://freifunk.in-kiel.de/firmware/nightly/2018.1~ngly-245/site/download/

1 Like