Unify native output buffer sizing for OCK and OpenSSL in SymmetricCipher

オープン
  • 何か問題がありますか？
#2,092 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 2 日以内に返信

まだ誰も着手していません。

評価

難易度
5/5
見積もり時間
1週間以上
初心者へのやさしさ
20/100
issue の種類
リファクタリング
明瞭さ
おおむね明確
活発さ
活発
技術スタック
c, java
領域
cryptography

調査の方向性

Start with SymmetricCipher.java (getOutputSizeForOCK, renamed to getNativeOutputSize in PR #2009) and the checkRange checks in the OpenSSL SymmetricCipher.c (len + blockSize for update/decryptFinal, len + 2*blockSize - 1 for encryptFinal). Trace the OCKC native update/doFinal paths to find where the temporary buffer is sized and add the equivalent range check there, then build the stress loop the issue describes (all modes, padding on/off, lengths 0..~100, multiple threads). Done means one buffer calculation shared by both backends and a clean, ideally crashing-revealing, stress run.

索引モデルが issue の本文から書いたものです。

説明

bug

Summary

SymmetricCipher allocates a temporary buffer for each native update/doFinal call. OCK and OpenSSL use different sizing today, and OCK's is based on an unproven assumption. We want a single calculation for both backends, backed by each library's real maximum write. Failing to provide a big enough buffer will cause an overlay.

Background

  • On main, getOutputSizeForOCK(inputLen) returns (bufferedCount + inputLen) rounded up to the next block, plus OCK_ENCRYPTION_RESIDUE (16). The 16 comes from the comment "GSKit code adds 16 to the input buffer length for every update and provides a 16 byte buffer for the Final". No source in the repo proves it is the maximum OCK writes.
  • PR #2009 adds the OpenSSL backend for AES. Its native code (SymmetricCipher.c) checks the real array length against the worst-case write before every EVP_* call (checkRange) and throws if too small:
    • update / decryptFinal: len + blockSize
    • encryptFinal: len + 2*blockSize - 1
      This change was only applied to the OpenSSL backend even though the same change is expected on both backends. The method getOutputSizeForOCK was also renamed to getNativeOutputSize.

Findings

  • OCKC's real maximum write per mode cannot be determined from documentation or source. Do not plan on "finding" it. OpenSSL is in use so likely it has the same constraint / buffer requirements. Whatever buffer is used should be applied to both OCKC and OpenSSL backends.
  • The fix must be safe and verified empirically with a test.

Proposed work

  1. Add the same range defensive check to the OCKC backend and remove the buffer resize code from the Java driving class SymmetricCipher method getOutputSizeForOCK or getNativeOutputSize that was changed in the mentioned PR.
  2. Stress test to try to force an overwrite, run in a loop on OCK :
    • All modes (ECB, CBC, CFB, CFB8, OFB, CTR; z fast path where available), padding and no padding.
    • update and doFinal, with buffered partial blocks, input lengths 0..~100 and around block boundaries, small and large caller buffers.
    • Many iterations and multiple threads, watching for crashes, heap corruption, or wrong output.
    • Limitation: a Java array overflow does not always crash, so a clean run is evidence, not proof.
  3. Apply a new buffer size common for both backends and validate using developed test. We can keep range checks in for a period of time and later remove them for performance reasons given that Java already should perform caluclations of correct size buffers and they woudl become redundant. Consider keeping the C range checks in if performance is not impacted.
主要言語
Java
スター
16
フォーク
26
平均マージ
3日 7時間
マージ済み PR（30日）
108

環境構築

このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

IBM/OpenJCEPlus のほかの issue

IBM/OpenJCEPlus の issue をすべて見る

似ている issue

Java の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。