Skip to content

Commit 9360b0a

Browse files
authored
Add inline documentation for API contracts (#3267)
### Issues: Addresses P409219367 ### Description of changes: Adds clarifying comments to document existing API contracts that were reviewed and determined to be working as designed: - SSL session deserialization: document that session bytes are assumed non-malleable - BIO pair: document that concurrent free requires caller synchronization - BIO callbacks: document that callbacks must not free the BIO - X509 thread safety: add general thread-safety note to the public header By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
1 parent 462ca26 commit 9360b0a

5 files changed

Lines changed: 23 additions & 5 deletions

File tree

crypto/bio/pair.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ static int bio_new(BIO *bio) {
4242
return 1;
4343
}
4444

45+
// bio_destroy_pair unlinks the two halves of a BIO pair. Concurrent calls to
46+
// BIO_free on both halves from separate threads are the caller's responsibility
47+
// to synchronize. As in OpenSSL, the BIO pair API does not provide internal
48+
// locking.
4549
static void bio_destroy_pair(BIO *bio) {
4650
struct bio_bio_st *b = bio->ptr;
4751
BIO *peer_bio;

include/openssl/bio.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -286,12 +286,15 @@ OPENSSL_EXPORT uint64_t BIO_number_read(const BIO *bio);
286286
// |bio|.
287287
OPENSSL_EXPORT uint64_t BIO_number_written(const BIO *bio);
288288

289-
// BIO_set_callback_ex sets the |callback_ex| for |bio|.
289+
// BIO_set_callback_ex sets the |callback_ex| for |bio|. The callback is
290+
// invoked without incrementing the BIO's reference count, so a callback must
291+
// not call BIO_free on the BIO it is invoked on.
290292
OPENSSL_EXPORT void BIO_set_callback_ex(BIO *bio, BIO_callback_fn_ex callback_ex);
291293

292-
// BIO_set_callback sets the legacy |callback| for |bio|. When both |callback| and
293-
// |callback_ex| are set, |callback_ex| will be used. Added for compatibility with
294-
// existing applications.
294+
// BIO_set_callback sets the legacy |callback| for |bio|. The same reference
295+
// count note as |BIO_set_callback_ex| applies. When both |callback| and
296+
// |callback_ex| are set, |callback_ex| will be used. Added for compatibility
297+
// with existing applications.
295298
OPENSSL_EXPORT OPENSSL_DEPRECATED void BIO_set_callback(BIO *bio, BIO_callback_fn callback);
296299

297300
// BIO_set_callback_arg sets the callback |arg| for |bio|.

include/openssl/ssl.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2037,7 +2037,9 @@ OPENSSL_EXPORT int SSL_SESSION_to_bytes_for_ticket(const SSL_SESSION *in,
20372037
size_t *out_len);
20382038

20392039
// SSL_SESSION_from_bytes parses |in_len| bytes from |in| as an SSL_SESSION. It
2040-
// returns a newly-allocated |SSL_SESSION| on success or NULL on error.
2040+
// returns a newly-allocated |SSL_SESSION| on success or NULL on error. The
2041+
// caller is responsible for protecting the integrity of the serialized session
2042+
// data; no minimum-length validation is performed on individual fields.
20412043
OPENSSL_EXPORT SSL_SESSION *SSL_SESSION_from_bytes(const uint8_t *in,
20422044
size_t in_len,
20432045
const SSL_CTX *ctx);

include/openssl/x509.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ extern "C" {
4848
//
4949
// In the future, a replacement library will be available. Meanwhile, minimize
5050
// dependencies on this header where possible.
51+
//
52+
// Thread safety: Unlike other objects in this library, |X509| objects are not
53+
// yet safe for concurrent use by non-mutating functions (see
54+
// https://crbug.com/boringssl/407). Callers must not concurrently read and
55+
// mutate the same |X509| object without external synchronization.
5156

5257

5358
// Certificates.

ssl/ssl_asn1.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,10 @@ UniquePtr<SSL_SESSION> SSL_SESSION_parse(CBS *cbs,
497497
return nullptr;
498498
}
499499

500+
// Note: The secret field is validated only with an upper-bound check. A
501+
// minimum-length check is intentionally omitted because the serialized
502+
// session bytes are assumed to be non-malleable: the calling application is
503+
// responsible for protecting the integrity of session data.
500504
CBS session_id, secret;
501505
if (!CBS_get_asn1(&session, &session_id, CBS_ASN1_OCTETSTRING) ||
502506
CBS_len(&session_id) > SSL3_MAX_SSL_SESSION_ID_LENGTH ||

0 commit comments

Comments
 (0)