OSSで不具合発見？他言語の実装はどうなっている？

はじめに

ビットキーで bitkey platform を開発しています @otakakot です。

bitkey platform の機能開発において一部 Google Cloud の Identity Platform を利用しています。
その機能において Could not find expiry time from HTTP headers というエラーが発生し IDトークンの認証が通らないという事象が発生しました。
そのときの対応について振り返るとともに学びになったことがあったので本記事にて記録しておくことにします。

学び

他言語ライブラリの実装を確認せよ！

不具合の概要

Firebase(Identity Platform) が発行した IDトークンの署名を検証するためには公開鍵を取得する必要があります。
この公開鍵を取得する処理において不具合が発生して IDトークンの署名検証ができなくなりました。
今回の不具合は2点原因があったと考えます。

• Google Cloud 側の障害（詳細は未だ見つけられず）
  • ヘッダーフィールド Cache-Control には max-age が設定されているが、実際には private が設定されていた
• firebase-admin-go の実装バグ
  • max-age ヘッダーから値が取得できなかったときエラーが発生して IDトークンの署名検証が失敗する

※ firebase-admin-go の実装バグは v4.14.1 にて修正されています。

https://github.com/firebase/firebase-admin-go/releases/tag/v4.14.1

振り返り

この事象は社内の開発環境において発生しました。
幸いにも本番環境で発生することはありませんでした。
ログからすぐに firebase-admin-go の実装においてエラーが発生していることはわかりました。
そして、実装や Firebase の公式ドキュメントを確認するとIDトークン検証のために https://www.googleapis.com/robot/v1/metadata/x509/[e-mail] から公開鍵を取得しているということに辿り着きました。

ここで私は勘違いをします。
我々の環境に問題があると思い込み調査を行います。
手元の Mac から該当の Go コードを実装しても同様のエラーが再現できませんでした。
bitkey platform は自身のチームが管理する開発環境と社内のエンジニアが利用する開発環境と2つ抱えています。
このエラーは社内のエンジニアが利用する方の環境でしか発生しませんでした。
自身のチームが管理する開発環境では再現しませんでした。

bitkey platform は GKE にアプリケーションをデプロイして運用しています。
社内のエンジニアが利用するネットワーク内にて curl (wget) コマンドを用いて取得したところ本問題が再現できました。
Cache-Control を確認しリクエスト側に問題があるのかと思い、いろいろ試してみますが private が返ってきます。

原因が特定できずにいましたが、しばらくすると firebase-admin-go に以下の issue が 起票されていました。

VerifyIDTokenandCheckRevoked returning error: Could not find expiry time from HTTP headers

https://github.com/firebase/firebase-admin-go/issues/621

まさにこれです。
どうやら us-west でこの事象が発生しているとのことです。
確かに我々の環境でも問題が発生しているのは us-west リージョンでした。

issue のコメントを見ていると error を返すのではなく、 default 値を返すように修正している方がいらっしゃいました。
我々も他の方同様 fork してこのように対応することとしました。
開発環境は毎日お昼に定期デプロイを実施しているのでそこに合わせてマージ・リリースを実施しようと思っていましたが、リリース前に該当の環境でエラーが発生する操作を実施したところエラー発生しませんでした。
そのため、特に今回は修正せずリリースは見送ることにしました。

反省ポイントなのですが、Cache-Control が正しく返ってくるような方法を探すのではなくアプリ側の修正を早急に行うべきでした。^[1]
コードがエラーを返してしまうことには早い段階で気づいていたのですぐに一時対応はできたはずです。
仮にその変更に問題があったとしても戻すのは簡単ですし、不具合が生じている時間を短くすることに努めるべきでした。

しかし、私の中で修正に対してもやもやするところがありました。

修正を考える

場当たり的な修正としては fork したようにとりあえず default 値を適当に返すだけでよいと思います。
しかし、ちゃんと修正するとなるとこれでいいんだっけ？とものすごくもやもやしていました。

そんなもやもやを抱えて issue を見返しているとこんなことをおっしゃっている方がいました。

breaking Firebase client code written in Go. (I'm not sure about client code written in other languages.)

なるほど ... firebase-admin って他の言語もライブラリあるはずだな。
え、じゃあ他言語の実装を確認してみればいいのでは？

言語として過去に触ったことのあった TypeScript のライブラリを検索してみました。

https://github.com/firebase/firebase-admin-node

探し方としてはベタ書きされていそうなワードで検索をかけます。

今回は max-age とか https://www.googleapis.com/robot/v1/metadata/x509/[e-mail] がベタ書きされそうです。

そして今回問題となった箇所と似たようなロジックを発見しました。

https://github.com/firebase/firebase-admin-node/blob/v12.1.1/src/utils/jwt.ts#L146-L159

ただ、しばらく TypeScript を触っていなかったので自信はありませんでした。
そこで登場するのが ChatGPT です。
該当のコードに対して何をしているか質問して回答を得ました。

この結果を踏まえて、いま一度 Go の実装を確認します。
俯瞰で該当のコードを見返してみると max-age で検索した値を元に公開鍵をリフレッシュするための期限に設定していることに気づきます。
なら、Go のコードも TypeScript と同様にエラーにするのではなくリフレッシュするための期限を設定しないようにすればいいのでは？

そう思い、下記のようなコメントを issue に記載しました。

Why does it generate an error if max-age cannot be obtained? I think this value will be used later to determine if it should be refreshed or not. So why not return 0 if the value cannot be retrieved so that it is not cached?
Also, I checked the implementation of the node library (firebase-admin-node), and it seems that if the max-age value could not be obtained, an error is not generated, but the default value of 0 is set (i.e., not cached).

（私は英語が得意ではないの翻訳に頼って文章を生成しています。）

するとこれに対して以下のようにコメントが返ってきて、最終的に PR が作成されました。

I agree with the comments above, we should update the SDK to handle this case gracefully without throwing and continue the token verification.

https://github.com/firebase/firebase-admin-go/pull/623

ほかにも...

ほかにも気づいたこととしてライブラリ起因で不具合を見つけたとき、他言語ライブラリの issue を見に行くのはひとつ知見だなと気づきました。
今回の issue については我々がこの現象に気づいてからだいたい 5時間後 に起票されていました。
もしかしたら他言語ライブラリに同様の問題が起票されてもう少し早く気づけたかもしれません。

さらに、気づいた点としてみなさん結構無邪気に issue を立てているんだなと思いました。
私はこの事象が我々の環境のみで起こっているのだと思い込み調査を進めてしまいましたが issue を立てることで
「私も同じエラーで失敗している」や「あなたの環境だけじゃない？」など調査に役立つコメントがもらえるかもしれません。

おわりに

本記事はスプリントレトロスペクティブ「KPT」にて話したことがよい行動であるとFBをもらい執筆に至りました。日々の活動をこうして振り返ることができる環境に感謝です。これからも精進してまいります。

余談

私はこの不具合に対して、もともとあった findMaxAge() メソッドを使う側に対して以下のような修正を考えていました。

maxAge, err := findMaxAge(resp)
if err != nil {
-	return err
+	maxAge = time.Duration(0) * time.Second
}

プライベート関数なのに error を握り潰すのは違和感がありますが findMaxAge() の責務に対しては見つからなかったらエラーが発生するのはまあ納得がいくかなという感じです。

最終的に目的は達成できるのでどちらでも問題ないかなと思います。

脚注

1. max-age はオプショナルだったので、ない場合に動かなくなるのはクライアント側の問題でした。ただし、必須属性だった場合は返却してこないサーバー側の問題となります。どちらにしても自分たちが手を加えられるのはクライアントの実装なので、SLOなどで許容できなくなる場合はクライアントの実装を修正して対応する必要があります。