Not only is the Internet dead, it's starting to smell really bad.:2018年05月上旬

2018/05/01(Tue)

[プログラミング] debugging MiniDLNA(ReadyMedia) その3

まとも技術者たるものDLNAの仕様書つーかガイドラインも読んでおくべきなのかもしれんけど、ワイは技術者じゃない上にそもそもメンバー以外お断りのよくあるパターンなので以下略。 今回のトラブルも現時点ではDLNAの仕様とは無関係の可能性が高いしな。

ちなみにUPnP AVの方は IEC 62481-1:2017でいつものスイスフランでお布施すれば買えるもよう、なおそんなお金があったらルーター買い換えるので以下略。

Miniというだけあってソース少ないので読むのは苦にならないと思う、コードも綺麗な部類でしょう(*BSD性パラノイア患ってたら知らん)

$ ls
albumart.c            getifaddr.c        log.c            minixml.c   sql.h              upnpdescgen.h         upnpsoap.c
albumart.h            getifaddr.h        log.h            minixml.h   tagutils           upnpdescstrings.h     upnpsoap.h
codelength.h          icons.c            Makefile         options.c   testupnpdescgen.c  upnpevents.c          utils.c
config.h              image_utils.c      metadata.c       options.h   tivo_beacon.c      upnpevents.h          utils.h
config_R6300v2.h      image_utils.h      metadata.h       playlist.c  tivo_beacon.h      upnpglobalvars.c      uuid.c
config_WNDR4000AC.h   inotify.c          minidlna.c       playlist.h  tivo_commands.c    upnpglobalvars.h      uuid.h
config_WNDR4500REV.h  inotify.h          minidlna.conf    po          tivo_commands.h    upnpglobalvars.h.old
CVS                   INSTALL            minidlnapath.h   README      tivo_utils.c       upnphttp.c
daemonize.c           LICENCE            minidlnatypes.h  scanner.c   tivo_utils.h       upnphttp.h
daemonize.h           LICENCE.miniupnpd  minissdp.c       scanner.h   TODO               upnpreplyparse.c
genconfig.sh          linux              minissdp.h       sql.c       upnpdescgen.c      upnpreplyparse.h

命名も簡潔でどこから読めばいいかgrepなどのツールに頼らずとも明確で人はかくあるべしという感じ、こういう整理ができない無能を滅ぼさないからプログラミング業界は地獄なんだよな。

とりあえずls叩いただけでまずワイが読むべきなのは優先順位的に

であって後は補助的に

あたりを読めばいいんだろうなーと理解できるというのはほんと素晴らしいことだと思うよ。

そしてこの優先順位どおりにscanner.hを開くと

 62 int
 63 is_video(const char * file);
 64
 65 int
 66 is_audio(const char * file);
 67
 68 int
 69 is_image(const char * file);
 70
 71 sqlite_int64
 72 get_next_available_id(const char * table, const char * parentID);
 73
 74 int
 75 insert_directory(const char * name, const char * path, const char * base, const char * parentID, int objectID);
 76
 77 int
 78 insert_file(char * name, const char * path, const char * parentID, int object);
 79
 80 int
 81 CreateDatabase(void);
 82
 83 void
 84 start_scanner();

とあってこのプロトタイプ宣言を読むだけで

何をしているのか把握できる理想的な命名規則ですな(スネークケースとキャメルケースが混在してるだけで発狂するパラノイアは知らん)、素晴らしい。 命名はコメントに勝るんですわ、命名に失敗してグダグだコメント書くやつは無能ってはっきりわかんだね。

そしてお次はinotify.hを開くと

  2 int
  3 inotify_remove_file(const char * path);
  4
  5 void *
  6 start_inotify();

とあってこちらもプロトタイプ宣言を読むだけで

とわかる、なんか今度は微妙にわかりづらくなってるのはそれがあなたがLinuxの inotify(7)をご存じないか、inotifyという命名がLinuxというかUNIX文化の連中のセンス壊滅的かつグロ放送禁止そして現代美術の醜さそびえ立つクソということですわ。

今回も相変わらず技術的な中身は無いけども、命名が下手糞なプログラマという存在はそれだけで死罪であの世でハートマン軍曹に罵られてきやがれという事が判っていただければそれでヨシ。

[プログラミング] debugging MiniDLNA(ReadyMedia) その4

前回ヘッダから発見したスキャン開始のコードと思われるstart_scanner()が呼ばれてるのは以下の場所

 849 /* === main === */
 850 /* process HTTP or SSDP requests */
 851 int
 852 main(int argc, char * * argv)
 853 {
...
 937                 if( CreateDatabase() != 0 )
 938                 {
 939                         DPRINTF(E_FATAL, L_GENERAL, "ERROR: Failed to create sqlite database!  Exiting...\n");
 940                 }
 941 #if USE_FORK
 942                 scanning = 1;
 943                 sqlite3_close(db);
 944                 scanner_pid = fork();
 945                 open_db();
 946                 if( !scanner_pid ) // child (scanner) process
 947                 {
 948                         start_scanner();
 949                         sqlite3_close(db);
…

937行目のCreateDataBase()でSQLite3データベースファイルを作成した後に944行目でfork、そして948行目からの子プロセス側で実行される処理部で呼び出している。

ここでわざわざforkしてるのはstart_scanner()はとても時間かかる処理なので、終わるまで何もできないよりも裏でやらせたほうが都合がいいからでしょうな。 デバッグする側としてもstart_scanner()の処理は専用プロセスで処理されてるので親が何やってるのか読む必要なくなるのでこれは楽。

そして今起きてる現象との整合性もばっちりよね。

って状態なので、このスキャン専用の子が何らかの理由で突然死したけども親はそれに気付いてないと想像つきますな、ひゃーネグレクト。

実際コード読んでも親と子の間でpipe(2)なんかのプロセス間通信を使ってる形跡が無さそうなんだよね、もしかすると

可能性はあるかも知れんけどね、まぁおいおい読んできゃ判るのでしょ(慢心)。

それでは実際にstart_scanner()の実装を読んでみる。

844 void
845 start_scanner()
846 {
847         struct media_dir_s * media_path = media_dirs;
848         char name[PATH_MAX];
849
...
860         while( media_path )
861         {
...
862                 strncpy(name, media_path->path, sizeof(name));
863                 GetFolderMetadata(basename(name), media_path->path, NULL, NULL, 0);
864                 ScanDirectory(media_path->path, NULL, media_path->type);
865                 media_path = media_path->next;
866         }

847行目にあるmedia_dir_s構造体はスキャン対象のディレクトリ(複数) *1を保持しているlinked list、これを860~866行目でループ処理している。

862行目では次行で呼ばれているbasename(3)の為にmedia_dirをname変数にコピーしている。 これ普段glibc相手にプログラムしてる人は忘れがちなんだけど、basename(3)やdirname(3)引数を破壊することをPOSIXは許容しているので必要な処理なのよね。

そして862行目、よく訓練されたプログラマならstrncpy(3)使ってることにブチ切れてバグレポート叩きつけるレベル。 strncpy(a, b, n)はbの文字列長がnを超える場合、aは終端処理されないからオーバーラン系のバグの温床になるのでこの使い方はアウト。 どうするのが正しいかは信じる宗教によって変わるので null 終端バイト文字列を不注意に切り捨てないあたり読んでどうぞ。

そしてこのループ処理の中で

という2つの処理を呼び出していることが判る。

そんでGetFolderMetadataは前回副次的に読めばいいんでねと書いたmetadata.[ch]にある。

 313 sqlite_int64
 314 GetFolderMetadata(const char * name, const char * path, const char * artist, const char * genre, sqlite3_int64 album_art)
 315 {
 316         int ret;
 317
 318         ret = sql_exec(db, "INSERT into DETAILS"
 319                            " (TITLE, PATH, CREATOR, ARTIST, GENRE, ALBUM_ART) "
 320                            "VALUES"
 321                            " ('%q', %Q, %Q, %Q, %Q, %lld);",
 322                            name, path, artist, artist, genre, album_art);
 323         if( ret != SQLITE_OK )
 324                 ret = 0;
 325         else
 326                 ret = sqlite3_last_insert_rowid(db);
 327
 328         return ret;
 329 }

おおう…なんもGetなんてしてないやん詐欺やんこんなの、Getすると思わせてINSERTするなんて卑怯だわ。

これはどうしてかというとディレクトリでない他のファイル形式の場合は

とファイルから情報をGetする処理があるからの命名なんだろう、なので本当はGetとInsertを分割すべきなんだけどな。

つまりここのGetFolderMetadataは単に検索開始するディレクトリ名をDETAILSテーブルに登録してるだけってこと。 それならScanDirectoryの中で呼べばいい気がするんですけどね…

次回はScanDirectoryの実装を読んでいく予定。

*1:設定ファイル(minidlna.conf)のmedia_dirで指定した値やね。

2018/05/02(Wed)

[プログラミング][今すぐ窓から投げ捨てろ] debugging MiniDLNA(ReadyMedia) その5(最終回)

いきなりだけどScanDirectoryのコードを読んだら原因判明してしまった…

719 #define MAX_FILE_NUMBER 25000
720 void
721 ScanDirectory(const char * dir, const char * parent, enum media_types dir_type)
722 {
...
738     if(fileno >= MAX_FILE_NUMBER) // stop scanner
739         return;
…
812         if( (type == TYPE_DIR) && (access(full_path, R_OK|X_OK) == 0) )
813         {
814             insert_directory(name, full_path, BROWSEDIR_ID, (parent ? parent:""), i+startID);
815             sprintf(parent_id, "%s$%X", (parent ? parent:""), i+startID);
816             ScanDirectory(full_path, parent_id, dir_type);
817         }
818         else if( type == TYPE_FILE && (access(full_path, R_OK) == 0) )
819         {
820             if( insert_file(name?name:namelist[i]->d_name, full_path, (parent ? parent:""), i+startID) == 0 )
821             {
822                 fileno++;
823                 if(fileno >= MAX_FILE_NUMBER){
824                     /*stop scanner*/
825                     n = 0;
826                 }
827
828             }
829         }

おわかりいただけただろうか、MAX_FILE_NUMBER(=25000)を超えたらそこで終了なんやねこれ。 要するに初期スキャンはファイル25000上限が仕様ちゅーこと、なんやこれクソが。

前に書いた「だいたい28000個くらい処理したところで止まる」という現象は

だったからその数字を出したのね、そしてその件数でソースをgrepしてもヒットしないからデバッグ開始したんだけど。

でもDETAILSテーブルからファイルでない余計なものを除いた件数を出力すると

$ sqlite3 files.db
SQLite version 3.21.0 2017-10-24 18:55:49
Enter ".help" for usage hints.
sqlite> select count(*) from DETAILS where SIZE is not null;
25000
sqlite>

はい25000で完全一致、お疲れ様でした。

いやー萎えるせっかく久々にdebugする気になったのにこのオチはねーよなジャンプの10週打ち切り漫画の最終回 *1より酷い…

果たしてこのクソコードは何がやりたかったのかは推測するしかないんだけども

あたりなのかなぁ、最新版のコード(1.2.1)ではこの制限は削除されてるのでまず前者は無さそうではあるんだけども。

ということでNETGEAR 6300v2に付属するバージョンのReadyDLNA=MiniDLNA(ReadyMedia)で25000ファイルを超える音楽・写真・動画ファイルが扱えないのは仕様ですってことやねん。 今すぐ窓から投げ捨てろシリーズ入りやなぁこれ。

*1:ちなみにワイはジャンプ買ったこと一度も無かったりする

[やきう] 今永炎上

肩の故障がルーズショルダーだって報道と前回当番時のクソフォームと投球テンポで察していたんだけど、これ今永は去年の日シリ登板のレベルには二度と復活せんなこれ。

そんで点取れない元凶の筒香の不調やけどこれも怪我隠し定期やろね。しれっと去年の日シリの前に首のヘルニアがなんて記事でたし、オープン戦のフォーム改造も首の痛み対策や。2016年二冠王レベルの復活はないな。

ちゅーことで優勝なんぞ監督代えてもカタツムリでまた一回り世代交代するまで無理や、やはり98年日本一の時の波留「また38年後お会いしましょう」の呪いか…

つーか同じチーム(セだと阪神)にまったく勝てないってのさすがにスコアラー代えた方がええのでは。 その辺が広島との差なんだよな、今日も巨人山口メンバーの死球の時に乱闘仕掛けにいってあの弱メンタルにきっちり精神攻撃与えとったしな。

[今すぐ窓から投げ捨てろ] NETGEAR R6300v2のReadyDLNAには25000ファイル上限がある件

さっき原因を突き止めたNETGEAR R6300v2の25000ファイル制限はどうもMiniDLNA(ReadyMedia)のメインストリームには一度も入ったことの無い制限で、機種独自の制限だなこれ。

んで25000をキーワードに検索かけるとオーストラリアのよくわからん掲示板に インデックス処理が遅いしクエリの結果が大き過ぎるなんて話がヒットして、途中でファイル数25000を上限にすれば改善するなんて発言がみつかるので、この男がサポートにバグレポとして上げた可能性が、余計な事を…

という事で性能問題ということなんだが、これまず起動時スキャンが遅い問題は

という実装が悪いよねとしかいいようが無いよな、そして上限を設けるにしても

ちゅーこと。

そんでクエリ結果が大き過ぎるってーのも

ReadyDLNA: R6300v2
  | 
  +- Music
      |
      +- Album
      |
      +- All Music (25000) ← これは酷い
   |
      +- Artist
   |
      ...

の「All Music」みたいなノードを用意して、全ての曲をプレイリストとして返すようクエリを許す設計が悪いとしかいいようが無いですわ。 全曲垂れ流したいなんてのはこの層で実装するんじゃなくてプレイリストを順に再生していけば良いし、そもそも25000曲全てを曲名の文字コード順に並べたリスト *1なんて100%不要と言い切れる。 これ実装してる人あまり音楽好きではないのでは…

少なくともMusicBeeのUPnP/DLNAサポートプラグインはそんな頭の悪い設計のノードは持ってない。

MusicBee
  | 
  +- 音楽
      |
      +- Album Artists
      |
      +- Albums
   |   |
      |    +- #
      |    |
      |    +- A
      |    |
      |    ...
      +- Artists
   |
      +- Composers
   |
      +- Genres
   |
      +- Years

どのサブノードも件数が爆発しないよう、アルバムタイトルなら頭文字を入れるとか対策してあるよね、つーかこうなってないと聴きたい曲なんか探せないよ。

ということでMiniDLNA(ReadyMedia)は音楽好きにはお勧めできませんって結論やな、まぁ自分で改造するならコード量も少なく読みやすい部類でRDBMS使ってるから変更に対して柔軟ではあるんだけど…

*1:アルバムとベストアルバムで同じ曲が重複して入ってるケースだと同じ曲が連続して流れるわけでな、New OrderやDavid Bowieみたいなリマスタ商法アーティストの曲が延々と…

2018/05/03(Thu)

[プログラミング] ひたすら MiniDLNA(ReadyMedia) のコードにダメ出ししていく(その1)

早々にバグ(というかクソ仕様)が特定できてしまい不完全燃焼、それならガソリン撒いて跡形も無く燃やしてしまえばいいと、所沢山賊隊こと西武打線がワイに囁いている。 BGMは Big Black/Keroseneでお願いします。

あ、NETGEAR R6300v2同梱版はもう窓から投げ捨てたので今回からはgit repositoryの最新版ベースで。

@ディレクトリの走査には必要が無い限り scandir(3) でなく opendir(3) + readdir(3)を使う

まず指定ディレクトリ以下の子要素を走査するに scandir(3) を使ってるのって微妙だねー感がある。

721 static void
722 ScanDirectory(const char *dir, const char *parent, media_types dir_types)
723 {
...
735                         n = scandir(dir, &namelist, filter_avp, alphasort);

そもそも scandir(3) って

  • 第1引数で指定されたディレクトリ以下に存在するファイル一覧を
  • 第3引数で指定された条件にマッチするファイルのみ選んで
  • 第4引数で指定されたソート関数を使って並び替え
  • malloc(3)によってメモリ確保した配列に↑の結果を格納し、第2引数にポインタをセットして返却する

という関数なので

  • 同一ディレクトリの下に
  • 大量にファイルが置かれる

みたいなユースケースには不向き、そして音楽ファイルや画像ファイル置き場なんてのは正にそういう使い方なのよね。

なので"Mini"を自称するのであれば opendir(3) + readdir(3) を使うべきとこかな。

サンプルコードだけど、↓は

#include <dirent.h>
#include <stdio.h>
#include <stdlib.h>
int
main(void)
{
	struct dirent **ent;
	int len, i;

	len = scandir(".", &ent, NULL, NULL);
	if (len < 0)
		abort();
	for (i = 0; i < len; ++i) {
		if (ent[i]->d_name[0] == '.')
			continue;
		printf("%s\n", ent[i]->d_name);
	}
	free(ent);
	exit(EXIT_SUCCESS);
}

↓と書き直せる。

#include <dirent.h>
#include <stdio.h>
#include <stdlib.h>
int
main(void)
{
	DIR *d;
	struct dirent *ent;

	d = opendir(".");
	if (d == NULL)
		abort();
	while ((ent = readdir(d)) != NULL) {
		if (ent->d_name[0] == '.')
			continue;
		printf("%s\n", ent->d_name);
	}
	exit(EXIT_SUCCESS);
}

そもそも scandir(3) の中身って opendir(3) + readdir(3) だし、参照 src/lib/libc/gen/scandir.c

 85 int
 86 scandir(const char *dirname, struct dirent ***namelist,
 87     int (*selectfn)(const struct dirent *),
 88     int (*dcomp)(const void *, const void *))
 89 {
...
 97         if ((dirp = opendir(dirname)) == NULL)
 98                 return -1;
...
108         while ((d = readdir(dirp)) != NULL) {

よって第4引数で指定するソート関数を使う用事が無いなら無駄でしかない。

元コードはソート関数として alphasort(3) を指定してるので、敢えてscandir(3)使った可能性は残ってる。alphasort(3) は

  • ファイル名をstrcoll(3)で比較し
  • qsort(3)でソートする

という関数でGNU拡張、strcoll(3)はLC_COLLATEつまり文字照合順序に応じてソートを実行する。

よって迂闊に書き換えてしまうと処理の順序が変わってしまうんだけど、ここで冷静に脳味噌を働かせて考えると

  • ソートしたいのはフォルダ名でなくID3タグ中の曲名やアーティスト名なので、ここで alphasort(3) 指定しても全く意味が無い
  • もちろん「Brows Folders」の検索結果はフォルダ名でソートされてる必要があるけど、それは今ソートする必要はゼロ
  • SQLite3 という RDBMS を使うことによるメリットを存分に享受して出力時に ORDER BY 使えばいいだけの話

ということ、やっぱりクソコードやんけ!

そもそも英語圏だとアーティスト名のソートって定冠詞(The)を無視する必要があったり、日本語圏なら漢字コード順でなく読み仮名順でのソートしないとならない。 なのでその辺ちゃんと実装しようと思ったら大変なんですよな。

@もし移植性に目を瞑れるのであれば opendir(3) + readdir(3) ではなく fts(3) の使用も検討する

readdir(3) は opendir(3) したディレクトリ直下の情報しか帰ってこないので、サブディレクトリを再帰的に処理したければコードも再帰を使って書く必要があり、ヘタクソなコードだとスタックオーバーフローの温床になりがち。

もっと高級言語だと指定したディレクトリ以下を再帰的に走査し要素をイテレーターとして返すAPIがありユーザーが再帰コード書く必要が無い、例としては

あたりがそう、実はCでも標準ではないんだけど似たような関数があってそれが表題の fts(3)なんよね。

簡単なコード例を示すと

#include <sys/types.h>
#include <sys/stat.h>
#include <fts.h>
int
main(void)
{
	FTS *d;
	FTSENT *ent;
	char *paths[] = { ".", NULL };

	d = fts_open(paths, FTS_PHYSICAL, NULL);
	if (d == NULL)
		abort();
	while ((ent = fts_read(d))) {
		if (ent->fts_level == FTS_ROOTLEVEL)
			continue;
		switch (ent->fts_info) {
		case FTS_D:
		case FTS_F:
			printf("%s\n",  ent->fts_name);
			break;
		}
	}
	exit(EXIT_SUCCESS);
}

みたいに使う、メリットとしては

  • バグの温床になりがちな再帰コードを書かずとよい
  • FTSENT には struct dirent だけでは取れないような情報も定義されてるので 改めて lstat(2) 呼ばんでも済む(TOCTTO対策だけは忘れずに)
  • fts_open(3) の第3引数には scandir(3) みたいにソート関数も指定できる

という至れり尽くせりの高機能、その分だけ仕様が複雑ではあるんだけど使うだけの価値はある *1

それに移植性は無いといえども

で動きゃ充分よね、AIXとHP-UXは隅っこで腹筋してろWindowsは好きな寿司ネタでも書いとけ。

@fts(3) ほど高機能である必要が無いのであれば、移植性のある nftw(3) を使う

つーか本当は POSIX に fts(3) によく似た nftw(3) - New File Tree Walkという関数があって、広い移植性を考えたらそっちを使う方が好ましい。

#include <sys/stat.h>
#include <sys/types.h>
#include <ftw.h>
#include <stdio.h>
#include <unistd.h>

static int
do_walk(const char *path, const struct stat *st,
    int flag, struct FTW *ftwp)
{
	switch (flag) {
	case FTW_D:
	case FTW_F:
		printf("%s\n", path);
		break;
	}
	return 0;
}

int
main(void)
{
	nftw(".", &do_walk, sysconf(_SC_OPEN_MAX), 0);
	exit(EXIT_SUCCESS);
}

ただし

  • コールバック関数(コード例だとdo_walk)にユーザー定義のパラメータ渡す方法が無い
  • グローバル変数あるいはスレッドローカル変数を使わざるをえない

というデザインが単純にダメ *2、そもそも中身 fts(3) の wrapper として実装されてるから使う意味が…

元々 fts(3) は POSIX 入る予定だったんだけどねぇ、マニュアルにも恨み言が書いてあったり。

 STANDARDS
      The fts utility was expected to be included in the IEEE Std 1003.1-1988
      (``POSIX.1'') revision.  But twenty years later, it still was not
      included in the IEEE Std 1003.1-2008 (``POSIX.1'') revision.

というか流し読みしてるとPOSIX:2008に入ってるように空目するからSTANDARDSに書くべくじゃねぇなこれ、CAVEANTSなりBUGSに移せや。

@特に理由が無いならシンボリックリンクは安全の為に無視する

次に気になったのが以下のコード

629 static int
630 filter_type(scan_filter *d)
631 {
632 #if HAVE_STRUCT_DIRENT_D_TYPE
633         return ( (d->d_type == DT_DIR) ||
634                  (d->d_type == DT_LNK) ||
635                  (d->d_type == DT_UNKNOWN)
636                 );
637 #else
638         return 1;
639 #endif
640 }

scandir(3) のフィルタに指定されてる関数のひとつなんだけど、DT_LNK つまりシンボリックリンクを除外してないのよね。

シンボリックリンクを無視するしないはあくまでアプリケーション側のポリシーの問題だけど、いったん受け入れると決めたなら注意深く実装しないと

  • シンボリックリンクが循環参照していると再帰しながらディレクトリを走査する関数なんかでスタックオーバーフローが発生する可能性がある
  • 本来はお見せしてはならないファイルにシンボリックリンクを貼るなどの方法で重要情報が流出する可能性がある

という可用性や機密性に関するセキュリティ問題が容易く発生するんですわ。

正直なところ音楽ファイルにシンボリックリンクが意味を持つとは思えないよね、ファイルシステム層でなくアプリケーション層つまりプレイリスト機能でいくらでもエイリアス切れるわけでな。 なのでこれはどんな判断だと思うけど、記事のネタ的にはシンボリックリンク扱う方が含蓄あるので以下その線で話を進める。

@どうやってシンボリックリンクの循環参照を検出するか

MiniDLNAでは↓のコードで実現してるらしい

482 int
483 resolve_unknown_type(const char * path, media_types dir_type)
484 {
...
490     if( lstat(path, &entry) == 0 )
491     {
492         if( S_ISLNK(entry.st_mode) )
493         {
494             if( (len = readlink(path, str_buf, PATH_MAX-1)) > 0 )
495             {
496                 str_buf[len] = '\0';
497                 //DEBUG DPRINTF(E_DEBUG, L_GENERAL, "Checking for recursive symbolic link: %s (%s)\n", path, str_buf);
498                 if( strncmp(path, str_buf, strlen(str_buf)) == 0 )
499                 {
500                     DPRINTF(E_DEBUG, L_GENERAL, "Ignoring recursive symbolic link: %s (%s)\n", path, str_buf);
501                     return type;

うーんこの、全然できとらんやんけ!

$ mkdir foo
$ cd foo
$ ln -sf . foo
$ ls -all foo
lrwxr-xr-x  1 tnozaki  tnozaki  1 May  2 12:00 foor -> .

みたいな循環参照は検出できない、検出可能なのは

$ ln -sf foo foo
$ ls -all foo
lrwxr-xr-x  1 tnozaki  tnozaki  1 May  2 13:14 foo -> foo

みたいにシンボリックリンクが自分自身を参照してるケースだけだわ、これ実体がはじめから存在しないから循環参照ではなく参照切れのケースですわ。

実はさっきの fts(3)を使うメリットにはもう一つあって、循環参照あるいは参照切れを回避しつつリンク先を処理するのも簡単なのよね。オプションに

     FTS_LOGICAL     This option causes the fts routines to return FTSENT
                     structures for the targets of symbolic links instead of
                     the symbolic links themselves.  If this option is set,
                     the only symbolic links for which FTSENT structures are
                     returned to the application are those referencing non-
                     existent files.  Either FTS_LOGICAL or FTS_PHYSICAL must
                     be provided to the fts_open() function.

をセットすれば簡単にチェックできる、fts_read(3) で返ってきた FTSENT の fts_info フィールドに

  • FTS_SLNONE … シンボリックリンクの参照先が存在しない
  • FTS_DC … ディレクトリが循環参照している

がついてたらビンゴなので、エラーなり警告なり無視なりすればいいだけ。

#include <sys/stat.h>
#include <sys/types.h>
#include <fts.h>
#include <stdio.h>

int
main(void)
{
	FTS *d;
	FTSENT *ent;
	char *paths[] = { ".", NULL };

	d = fts_open(paths, FTS_LOGICAL, NULL);
	if (d == NULL)
		abort();
	while ((ent = fts_read(d))) {
		if (ent->fts_level == FTS_ROOTLEVEL)
			continue;
		switch (ent->fts_info) {
		case FTS_SLNONE:
			fprintf(stderr, "ignore symlink without target: (%s)\n", ent->fts_name);
			break;
		case FTS_DC:
			fprintf(stderr, "ignore recursive symlink: (%s)\n", ent->fts_name);
			break;
		}
	}
	fts_close(d);
	exit(EXIT_SUCCESS);
}

これ ntfw(3) の方だと

  • リンク切れは FTW_SLN が帰ってくるので fts(3) と同様の処理ができる
  • 循環参照は安全の為に「完全に静かに無視」されるのでコールバック自体が呼ばれない
  • そのため↑の検出目的には使えない

ちゅーかんじ。

@次回

シンボリックリンク攻撃に対する注意を書こうかと思ってるけど、いつもの通り予定は未定。

*1:詳しい使い方は ls(1)find(1)の実装なんかで使われてるのでそっち読んでくだしあ。
*2:これでもftw(3)を廃止してのnftw(3)なんだよなぁ…

2018/05/04(Fri)

[やきう] 阪神のエサ

というか阪神の体力回復アイテムかなんかか横浜って

[やきう] イチロー

ついに引退かと思わせておいて今年の残りを調整に費やし来年は投手登録でくる可能性もあるよね。

ルースの安打記録はとっくに抜いてるし50歳まで毎年20勝すれば勝利数も余裕よな、大谷よりも先にルースの記録塗りかえるのはイチローやで。

漫画のMAJ○Rだと投手で右肩壊したら左投げにスイッチして左肩も壊したら打者転向とかやってたけど、リアルの場合老眼による動体視力の衰えは打者には致命的だけど、投手にはあまり関係ないので肩肘の消耗のない野手から投手転向は高齢でも成功する可能性はある。

いちおう去年マーリンズで投手温存の為に野手で登板して練習なしに143km/h出したし、来年まで投手調整すれば100マイル(=160km/h)くらいよゆーよゆー(マジキチスマイル)

2018/05/05(Sat)

[やきう] チーム打率 .225 ツツゴー

[プログラミング] ひたすら MiniDLNA(ReadyMedia) のコードにダメ出ししていく(その2)

前回の続き、そもそも UPnP/DLNA って認証/認可が存在しない上に通信も HTTP で暗号化も欠いてる時点でセキュリティ面にダメだしすんのもバカバカしいんですけどね(しろめ) *1、だんだんこんな機能がルーターに載ってる事自体が狂気なのではという気分に…

@シンボリックリンク攻撃とは何ぞや

気をとりなおして前回予告したとおりシンボリックリンク攻撃のお話、簡単に書くと

  • 本来はアクセスできない権限に設定されたファイルを奪取する、あるいは改竄することを目的とした攻撃
  • ↑のファイルのシンボリックリンクを攻撃者が書込権限を持つディレクトリに作成する
  • ↑を管理者権限など特権ユーザで動作するアプリケーションに読み込ませる
  • ↑の結果ファイルを攻撃者がゲットしたり破壊したりあるいは改竄によって誤動作を引き起こせれば攻撃成功

というシナリオ、特に httpd なんかは

  • 80や334443といった特権ポートをbind/listenするのに管理者権限で起動する
  • 共有サーバによるウェブホスティングサービスが広く普及している
  • /home/<username>/public_html のような一般ユーザが書き込み権限を持った公開ディレクトリが存在する
  • 通常ファイルでありさえすればbinary/octet-streamでまず確実に GET できる

という仕様からとっても狙われやすいのよね、Apache だと

  • FollowSymLinks
  • SymLinksIfOwnerMatch

そして Nginx だと

  • disable_symlinks

あたりの設定が意味するところを理解しとらんエンジニャーが設定するとまず事故る *2、数年前に ロリポップがやらかした時にちょっとだけ意識高まったけども。

ゆうてMiniDLNAにシンボリックリンク攻撃を試みたところでたかが音楽・動画・画像ファイルしか奪えないし被害も無いでしょっても

  • 最近コソコソしてる配偶者、ホームディレクトリに怪しげな動画ファイルあるけど、パーミッションが600で再生できない
  • せや、MiniDLNA のスキャン対象のディレクトリにシンボリックリンク貼って中身を確認すればええやんけ!

という家庭内からの攻撃も成り立つのよな、中身がただのエロ動画ならまだしもピーとかピーやったら結婚生活終わるで…慰謝料払えるん…

@必要も無いのにアプリケーションを特権プロセスとして動かさない

まずシンボリックリンク攻撃に関してのよくある誤解としては

  • リンク先ファイルの権限は関係なく
  • 書き込み権限のあるディレクトリさえあれば
  • 自由にシンボリックリンクが作れる

というシンボリックリンクの仕様自体が危険なわけではないということですな、リンク先のファイルに権限が無ければ攻撃のしようが無いわけで。 そこを勘違いした Windows が mklink コマンドの実行に長らく管理者権限を必要とする愚を冒してたのを改め、Windows10 から一般ユーザ権限に引き下げたあたりから察しよう。

失敗の本質は「シンボリックリンク攻撃の対象となるアプリケーションが不必要に高い権限で動いてる」のが悪いのよね。

今回コード監査している MiniDLNA において特権プロセスが必要になるのはざっと眺めた限り

  1. UPnP ではデバイスがネットワークに参加した時に
  2. SSDP(Simple Service Discovery Protocol) でマルチキャストして存在を知らせる
  3. Linuxでは rtnetlink(7) を使ってルーティングテーブルの変更を監視し
  4. ネットワークが変更されたらまた1.から繰り返し

という部分かねぇ、SSDP(UDP) 自体そして

  • デバイスの Web ページ
  • SOAP over HTTP
  • メディアのストリーミング

のための HTTP(TCP) に必要な socket(2) は bind(2)するポートが

  • SSDP … 1900
  • HTTP … 8200(デバイスによって違いこれは MiniDLNA のもの、特に規格では定められてないっぽい)

の非特権ポートとなので特権プロセスである必要は無さそうなんだけどね。

あと rtnetlink(7) だけど移植性無いよね…ってのはさておいて

351 int
352 OpenAndConfMonitorSocket(void)
353 {
354 #ifdef HAVE_NETLINK
355         struct sockaddr_nl addr;
356         int s;
357         int ret;
358
359         s = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
360         if (s < 0)
361         {
362                 perror("couldn't open NETLINK_ROUTE socket");
363                 return -1;
364         }

ここ SOCK_RAW 指定してるけど、非特権プロセスでもエラー起きないしマニュアルの例は SOCK_DGRAM になってるしで、もしかして区別ないんですかねこれ…と思ったら

       Netlink is a datagram-oriented service.  Both SOCK_RAW and SOCK_DGRAM
       are valid values for socket_type.  However, the netlink protocol does
       not distinguish between datagram and raw sockets.

その通りだった、じゃあ特権プロセスである必要無いのでは…

@特権は不要になったら捨てる、あるいは隔離する

これ、MiniDLNA がセキュリティに無頓着なわけではなく特権は破棄してるからセーフ。

 558         uid_t uid = 0;
 559         gid_t gid = 0;
...
 915                 case 'u':
...
 919                                 uid = strtoul(argv[i], &string, 0);
...
 934                 case 'g':
...
 938                                 gid = strtoul(argv[i], &string, 0);
...
1066         if (gid > 0 && setgid(gid) == -1)
...
1070         if (uid > 0 && setuid(uid) == -1)
...

しかしサンプルの起動スクリプト

 1 #!/bin/sh
 2
 3 # chkconfig: 345 99 10
 4 # description: Startup/shutdown script for MiniDLNA daemon
...
30 case "$1" in
31 start)  log_daemon_msg "Starting minidlna" "minidlna"
32         start-stop-daemon --start --quiet --pidfile $PIDFILE --startas $MINIDLNA -- $ARGS $LSBNAMES
33         log_end_msg $?
34         ;;

およびデフォルトの設定ファイル

7 # specify the user account name or uid to run as
8 #user=jmaggard

を使うと root で起動したまんまなんでやっぱりアウトなんやな(チャレンジ制度)。

どうしても特権プロセスが必要なのであれば、その部分だけ別のプロセスとして切り離す 特権分離(privilege separation)というアプリ設計の採用も検討すべきですわね、特権分離についてはまたいずれ書く。

それと特権についてもう少しだけ話をしておくと、伝統的なUNIXにおいては

  • 特権プロセス … 実効ユーザIDが0
  • 非特権プロセス … 実効ユーザIDが0以外

という白か黒かしかない世界で、一般ユーザが動かすけど特権が必要なプログラム(foo)には

# chmod ug+s foo
# ls -all foo
-rwsr-sr-x  1 root  wheel  10 May  6 00:00 foo

として実行ファイルに setuid/setgid ビットを与えるのがしきたりだったんだけど、例えば X Server のような複雑なプログラムにまで与えてしまう *3ケースがあって脆弱性あったときの被害が甚大だったのよね。

なのでよりモダンなUNIX like OSにおいては実行ファイルの setuid/setgid ビットを撲滅すべく

というAPIで特権を細かく制限できるようになってるので、こいつらの利用も検討したほうがいいのかもね。よし全員 *4居るなセーフ!

@次回

だいぶ書く気失せてきたけど、シンボリックリンク攻撃の回避はいうほど簡単でない原因の TOCTTOU(Time Of Check To Time Of Use) という競合問題ついてのお話の予定。

*1:こんなガバガバなもん作るエンジニャー村が送信可能化権ガーとかフィルタリングガーとかもう本来の意味での確信犯なんやな…
*2:つーか .htaccess とか AllowOverride みたいな無駄と有害を混ぜ合わせて20年間コトコト煮込んだ Apache をまだ使ってるの?
*3:なのでXwrapperが生まれたりしてね…
*4:某Nにも systrace(4)というのがあったんだけど、それ自体が権限昇格に悪用できてしまったので消された。