Skip to content

Commit

Permalink
Get session hash pad input bytes after format seen
Browse files Browse the repository at this point in the history
The use of some raw pad bytes as part of the input to the session hash
is a feature of internal format 0, and therefore those bytes must be
taken only after format indicator has been handled.  This follows up
to commit 6774b32 to fix that.

* onetime:
  (Design Overview): Update where the session hash input bytes live.
  (PadSession._digest_source_length): This is just a length, so don't
    try to document where the initialization bytes are located.
  (PadSession._session_hash): Update documentation here too.
  (PadSession.prepare_for_encryption): Don't initialize hash here, and...
  (PadSession.prepare_for_decryption): ...update related comment here.
  (PadSession.convert): Don't initialize hash here either.
  (PadSession._make_inner_header, PadSession._handle_inner_header):
    Initialize session hash right after the fuzz source length bytes.

* check.sh: Update accordingly.
  • Loading branch information
kfogel committed Oct 29, 2016
1 parent ed6f171 commit 76c46de
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 75 deletions.
75 changes: 38 additions & 37 deletions check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -253,36 +253,36 @@ start_new_test "decryption should not shrink pad usage"
# ==> pad-records says length reverted to 27340!
../../onetime -C dot-onetime -e -p ../test-pad-1 \
-o tmp-ciphertext-b-1.onetime ../test-plaintext-b
if ! grep -q "<length>12846</length>" dot-onetime/pad-records; then
if ! grep -q "<length>12555</length>" dot-onetime/pad-records; then
echo ""
echo "ERROR: Pad usage length is not 12846 after encryption iteration 1."
echo "ERROR: Pad usage length is not 12555 after encryption iteration 1."
cat dot-onetime/pad-records
PASSED="no"
fi

../../onetime -C dot-onetime -e -p ../test-pad-1 \
-o tmp-ciphertext-b-2.onetime ../test-plaintext-b
if ! grep -q "<length>25023</length>" dot-onetime/pad-records; then
if ! grep -q "<length>25250</length>" dot-onetime/pad-records; then
echo ""
echo "ERROR: Pad usage length is not 25023 after encryption iteration 2."
echo "ERROR: Pad usage length is not 25250 after encryption iteration 2."
cat dot-onetime/pad-records
PASSED="no"
fi

../../onetime -C dot-onetime -e -p ../test-pad-1 \
-o tmp-ciphertext-b-3.onetime ../test-plaintext-b
if ! grep -q "<length>37273</length>" dot-onetime/pad-records; then
if ! grep -q "<length>37873</length>" dot-onetime/pad-records; then
echo ""
echo "ERROR: Pad usage length is not 37273 after encryption iteration 3."
echo "ERROR: Pad usage length is not 37873 after encryption iteration 3."
cat dot-onetime/pad-records
PASSED="no"
fi

../../onetime -C dot-onetime -d -p ../test-pad-1 \
-o tmp-plaintext-b-1 tmp-ciphertext-b-1.onetime
if ! grep -q "<length>37273</length>" dot-onetime/pad-records; then
if ! grep -q "<length>37873</length>" dot-onetime/pad-records; then
cat dot-onetime/pad-records
if grep -q "<length>12846</length>" dot-onetime/pad-records; then
if grep -q "<length>12555</length>" dot-onetime/pad-records; then
# Note that as long as everything is working, this case will not
# be triggered in normal test suite runs even if the length that
# *would* indicate the return of this bug has changed from 12523
Expand All @@ -294,7 +294,7 @@ if ! grep -q "<length>37273</length>" dot-onetime/pad-records; then
echo "ERROR: 'Decryption wrongly shrinks pad usage' bug is back."
else
echo ""
echo "ERROR: Pad usage length is not 37273 after decryption 1, but don't know why."
echo "ERROR: Pad usage length is not 37873 after decryption 1, but don't know why."
fi
PASSED="no"
fi
Expand Down Expand Up @@ -324,14 +324,14 @@ cp -a dot-onetime d-dot-onetime # separate decryption copy
../../onetime -C d-dot-onetime -d -p ../test-pad-1 \
-o tmp-plaintext-b-1 tmp-ciphertext-b-1

if ! grep -q "<length>12846</length>" e-dot-onetime/pad-records; then
if ! grep -q "<length>12555</length>" e-dot-onetime/pad-records; then
grep "<length>" e-dot-onetime/pad-records
echo ""
echo "ERROR: expected pad usage length of 12846 for encryption"
echo "ERROR: expected pad usage length of 12555 for encryption"
PASSED="no"
fi

if ! grep -q "<length>12846</length>" d-dot-onetime/pad-records; then
if ! grep -q "<length>12555</length>" d-dot-onetime/pad-records; then
if grep -q "<length>12265</length>" d-dot-onetime/pad-records; then
# Note that as long as everything is working, this case will not
# be triggered in normal test suite runs even if the length that
Expand All @@ -345,7 +345,7 @@ if ! grep -q "<length>12846</length>" d-dot-onetime/pad-records; then
else
grep "<length>" d-dot-onetime/pad-records
echo ""
echo "ERROR: pad usage length is not 12846 after decryption, for some new reason"
echo "ERROR: pad usage length is not 12555 after decryption, for some new reason"
fi
PASSED="no"
fi
Expand Down Expand Up @@ -646,11 +646,11 @@ then
PASSED="no"
fi

if ! grep -q "<length>588</length></used>" v1-dot-onetime/pad-records
if ! grep -q "<length>695</length></used>" v1-dot-onetime/pad-records
then
echo ""
cat v1-dot-onetime/pad-records
echo "ERROR: decoding v2 input affected length 588 in pad-records"
echo "ERROR: decoding v2 input affected length 695 in pad-records"
PASSED="no"
fi

Expand Down Expand Up @@ -716,10 +716,10 @@ then
PASSED="no"
fi

if ! grep -q "<length>588</length></used>" v1-dot-onetime/pad-records
if ! grep -q "<length>695</length></used>" v1-dot-onetime/pad-records
then
echo ""
echo "ERROR: decoding v2 input failed to use length 588 in pad-records"
echo "ERROR: decoding v2 input failed to use length 695 in pad-records"
cat v1-dot-onetime/pad-records
PASSED="no"
fi
Expand Down Expand Up @@ -792,10 +792,10 @@ then
PASSED="no"
fi

if ! grep -q "<length>556</length></used>" v1-dot-onetime/pad-records
if ! grep -q "<length>663</length></used>" v1-dot-onetime/pad-records
then
echo ""
echo "ERROR: decoding v2 input failed to add length 556 to pad-records"
echo "ERROR: decoding v2 input failed to add length 663 to pad-records"
cat v1-dot-onetime/pad-records
PASSED="no"
fi
Expand Down Expand Up @@ -914,18 +914,18 @@ then
fi

# Expect the new length on the 7th line, in second range of first entry.
if ! grep -q "<length>12552</length></used>" v1-dot-onetime/pad-records
if ! grep -q "<length>12659</length></used>" v1-dot-onetime/pad-records
then
echo ""
echo "ERROR: failed to insert new length 12552 into pad-records"
echo "ERROR: failed to insert new length 12659 into pad-records"
cat v1-dot-onetime/pad-records
PASSED="no"
elif grep -q "<length>12552</length></used>" v1-dot-onetime/pad-records && \
[ `grep -n "<length>12552</length></used>" v1-dot-onetime/pad-records \
elif grep -q "<length>12659</length></used>" v1-dot-onetime/pad-records && \
[ `grep -n "<length>12659</length></used>" v1-dot-onetime/pad-records \
| cut -d ":" -f 1` -ne 7 ]
then
echo ""
echo "ERROR: encoding mis-inserted new length 12552 into pad-records"
echo "ERROR: encoding mis-inserted new length 12659 into pad-records"
PASSED="no"
fi
check_result
Expand Down Expand Up @@ -961,12 +961,13 @@ start_new_test "tampered head fuzz is detected, but decryption succeeds"
../../onetime --test-mode \
--config=blank-dot-onetime -e -p ../test-pad-1 \
-o tmp-ciphertext-b-1 < ../test-plaintext-b 2>err.out
../zap tmp-ciphertext-b-1 284 ? 71
# In the base64-encoded ciphertext file, position 286 is 'v' (118).
../zap tmp-ciphertext-b-1 286 118 119 # tweak 'v' to 'w'
../../onetime --config=blank-dot-onetime -d -p ../test-pad-1 \
-o tmp-plaintext-b-1 < tmp-ciphertext-b-1 2>err.out
if ! grep -q "DigestMismatch: digest mismatch:" err.out || \
! grep -q " computed: 6be52f46065d00c652e54f8770d6bf072115ffd2a1e4f3bb95bd3a49ba83161d" err.out || \
! grep -q " received: d3d8a00fa588756733de1cf9f39a76ff58157d6e203c448b91d4a7b8e780f6fd" err.out
! grep -q " computed: fa258423e6c7559a529905a001d2e4c32ba322a73f5e867589025b1eeb91b220" err.out || \
! grep -q " received: 9581b039c3902390984fc98ef38f0ea1e44fb5a401cc0eb4a237b6777ffe0bb5" err.out
then
echo ""
echo "ERROR: did not see expected DigestMismatch error from tampered head fuzz"
Expand All @@ -990,8 +991,8 @@ start_new_test "tampering with ciphertext causes bzip decoder error"
## Encrypt message
../../onetime --config=blank-dot-onetime -e -p ../test-pad-1 \
-o tmp-ciphertext-b-1 < ../test-plaintext-b 2>err.out
# In the base64-encoded ciphertext file, position 8563 is 'd' (100).
../zap tmp-ciphertext-b-1 8563 100 101
# In the base64-encoded ciphertext file, position 8563 is 'h' (104).
../zap tmp-ciphertext-b-1 8563 104 105 # tweak 'h' to 'i'
../../onetime --config=blank-dot-onetime -d -p ../test-pad-1 \
< tmp-ciphertext-b-1 2>err.out
if ! grep -q "IOError: invalid data stream" err.out
Expand Down Expand Up @@ -1096,14 +1097,14 @@ start_new_test "tampering with message digest causes authentication error"
../../onetime --test-mode \
--config=blank-dot-onetime -e -p ../test-pad-1 \
-o tmp-ciphertext-a-1 < ../test-plaintext-a 2>err.out
# In the base64-encoded ciphertext file, position 626 is 'e' (101).
../zap tmp-ciphertext-a-1 626 101 102 # tweaking 'e' to 'f'
# In the base64-encoded ciphertext file, position 824 is 'a' (97).
../zap tmp-ciphertext-a-1 824 97 98 # tweaking 'a' to 'b'
../../onetime --config=blank-dot-onetime -d -p ../test-pad-1 \
-o tmp-plaintext-a-1 < tmp-ciphertext-a-1 2>err.out
if ! grep -q "DigestMismatch: digest mismatch:" err.out || \
! grep -q " computed: ec6478d952bf3b13bd43dc2dd689d789e5fb9f408138fb71672ff2194038b913" err.out || \
! grep -q " received: ec6478d952bf3b13bd43dc2dd689d788e5fb9f408138fb71672ff2194038b913" err.out
# here is where they differ ------------------------> ^
! grep -q " computed: b0b8c3001080d56ace90280f2cebc389e9b7df5a31064aaf1ca26f80a92f3889" err.out || \
! grep -q " received: b0b8c3001080d56ace90280f2cabc389e9b7df5a31064aaf1ca26f80a92f3889" err.out
# here is where they differ -------------------> ^
then
echo ""
echo "ERROR: did not see expected DigestMismatch error (30c7cd...)"
Expand Down Expand Up @@ -1133,11 +1134,11 @@ start_new_test "tampering with head fuzz causes authentication error"
../../onetime --config=blank-dot-onetime -d -p ../test-pad-1 \
-o tmp-plaintext-a-1 < tmp-ciphertext-a-1 2>err.out
if ! grep -q "DigestMismatch: digest mismatch:" err.out || \
! grep -q " computed: fdaa87d94780a35bd6c891e23ec4c1b10b6aa9d52f55684984dc4111f1a30c1b" err.out || \
! grep -q " received: ec6478d952bf3b13bd43dc2dd689d789e5fb9f408138fb71672ff2194038b913" err.out
! grep -q " computed: 7a177e60428451ac87eb9eb59c9b37e26b7061e5ef36ddf316333dc6baf821a8" err.out || \
! grep -q " received: b0b8c3001080d56ace90280f2cebc389e9b7df5a31064aaf1ca26f80a92f3889" err.out
then
echo ""
echo "ERROR: did not see expected DigestMismatch error (fdaa87d vs ec6478d)"
echo "ERROR: did not see expected DigestMismatch error (7a177e vs b0b8c3)"
cat err.out
PASSED="no"
fi
Expand Down
64 changes: 26 additions & 38 deletions onetime
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,12 @@ import random
# after the core data, there's some bookkeeping, all base64-encoded.
# Here's a diagram, with index increasing from left to right:
#
# RRRRFFHHTT******-------------------------------------DDDD*******
# FFHHTTRRRR******-------------------------------------DDDD*******
#
# The precise number of same characters in a row is not significant
# above, except as a rough guide to the relative lengths of the
# different sections. This is what the different sections mean:
#
# R == some bytes of raw pad input for the session hash
#
# These bytes are among the data fed to the session hash, so
# that the final message digest authenticates the pad as well
# as the plaintext. (The actual number of bytes used is
# PadSession._digest_source_length.)
#
# F == a few format indicator bytes:
#
# These tell OneTime what version of its internal format it
Expand All @@ -121,6 +114,13 @@ import random
# the number of bytes of tail fuzz (the concluding
# set of asterisks) that will be used.
#
# R == some bytes of raw pad input for the session hash
#
# These bytes are among the data fed to the session hash, so
# that the final message digest authenticates the pad as well
# as the plaintext. (The actual number of bytes used is
# PadSession._digest_source_length.)
#
# * == a random number of fuzz bytes (head fuzz or tail fuzz):
#
# A random number (derived from the pad -- see above) of
Expand Down Expand Up @@ -640,9 +640,8 @@ but it still needs to be a new object due to certain initializations)."""
# of a hexadecimal representation of the digest.
_digest_length = 32

# A stretch of pad bytes, starting at self._offset, are included in
# the source material for the digest computed in self._session_hash.
# This is how many bytes to use.
# Number of contiguous raw pad bytes to use as the source
# material for the digest computed in self._session_hash.
_digest_source_length = 32

def __init__(self, pad_path, config_area=None,
Expand Down Expand Up @@ -680,11 +679,7 @@ but it still needs to be a new object due to certain initializations)."""

# We compute a hash of the plaintext head fuzz + plaintext message
# and embed that hash into the encrypted text, for authentication
# of the overall encrypted message.
#
# Instead of initializing it here, we initialize it in the first
# call to self.convert(), so that any attempt to use the session
# hash before it's initialized will raise an error.
# of the overall encrypted message. See self._initialize_hash().
self._session_hash = None

# There are both head and tail fuzz, but we only need to remember
Expand Down Expand Up @@ -774,7 +769,6 @@ but it still needs to be a new object due to certain initializations)."""
if self._decrypting:
raise PadSession.OverPrepared(
"cannot prepare for both encryption and decryption")
self._initialize_hash()
self._head_buffer = self._make_inner_header()
self._encrypting = True

Expand All @@ -786,16 +780,12 @@ but it still needs to be a new object due to certain initializations)."""
if self._encrypting:
raise PadSession.OverPrepared(
"cannot prepare for both decryption and encryption")
# The fact that we don't call self._initialize_hash() nor
# self._handle_inner_header() here is an ugly asymmetry
# w.r.t. self.prepare_for_encryption(). The reason for it is that
# the decryption code flow is complicated by the need to handle
# remainder input, in a way that encryption is not. Notice how
# self._initialize_hash() and self._handle_inner_header() are
# called together in self.convert(). Those two calls really
# should be chained together in one routine, and this would be the
# routine for it to happen in. Maybe some day someone will be
# motivated enough to clean this up.
# The fact that we don't call self._handle_inner_header() here is
# an unfortunate asymmetry w.r.t. self.prepare_for_encryption().
# The reason for it is that the decryption code flow is
# complicated by the need to handle remainder input, in a way that
# encryption is not. This is why self._handle_inner_header() has
# to be called from self.convert().
self._decrypting = True

def set_offset(self, offset):
Expand Down Expand Up @@ -859,17 +849,13 @@ must be used for all subsequent calls with that instance.
# is likely to deliver string in chunks so small. So, saving
# it to solve later.

# The complement of these calls to self._initialize_hash()
# and self._handle_inner_header(string) is located, in
# the encryption case, in self._prepare_for_encryption(),
# which also prepares self._head_buffer for the first call
# to convert(). However, the decryption case is complicated
# by the need to return remainder information, in a way that
# the encryption case is not. This asymmetry is reflected
# in the code, unfortunately; ideally, these calls would be
# chained together in self.prepare_for_decryption(). It
# would be nice to clean this up some day.
self._initialize_hash()
# The complement of this call to self._handle_inner_header()
# is located in self._prepare_for_encryption() in the
# encryption case, which also prepares self._head_buffer for
# the first call to convert(). However, the decryption case
# is complicated by the need to return remainder
# information, in a way that the encryption case is not.
# This asymmetry is reflected in the code.
string, fuzz_remaining = self._handle_inner_header(string)
if string != "" and fuzz_remaining != 0:
raise PadSession.InnerFormat(
Expand Down Expand Up @@ -1245,6 +1231,7 @@ of random fuzz data generated herein."""
self._tail_fuzz_length, self._tail_fuzz_length_source_bytes \
= self._get_fuzz_length_from_pad(
self._default_fuzz_source_length, self._default_fuzz_source_modulo)
self._initialize_hash()
head_fuzz = self._make_fuzz(head_fuzz_length, is_head_fuzz=True)
result = chr(inner_header_format_version) \
+ chr(fuzz_source_length_indicator) \
Expand Down Expand Up @@ -1294,6 +1281,7 @@ of random fuzz data generated herein."""
= self._get_fuzz_length_from_pad(
self._default_fuzz_source_length,
self._default_fuzz_source_modulo)
self._initialize_hash()
# TODO: We're capturing these return values but then never
# using them, except for debugging. That seems odd. Check
# with later return of remaining amount to see if at least
Expand Down

0 comments on commit 76c46de

Please sign in to comment.