ゴミコード

Posts filed under ゴミコード

javaでHTTPS通信の実装をするという記事を適当に書くのはやめるべき

Filed in ゴミコード, コンピュータ, プログラム

いろんなブログでjavaを使ったHTTPSクライアントの実装について書かれている。
だが適当な記事が多すぎて困る。RFCを読んでいるのか疑問に思うことがほとんどだ。というかRFCの存在を知らないのかもしれない。

このブログではリンクは基本的に張らないことにしているが特別に張っておこう。
こちらはIPAが提供してくれているHTTP オーバー TLSを規定したRFC2818の和訳である。
http://www.ipa.go.jp/security/rfc/RFC2818JA.html#31
こちらはRFC9110の日本語訳である。(RFC9110はRFC2818の修正版)
https://tex2e.github.io/rfc-translater/html/rfc9110.html

javaはセキュリティライブラリをJSSEとしてコアパッケージと分離することでセキュリティ機能をサードパーティによるカスタマイズが可能となるような思想で設計されている。そのためSSL/TLSについても柔軟なカスタマイズが可能である。

逆に言うとしっかりと理解していないとセキュリティホールを作りこんでしまうことになる。世のブログはこれに該当してしまう記事が異常に多い。さらに悪いのはそれが正解であるかのように広がってしまっていることだ。
エンジニアの中にもそういったブログの記事が正解だと思っている人が思いのほか多いという事実があっては笑ってもいられない。

よく紹介されているHTTPSクライアントの実装はこうである。

TrustManager[] tm = {
      new X509TrustManager() {
           public X509Certificate[] getAcceptedIssuers() {
               return null;
           }
           public void checkClientTrusted(X509Certificate[] xc,String type) {
           }
           public void checkServerTrusted(X509Certificate[] xc,String type) {
           }
     }
};
SSLContext ctx = SSLContext.getInstance("TLS");
ctx.init(null, tm, new SecureRandom());

URL url = new URL("https://hogehoge/foo/bar");
HttpsURLConnection conn = (HttpsURLConnection)url.openConnection();
conn.setDefaultHostnameVerifier(new HostnameVerifier() {
      public boolean verify(String hostname, SSLSession session) {return true;}
    }
);

  以下略

まあ通信できるでしょう。しかしこれはやるべきことを全くやっていません。
このコードには簡単に以下の2つの問題がある。

  • 証明書が信頼できるかをチェックしないトラストマネージャを無条件で使っている(1~13行目)
  • サーバ名の検証を無条件でOKにしている(17~20行目)

1点目の問題では、証明書が送られてきさえすればそれが偽造されたものであろうと何だろうと通信OKとなってしまう。
2点目の問題では、信頼できる認証局から発行された証明書でありさえすればそれが目的のサーバのものでなくても通信OKとなってしまう。

1点目の偽造証明書にOKを出すのがダメなことは直感的にも分かるでしょう。
では、2点目はなぜダメなのでしょう。

たとえば、こんなケースがある。

上記のような実装をされたウェブブラウザであなたが銀行のサイトに接続したつもりでいます。
しかし、あなたが使っているDNSがレコード書き換えの攻撃を受けており実際には別のサイト(攻撃者が管理するサイト)に繋がってしまいました。
ですが、そのサーバには第三者機関から発行された証明書が設定されていました。
ブラウザは通信相手は期待したサーバであると判断して通信を続行してしまいます。
あなたは銀行のサイトだと思ってIDとパスワードを入力してしまいます。
攻撃者はあなたのIDとパスワードをまんまと手に入れてしまうわけです。

本来は偽のサーバに設定されている証明書のサーバ名とあなたが接続しようとしたサーバ名は異なっているので、接続は拒否されるべきである。ですが上記のコードではこれがなされません。さらに悪いことに上記のコードでは無条件で処理がされてしまうためユーザがそんなことが起こっていることを知る術もない。

では正しく処理するにはどう実装したらよいのだろうか。こうです。

URL url = new URL("https://hogehoge/foo/bar");
HttpsURLConnection conn = (HttpsURLConnection)url.openConnection();
以下略

非常に簡単です。なんのことはない。javaは特に設定を変えない限りデフォルトで正しく動作をするようになっている。
無理やりその動作を歪めてしまう必要はないのである。

自己署名証明書を設定したサーバに対してはこのままで通信をしようとしても通信はできない。
それは当然である。なぜなら自己署名証明書は信用できる証明書ではないからだ。
どうにもならないのかといえばそんなことはない。

自己署名証明書を信用できるという前提でクライアント側のトラストストアにインポートすればよいのだ。
これにより通信は可能になる。
ただ、これはあくまで普通の行為ではないということを認識したうえで行う必要がある。

セキュリティとはかくも面倒くさいことであるが、こういったことが守られてようやく実用レベルの安全性をもったネットワークというのが実現できるのである。守られていてもクラッカーとはイタチごっこであるのに守られていない場合に安全であるなど言えっこないのである。

見ろ!!コードがゴミのようだ!! part3

Filed in ゴミコード, プログラム

さて、、次のjavaのコードを見て貰いましょう。
mainでほぼ同じ処理を2回しています。
違いは呼び出しているメソッドがtest1,test2と違うだけ。
そしてtest1,test2の違いはメソッド内の最初の行だけです。
結果を受け取った後に戻り値を更新しています。

その後、fooXの値を2行ずつ出力していますが何が出力されるか分かりますか?

public class Test {
	public static void main(String[] args) {
		Test test = new Test();

		Bar bar1 = new Bar(100, "abc");
		Foo foo1 = new Foo(bar1);
		Bar ret1 = test.test1(foo1);
		ret1.i = 10000;
		ret1.s = "ABC";
		System.out.println(foo1.bar.i); //1回目処理の出力1
		System.out.println(foo1.bar.s); //1回目処理の出力2
		
		Bar bar2 = new Bar(100, "abc");
		Foo foo2 = new Foo(bar2);
		Bar ret2 = test.test2(foo2);
		ret2.i = 10000;
		ret2.s = "ABC";
		System.out.println(foo2.bar.i); //2回目処理の出力1
		System.out.println(foo2.bar.s); //2回目処理の出力2
	}

	public Bar test1(Foo foo) {
		Bar bar = new Bar(foo.bar.i, foo.bar.s);
		// ここでいろんな処理
		return bar;
	}

	public Bar test2(Foo foo) {
		Bar bar = foo.bar;
		// ここでいろんな処理
		return bar;
	}
}

class Foo {
	Bar bar;
	public Foo(Bar bar) {
		this.bar = bar;
	}
}

class Bar {
	int i;
	String s;
	public Bar(int i, String s) {
		this.i = i;
		this.s = s;
	}
}

答えはこうです。

100
abc
10000
ABC

コードのどこを見てもfooXの更新なんてないですね。でも値は変わります。
test1とtest2で違いが出る理由が分からなかった人はもう少し頑張りましょう。
そして分かった人もtest2のようなメソッドは書かないようにしましょう。

test2メソッドを書いた人はjavaを理解していないか、
保守を全く考えていないか、
小さいプログラムしか書いたことが無いかの三択でしょうか。。

エンハンスやバグ修正などでfooXの更新場所を探さないといけなくなったらどうするんでしょうか。。
test2のようなのがあると機械的に探すのがほぼ不可能です。
こんなじゃTDDだってできないだろうに。。

続・見ろ!!コードがゴミのようだ!!

Filed in ゴミコード, プログラム

せっかくなので先日書いたゴミサンプルをC++で書き直してみた。

#include <iostream>
#include <string>

class SampleClass {
public:
	int i;
	std::string s;
};

class Test {
	public:
		SampleClass* methodA(SampleClass* a, SampleClass* b);
};

int main(int argc, char* argv[])
{
	SampleClass a = SampleClass();
	a.i = 1;
	a.s = "a";

	SampleClass b = SampleClass();
	b.i = 2;
	b.s = "b";

	Test t = Test();
	
	std::cout << "== before ==" << std::endl;
	std::cout << "a= " << a.i << ":" << a.s << std::endl;
	std::cout << "b= " << b.i << ":" << b.s << std::endl;
	
	SampleClass* c = t.methodA(&a, &b);

	std::cout << "== after ==" << std::endl;
	std::cout << "a= " << a.i << ":" << a.s << std::endl;
	std::cout << "b= " << b.i << ":" << b.s << std::endl;
	std::cout << "c= " << c->i << ":" << c->s << std::endl;
	
	delete(c);
	return 0;
}

SampleClass* Test::methodA(SampleClass *a, SampleClass *b) {
	a->i = 11;
	a->s = "A";
	
	SampleClass b2 = SampleClass();
	b2.i = 12;
	b2.s = "B";
	b = &b2;

	SampleClass* c = new SampleClass();
	*c = b2;
	return c;
};

同じように作ったのだから結果は当然同じ。
ただ、戻り値cはnewしたものなので最後にdeleteしている。
この辺はJavaではそんなに意識しないことだろう。

== before ==
a= 1:a
b= 2:b
== after ==
a= 11:A
b= 2:b
c= 12:B

しかし超久しぶりにC++を書いたのでアクセス修飾子ってどうするんだっけ?と迷ってしまった。。
昔は当然と思っていたのに宣言と定義を分けるというのもかなり煩わしく感じた。
“System.out.println”と”std::cout <<" を比較すると"std::cout <<"などはプログラムを知らない人には謎の暗号にしか思えまい。。 久しぶりにC++のコードを書いて思ったのはポインタを使いたいとは思わないが、やはり概念は知っておくべきということ。 Javaで参照がどうとか勉強するよりもC++やCでポインタを学んだ方が近道な気がしている。 Javaにはガベッジコレクタがあると言ってもメモリの無駄な消費はメモリの断片化が起こってガベッジコレクタの頻発などパフォーマンスに悪影響です。インスタンスの無駄な生成自体もコストがかかりますしね。やはりJavaであってもいざというときはメモリを意識したプログラムを書くべきです。 こんなコード書いてもいいこと何もないですよ。 [java] SomeClass a = new SomeClass(); SomeClass b = new SomeClass(); // このインスタンスが作られてすぐ捨てられる b = a; [/java]

見ろ!!コードがゴミのようだ!!

Filed in ゴミコード, プログラム

まずは以下のjavaのコードをみてもらおう。

class SampleClass {
  public int i;
  public String s;
}

public class Test {
  static public void main(String[] args) {
    Test test = new Test();


    SampleClass a = new SampleClass();
    a.i = 1;
    a.s = "a";
    
    SampleClass b = new SampleClass();
    b.i = 2;
    b.s = "b";

    System.out.println("== before ===");
    System.out.println("a= " + a.i + ":" + a.s);
    System.out.println("b= " + b.i + ":" + b.s);

    SampleClass c = test.antiPattern(a,b);

    System.out.println("== after ===");
    System.out.println("a= " + a.i + ":" + a.s);
    System.out.println("b= " + b.i + ":" + b.s);
    System.out.println("c= " + c.i + ":" + c.s);
  }

  public SampleClass antiPattern(SampleClass a, SampleClass b) {
    a.i = 11;
    a.s = "A";

    SampleClass b2 = new SampleClass();
    b2.i = 12;
    b2.s = "B";
    b = b2;

    SampleClass c = new SampleClass();
    c = b2;
    return c;
  }
}

mainメソッドでantiPatternメソッドを呼ぶ前後で変数値を出力している訳だが何が出力されるか分かるだろうか。

結果はこうである。

== before ===
a= 1:a
b= 2:b
== after ===
a= 11:A
b= 2:b
c= 12:B

変数cは良いとしても、aやbまでもmainメソッドだけを見ていては決して分かりようがない結果が出力されるのである。さらに言うとantiPatternメソッドを見てみると直感的にはbも更新されてよさそうな気がするのだが実際はそうはならない。この辺はjavaのメソッドはポインタの値渡しであるという言語仕様によるものである。

しかしこれは言語仕様が許しているからと言って書いてはいけないコードの代表格と言えるだろう。mainメソッドを読んでいるのにantiPatternメソッドがどうなっているかまで意識しなくてはいけない。

上の例ではantiPatternメソッドが実行ステップにして10行足らずで完結しているのでまだよいが、antiPatternメソッドも同じように別のメソッドAを呼びだしてAの中でも渡した変数の中身が変わって返ってくるとなったらどうだろうか。さらにAの中でもBを呼び出し、Bの中でもCを呼び出し・・・。
結局全部のコードを理解しないと高々数十行のmainメソッドすら読み解くことができない。
これでは保守性が悪くてどうしようもない。
これをゴミコードと呼ばずにいられるだろうか。いや、いられまい(反語)。

さて、突然こんなことを書いたのは仕事でこんなコードを見たからである。
実際はもっと処理があるが、ポイントだけ抜粋した。

public class Y {
  public void methodA() {
    ・・・
    XListGenerator generator = new XListGenerator(a,b,c);
    List<X> xList = new ArrayList<X>();
    ・・・
    List<Something> somethingList = new ArrayList<Something>();
  ・・・
    generator.set(xList, somethingList);
  }
}

クラス名が悪い、変数名が悪いとか言いたいわけではない。
methodAが何をやっているか分かるだろうか。
「generatorに2つのlistを設定している。」と読みとるのが自然ではないだろうか。

しかしgenerator.set(list,somethingList)の中を見て愕然とした。以下のような内容なのであった。

   public set(List<X> xList, List<Something> somethingList) {
       for(i=0; i<somethingList.size(); i++) {
          String s= somethingList.get(i).getString();
          xList.get(i).setS(s);
       }
   }

なんなんでしょうかこれ。

完全に依存関係があるんだしメソッドを分けたって百害あって一利なしだ。setの主体もgeneratorなのかなんなのかよく分からない。
これでいいじゃない。

public class Y {
  public void methodA() {
    ・・・
    List<X> xList = new ArrayList<X>();
    ・・・
    List<Something> somethingList = new ArrayList<Something>();
  ・・・
    for(i=0; i<somethingList.size(); i++) {
       String s= somethingList.get(i).getString();
       xList.get(i).setS(s);
    }
  }
}

どうしてもgeneratorを使いたいなら、なるべく原型を残したとしてこうだ。

public class Y {
  public void methodA() {
    ・・・
  XListGenerator generator = new XListGenerator(a,b,c);
    ・・・
    List<Something> somethingList = new ArrayList<Something>();
  ・・・
    List<X> xList = generator.generate(somethingList);
  }
}

プログラムは動けばよい!というのは自分一人で作っている場合に限るのである。
こんなコードを読まなければいけない他人の身にもなってコードは書くべきである。