Low-level hash 'final' functions cannot fail.
The SHA-2 family has some exceptions, but they're all programmer errors
and should be documented as such. (Are the failure cases even
necessary?)
Change-Id: I00bd0a9450cff78d8caac479817fbd8d3de872b8
Reviewed-on: https://boringssl-review.googlesource.com/4953
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/sha/sha256.c b/crypto/sha/sha256.c
index 39ce52e..8276bbb 100644
--- a/crypto/sha/sha256.c
+++ b/crypto/sha/sha256.c
@@ -144,7 +144,10 @@
* to truncate to amount of bytes not divisible by 4. I bet not, but if it is,
* then default: case shall be extended. For reference. Idea behind separate
* cases for pre-defined lenghts is to let the compiler decide if it's
- * appropriate to unroll small loops. */
+ * appropriate to unroll small loops.
+ *
+ * TODO(davidben): The small |md_len| case is one of the few places a low-level
+ * hash 'final' function can fail. This should never happen. */
#define HASH_MAKE_STRING(c, s) \
do { \
uint32_t ll; \
diff --git a/crypto/sha/sha512.c b/crypto/sha/sha512.c
index b68d5a2..57c96ab 100644
--- a/crypto/sha/sha512.c
+++ b/crypto/sha/sha512.c
@@ -276,7 +276,9 @@
sha512_block_data_order(sha, p, 1);
- if (md == 0) {
+ if (md == NULL) {
+ /* TODO(davidben): This NULL check is absent in other low-level hash 'final'
+ * functions and is one of the few places one can fail. */
return 0;
}
@@ -312,6 +314,8 @@
break;
/* ... as well as make sure md_len is not abused. */
default:
+ /* TODO(davidben): This bad |md_len| case is one of the few places a
+ * low-level hash 'final' function can fail. This should never happen. */
return 0;
}
diff --git a/include/openssl/sha.h b/include/openssl/sha.h
index 0e37c45..ac2ab75 100644
--- a/include/openssl/sha.h
+++ b/include/openssl/sha.h
@@ -120,7 +120,8 @@
OPENSSL_EXPORT int SHA224_Update(SHA256_CTX *sha, const void *data, size_t len);
/* SHA224_Final adds the final padding to |sha| and writes the resulting digest
- * to |md|, which must have at least |SHA224_DIGEST_LENGTH| bytes of space. */
+ * to |md|, which must have at least |SHA224_DIGEST_LENGTH| bytes of space. It
+ * returns one on success and zero on programmer error. */
OPENSSL_EXPORT int SHA224_Final(uint8_t *md, SHA256_CTX *sha);
/* SHA224 writes the digest of |len| bytes from |data| to |out| and returns
@@ -144,7 +145,8 @@
OPENSSL_EXPORT int SHA256_Update(SHA256_CTX *sha, const void *data, size_t len);
/* SHA256_Final adds the final padding to |sha| and writes the resulting digest
- * to |md|, which must have at least |SHA256_DIGEST_LENGTH| bytes of space. */
+ * to |md|, which must have at least |SHA256_DIGEST_LENGTH| bytes of space. It
+ * returns one on success and zero on programmer error. */
OPENSSL_EXPORT int SHA256_Final(uint8_t *md, SHA256_CTX *sha);
/* SHA256 writes the digest of |len| bytes from |data| to |out| and returns
@@ -179,7 +181,8 @@
OPENSSL_EXPORT int SHA384_Update(SHA512_CTX *sha, const void *data, size_t len);
/* SHA384_Final adds the final padding to |sha| and writes the resulting digest
- * to |md|, which must have at least |SHA384_DIGEST_LENGTH| bytes of space. */
+ * to |md|, which must have at least |SHA384_DIGEST_LENGTH| bytes of space. It
+ * returns one on success and zero on programmer error. */
OPENSSL_EXPORT int SHA384_Final(uint8_t *md, SHA512_CTX *sha);
/* SHA384 writes the digest of |len| bytes from |data| to |out| and returns
@@ -207,7 +210,8 @@
OPENSSL_EXPORT int SHA512_Update(SHA512_CTX *sha, const void *data, size_t len);
/* SHA512_Final adds the final padding to |sha| and writes the resulting digest
- * to |md|, which must have at least |SHA512_DIGEST_LENGTH| bytes of space. */
+ * to |md|, which must have at least |SHA512_DIGEST_LENGTH| bytes of space. It
+ * returns one on success and zero on programmer error. */
OPENSSL_EXPORT int SHA512_Final(uint8_t *md, SHA512_CTX *sha);
/* SHA512 writes the digest of |len| bytes from |data| to |out| and returns
