Project

General

Profile

Actions

Task #628

closed

Audit shellovych skriptu

Added by Tomáš Plesník almost 12 years ago. Updated over 9 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Tomáš Plesník
Category:
-
Target version:
Start date:
01/16/2013
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)

Description

Je potreba udelat audit shellovych skriptu, abychom se vyhnuli pripadnym problemum, zpusobenym chybami ve skriptech. Jedna se o install.sh, update.sh a uninstall.sh jak u serveru, tak i u klienta.


Subtasks 4 (0 open4 closed)

Task #801: Update dokumentace po zapracovani Auditu shellovych skriptuClosedTomáš Plesník01/16/2013

Actions
Task #733: Zmena datumu v licencich - pridat rok 2013ClosedTomáš Plesník01/16/2013

Actions
Task #850: Upravit popis odregistrce klienta v README - klient se nesmaze, ale znevalidni seClosedTomáš Plesník02/11/2013

Actions
Task #992: Do README doplnit popis a navod jak vytvorit indexy na doporucovanych sloupcich jednotlivych tabulekClosedTomáš Plesník05/07/2013

Actions

Related issues

Related to Warden - Task #837: Zkontrolovat seznam kontrolovanych perlovych balikuClosedTomáš Plesník02/06/2013

Actions
Related to Warden - Task #846: Odstranit root kontroluClosedTomáš Plesník02/11/2013

Actions
Related to Warden - Task #847: Pridat kontrolu, zdali je rsync nainstalovan v OSClosedTomáš Plesník02/11/2013

Actions
Related to Warden - Bug #910: rsync při update a unistallClosedTomáš Plesník02/26/2013

Actions
Related to Warden - Bug #802: 8. Server: Update instalacnich + buildovacich skriptuClosedTomáš Plesník01/28/2013

Actions
Related to Warden - Task #848: Pri mazani ci updatu Warden serveru zajistit ponechani uzivatelskych souboruClosedTomáš Plesník02/11/2013

Actions
Actions #1

Updated by Pavel Kácha over 11 years ago

Podivnosti ve skriptech

Chyby, bylo by vhodné upravit do 2.2

Robustnost s whitespace

Skripty nejsou odolné proti mezerám/enterům v cestách, řada proměnných (err, backup_dir) je používána tak, že nesmí obsahovat mezeru. V těchto případech si to dokážeme zařídit (jsou prakticky statické a námi nastavované), ale riskujeme lidskou chybu v budoucnu. ALE v případě například client_path, kterou předává parametrem správce při instalaci, máme problém, pokud chce v názvu použít nestandardní znak (mezera, tabelátor, enter). Pomiňme nerozumnost takového počínání, my bychom měli být robustní a ne bídně hynout s podivnými chybovkami. Interpolace proměnných je potřeba obalovat uvozovkami: "$err"

Nutný root

rootChck, root_chck - proč? Mám za to, že by mělo být možné instalovat/updatovat i jako uživatel, zejména klientskou knihovnu, ale nevidím důvod, proč ne i server.

Instalace/upgrade vytváří nadbytečný podadresář

update.sh, install.sh: makeWardenDir vytváří podadresář warden-server
cp -R ${dirname}/warden-server $prefix do požadované cesty zkopíruje celý
adresář warden-server, zatímco by měl zkopírovat pouze jeho obsah.

Závažné, lépe upravit do 2.2, ale v případě nutnosti přežijeme stávající stav

Nerobustní migrace konfigurace v update.sh

Veškerá grepování konfiguračních souborů (apache_conf_file, server_conf_file) na konfigurační hodnoty spoléhají na to, že správce změnil soubor tak, aby tam hledaný řetězec zůstal právě jednou. Pokud se hledaný řetězec bude vyskytovat na více řádcích, např. jednou v poznámce, např. takto:

$BASEDIR="/usr/local/warden-server" 
# $BASEDIR="/opt/warden-server" 

interpolace do proměnné selže (a navíc shell trimuje whitespace, \r, \n).

V případě perlových konfiguračních souborů je řešením například získávat hodnoty přímo pomocí Perlu:

getConfValue() {
        perl -e "require \"$1\"; print \"\$$2\";" 
}

db_name=$(getConfValue "$server_conf_file" DB_NAME)

V případě apache_conf_file je robustní parsování nemožné, ale můžeme ho vygenerovat z hodnot, získaných z konfigurace serveru a z parametru příkazové řádky.

Text defaultních konfiguračních souborů je součástí instalačního skriptu

Generování souborů přímo z instalátoru není zrovna pěkné, každá změna defaultní konfigurace znamená i změnu instalačních skriptů. Bylo by fajn mít šablony mimo samotný skript, např. warden-server.conf.tmpl a v nich jen nahrazovat placeholdery. Rychlý nástřel (perl musíme mít, tak proč ho nepoužít):

doTemplate() {
        vars="" 
        while (($#)); do
                vars="'$1'=>'$2', $vars" 
                shift; shift
        done
        perl -e "my %repl=($vars); foreach my \$l (<>) {\$l =~ s/\$_/\$repl{\$_}/g for keys %repl; print \$l}" 
}

doTemplate \
        _CERT_          "$cert" \
        _KEY_           "$key" \
        _CA_FILE_       "$ca_file" \
        _LIB_           "$lib" \
        < warden-server.conf.tmpl \
        > warden-server.conf

Nahradí text _CERT_ pomocí $cert atd.

Seznam závislostí na více místech

Seznam potřebných perlových modulů je udržován na dvou místech pro server, na dalších dvou pro klient. Navrhuju do dokumentačního adresáře přidat soubor např. CPAN_DEPS, co řádek, to jméno modulu, a ten používat. Dal by se následně i generovat, místo ruční správy. Je to podobný problém jako templaty - každá změna teď znamená změnu instalátoru.

Mazání bez ohledu na správce

update.sh: rsync smaže jakékoliv změny, které správce ve stromu udělal. uninstall.sh: smaže celý strom, včetně změn, které si mohl udělat správce, a včetně souborů, které si do stromu přihodil. Vytvořit doc/MANIFEST se seznamem souborů, které se mají instalovat a odinstalovávat, a tím se ve skriptech řídit?

Drobnosti a subjektivní

Přemíra konstrukcí

cat XXX | grep YYY

aneb autor je výhercem ceny nadbytečných catů. Stačí:
grep YYY XXX

Pro konstrukce typu

old_package_version=`cat $old_package_version_file`

také není potřeba cat, stačí:
old_package_version=$(< $old_package_version_file)

Konstrukce ret_val=`echo $?` - proč nestačí ret_val=$?

Ostatně na testování návratových hodnot:

příkaz
ret_val=`echo $?`
if [ $ret_val -eq 0 ]; then echo "OK"; else errClean; fi

není potřeba několik interpolací a řetězcových porovnání, stačí

příkaz && echo "OK" || errClean

případně
if příkaz; then
        echo "OK" 
else
        errClean
fi

Basename je externí program, jehož existenci bychom měli otestovat. Není ale potřeba, místo basename "$value" stačí ${value##*/}. Totéž dirname: ${value%/*}

createSymlinks, deleteSymlinks: závislost na GNU ls (a jeho parametru -1).
for file in "$bin/"* a ve smyčce ln -s "$file" "$local_bin/${file##*/}" odvede stejnou práci.

Pokud víme, že mažeme soubor, mělo by být použito rm -f (např. deleteSymlinks v uninstall.sh), ne rm -rf.

Nepoužité proměnné, namátkou (v různých skriptech různé) key_file, cert_file, hostname, var, dirname, etc, doc.

Actions #2

Updated by Tomáš Plesník over 11 years ago

Cau Pavle, mam se ujmout techto uprav, nebo je do skriptu udelas ty?

Actions #3

Updated by Pavel Kácha over 11 years ago

  • Due date set to 07/22/2013
  • Assignee changed from Pavel Kácha to Tomáš Plesník

Po domluvě s A. předávám tobě. Dělej to pls pro server a vždy po hotovém kusu drbni do Sukiho, ať si to příslušně inkorporuje do klientského skriptu. První tři bychom měli do 2.2 ještě dostat, zbytek uvidíme.

Actions #4

Updated by Tomáš Plesník about 11 years ago

Pavel Kácha wrote:

Nutný root

rootChck, root_chck - proč? Mám za to, že by mělo být možné instalovat/updatovat i jako uživatel, zejména klientskou knihovnu, ale nevidím důvod, proč ne i server.

Jak se bez rootovskeho opravneni postavime k vytvareni symlinku z

/usr/local/bin
smerujicich do adresare bin prave nainstalovaneho warden serveru? Do
/usr/local/bin
ma totiz pravo zapisovat pouze root.

Actions #5

Updated by Pavel Kácha about 11 years ago

No, když o tom tak přemýšlím, tak ono není právě korektní psát mimo strom, kam se instaluje. Kromě těch symlinků, píšeme mimo instalační adresář ještě něco jiného?

Je to ale užitečná fíčura, která usnadňuje práci, co takhle přidat explicitní parametr, třeba --symbin? Bez něj se nebude dít nic, s ním se do příslušné cesty vytvoří symlinky.

Actions #6

Updated by Tomáš Plesník about 11 years ago

Pavel Kácha wrote:

No, když o tom tak přemýšlím, tak ono není právě korektní psát mimo strom, kam se instaluje. Kromě těch symlinků, píšeme mimo instalační adresář ještě něco jiného?

Nee, toto je jedine psani mimo strom. Nepocitam tmp soubory, ktere se ukladaji do /tmp adresare a ktere skripty po sobe uklidi.

Je to ale užitečná fíčura, která usnadňuje práci, co takhle přidat explicitní parametr, třeba --symbin? Bez něj se nebude dít nic, s ním se do příslušné cesty vytvoří symlinky.

Klidne muze byt. Znamena to tedy, ze si ale uzivatel bude muset zajistit prislusna prava pro zapis do teto prislusne cesty?

Actions #7

Updated by Pavel Kácha about 11 years ago

Je to ale užitečná fíčura, která usnadňuje práci, co takhle přidat explicitní parametr, třeba --symbin? Bez něj se nebude dít nic, s ním se do příslušné cesty vytvoří symlinky.

Klidne muze byt. Znamena to tedy, ze si ale uzivatel bude muset zajistit prislusna prava pro zapis do teto prislusne cesty?

Určitě. Ve většině případů to stejně budou lidi instalovat jako rooti, tam není problém, a pokud si vytvoří uživatele, obvykle vědí co dělají a linky budou chtít třeba v $HOME/bin, anebo musí s právy stejně zahýbat výrazněji, a pak bude lepší, když si je vyrobí sami.

(Pokud si balík nainstaluju do /home/ph pod uživatelem ph:users, linky v /usr/local/bin jsou k ničemu - velmi pravděpodobně je budu moci použít jen já, takže nemají co dělat v /usr/local/bin, nebo je bude moci použít root, což by ale neměl, protože soft je nainstalovaný pod uživatelem ph nejspíš záměrně právě proto, aby pod rootem neběžel. A nikdo jiný je obvykle použít nemůže, protože se do /home/ph nejspíš nedostane.)

Actions #8

Updated by Tomáš Plesník about 11 years ago

wardenWatchdog se bude take distribuovat s balickem warden-server-2.2? Jen at vim jestli se ma o nej instalator take starat.

Actions #9

Updated by Tomáš Plesník about 11 years ago

Tomáš Plesník wrote:

wardenWatchdog se bude take distribuovat s balickem warden-server-2.2? Jen at vim jestli se ma o nej instalator take starat.

A jeste by me zajimalo, pro ma sve vlastni README, kdyz je cely pridruzen do serveroveho stromu, proc je jeho conf soubor pojmenovan jinou konvenci nez conf soubory serveru a proc i jeho help nevypada vizuelne stejne jako ostatni soubory v adresari bin?

Actions #10

Updated by Pavel Kácha about 11 years ago

Ano, bude. S ostatním na Kubu, některé věci, kterých jsem si všiml, už upravoval.

Actions #11

Updated by Pavel Kácha about 11 years ago

Ahoj, Andrea hlásila, že mám zkouknout skripty - koukal jsem na verzi 262ff7d0.

update.sh zřejmě ještě není upravený, takže okometricky co se týče install a uninstall:

V modulesChck si vytváříš mezisoubor, který pak musíš mazat a starat se o něj v případě chyb:

        sed '/^use [A-Z]/!d; /Warden/d' $(find "${dirname}" -type f) 2>/dev/null | cut -f 2 -d " " | sed 's/;//' | sort -u > "$modules_file" 
        for module in $(<"$modules_file");
        ...

Nejspíš by stačila roura a máš o starost míň:

modulesChck()
{
        sed '/^use [A-Z]/!d; /Warden/d' $(find "${dirname}" -type f) 2>/dev/null | cut -f 2 -d " " | sed 's/;//' | sort -u | \
        while read module;
        ...
}

Je obvykle rozumné při generování dočasných souborů (warden-err,
warden-backup, atd.) se pokusit minimalizovat race conditions, třeba:

err="/tmp/warden-err".$$.$RANDOM

Nebylo by dobré err() dát možnost parametru a u volání přidat lidský popis co se podělalo?

To vytváření manifestu hned po instalaci je docela dobrý kompromis (proti
dodávání manifestu s instalací), to se mi líbí.

Kontroly na koncový slash u basedir a symbin - nice touch.

Dobrá práce, úsporné, čitelné!

Actions #12

Updated by Tomáš Plesník almost 11 years ago

Pavel Kácha wrote:

Ahoj, Andrea hlásila, že mám zkouknout skripty - koukal jsem na verzi 262ff7d0.

Ahoj,

ano je to tak. Dival ses na spravnou verzi.

update.sh zřejmě ještě není upravený, takže okometricky co se týče install a uninstall:

Ano, ten jeste neni, ale pracuji na nem. Od 13.12.2013 (od tohoto patku) mam az do 6.1.2014 dovolenou, kterou hodlam zasvetit praci na wardenu a to prave na dodelani update.sh skriptu, kontrole kodu klienta a upravu instalacnich skriptu klienta podle techto serverovych tak, aby bylo mozne vybuildovat oba baliky v beta verzi.

V modulesChck si vytváříš mezisoubor, který pak musíš mazat a starat se o něj v případě chyb:
[...]

Nejspíš by stačila roura a máš o starost míň:
[...]

Ano je to tak a uz jsem to i podle toho upravit a nahral do GITu.

Je obvykle rozumné při generování dočasných souborů (warden-err,
warden-backup, atd.) se pokusit minimalizovat race conditions, třeba:
[...]

Ano to mas pravdu. Pokusim se rovnez predelat.

Nebylo by dobré err() dát možnost parametru a u volání přidat lidský popis co se podělalo?

To vytváření manifestu hned po instalaci je docela dobrý kompromis (proti
dodávání manifestu s instalací), to se mi líbí.

Diky! Ted se vlastne vytvari prvni MANIFEST file, ciste a jen pro balicek, a druhy se sestavi pro nainstalovany server ihned po jeho instalaci.

Kontroly na koncový slash u basedir a symbin - nice touch.

Oka.

Dobrá práce, úsporné, čitelné!

Dekuji, tohoto radku si opravdu moc vazim. Potesi!

Tom

Actions #13

Updated by Tomáš Plesník about 10 years ago

  • Status changed from New to In Progress

Zbyva uz jen doresit funkci errClean ve skriptu update.sh.

Actions #14

Updated by Tomáš Plesník almost 10 years ago

Aktualne se jedna o audit shellovych skriptu klienta.

Actions #15

Updated by Tomáš Plesník over 9 years ago

  • Status changed from In Progress to Closed

Hotovo jak pro server, tak i pro klienta.

Actions

Also available in: Atom PDF