From 0ffd3658dcdc21a6c56d234cf2a6008487dcfaa7 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sun, 23 Jul 2023 13:18:31 -0700 Subject: [PATCH] Use a stub fopen implementation when OPENSSL_NO_FILESYSTEM is set Detecting errors (i.e. fs-less platforms using fs-only APIs) at compile time is generally preferable to doing so at runtime, so https://boringssl-review.googlesource.com/c/boringssl/+/61726 opted to remove the APIs altogether on applicable targets. However, Trusty uses rust-openssl somewhere and rust-openssl binds a bunch of filesystem-dependent APIs unconditionally. To keep that working, switch to a stub fopen when OPENSSL_NO_FILESYSTEM is set. We effectively model a platform where the filesystem "exists", but is empty. Upstream OpenSSL similarly has OPENSSL_NO_STDIO still define the file BIO (unlike the socket BIO, which is excluded), but in a stub form. As part of this, I've gone ahead and resolved one of the Trusty TODOs. It does produce a duplicate symbol with [1], but things seem to link fine in treehugger. In case it does break, I've bumped BORINGSSL_API_VERSION, so we can go in and condition it if needed. [1] https://android.googlesource.com/trusty/lib/+/refs/heads/main/lib/openssl-stubs/bio.c Bug: 629 Change-Id: I4f20d872a7cde863d21c78090f270b77b03545fa Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/61925 Commit-Queue: Bob Beck Auto-Submit: David Benjamin Reviewed-by: Bob Beck --- crypto/bio/file.c | 23 ++++++++++------------- crypto/conf/conf.c | 2 -- crypto/x509/by_dir.c | 4 ---- crypto/x509/by_file.c | 3 --- crypto/x509/x509_d2.c | 3 --- include/openssl/base.h | 2 +- include/openssl/bio.h | 4 ---- include/openssl/conf.h | 2 -- include/openssl/ssl.h | 8 -------- include/openssl/x509.h | 6 ------ ssl/ssl_file.cc | 2 -- ssl/ssl_x509.cc | 2 -- 12 files changed, 11 insertions(+), 50 deletions(-) diff --git a/crypto/bio/file.c b/crypto/bio/file.c index f0801ba9e..dd11e5030 100644 --- a/crypto/bio/file.c +++ b/crypto/bio/file.c @@ -73,10 +73,6 @@ #include -// TODO(crbug.com/boringssl/629): Remove this in favor of the more fine-grained -// OPENSSL_NO_FILESYSTEM ifdef. -#if !defined(OPENSSL_TRUSTY) - #include #include #include @@ -92,11 +88,19 @@ #define BIO_FP_APPEND 0x08 #if !defined(OPENSSL_NO_FILESYSTEM) +#define fopen_if_available fopen +#else +static FILE *fopen_if_available(const char *path, const char *mode) { + errno = ENOENT; + return NULL; +} +#endif + BIO *BIO_new_file(const char *filename, const char *mode) { BIO *ret; FILE *file; - file = fopen(filename, mode); + file = fopen_if_available(filename, mode); if (file == NULL) { OPENSSL_PUT_SYSTEM_ERROR(); @@ -117,7 +121,6 @@ BIO *BIO_new_file(const char *filename, const char *mode) { return ret; } -#endif // !OPENSSL_NO_FILESYSTEM BIO *BIO_new_fp(FILE *stream, int close_flag) { BIO *ret = BIO_new(BIO_s_file()); @@ -197,7 +200,6 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr) { b->ptr = ptr; b->init = 1; break; -#if !defined(OPENSSL_NO_FILESYSTEM) case BIO_C_SET_FILENAME: file_free(b); b->shutdown = (int)num & BIO_CLOSE; @@ -219,7 +221,7 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr) { ret = 0; break; } - fp = fopen(ptr, mode); + fp = fopen_if_available(ptr, mode); if (fp == NULL) { OPENSSL_PUT_SYSTEM_ERROR(); ERR_add_error_data(5, "fopen('", ptr, "','", mode, "')"); @@ -230,7 +232,6 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr) { b->ptr = fp; b->init = 1; break; -#endif // !OPENSSL_NO_FILESYSTEM case BIO_C_GET_FILE_PTR: // the ptr parameter is actually a FILE ** in this case. if (ptr != NULL) { @@ -290,7 +291,6 @@ int BIO_set_fp(BIO *bio, FILE *file, int close_flag) { return (int)BIO_ctrl(bio, BIO_C_SET_FILE_PTR, close_flag, (char *)file); } -#if !defined(OPENSSL_NO_FILESYSTEM) int BIO_read_filename(BIO *bio, const char *filename) { return (int)BIO_ctrl(bio, BIO_C_SET_FILENAME, BIO_CLOSE | BIO_FP_READ, (char *)filename); @@ -311,12 +311,9 @@ int BIO_rw_filename(BIO *bio, const char *filename) { BIO_CLOSE | BIO_FP_READ | BIO_FP_WRITE, (char *)filename); } -#endif // !OPENSSL_NO_FILESYSTEM long BIO_tell(BIO *bio) { return BIO_ctrl(bio, BIO_C_FILE_TELL, 0, NULL); } long BIO_seek(BIO *bio, long offset) { return BIO_ctrl(bio, BIO_C_FILE_SEEK, offset, NULL); } - -#endif // OPENSSL_TRUSTY diff --git a/crypto/conf/conf.c b/crypto/conf/conf.c index 18bec19aa..ca950d628 100644 --- a/crypto/conf/conf.c +++ b/crypto/conf/conf.c @@ -585,7 +585,6 @@ err: return 0; } -#if !defined(OPENSSL_NO_FILESYSTEM) int NCONF_load(CONF *conf, const char *filename, long *out_error_line) { BIO *in = BIO_new_file(filename, "rb"); int ret; @@ -600,7 +599,6 @@ int NCONF_load(CONF *conf, const char *filename, long *out_error_line) { return ret; } -#endif // !OPENSSL_NO_FILESYSTEM int CONF_parse_list(const char *list, char sep, int remove_whitespace, int (*list_cb)(const char *elem, size_t len, void *usr), diff --git a/crypto/x509/by_dir.c b/crypto/x509/by_dir.c index 95a69bf0d..57b3a8351 100644 --- a/crypto/x509/by_dir.c +++ b/crypto/x509/by_dir.c @@ -64,8 +64,6 @@ #include #include -#if !defined(OPENSSL_NO_FILESYSTEM) - #include "../internal.h" #include "internal.h" @@ -402,5 +400,3 @@ finish: BUF_MEM_free(b); return ok; } - -#endif // OPENSSL_NO_FILESYSTEM diff --git a/crypto/x509/by_file.c b/crypto/x509/by_file.c index 33bd97819..2c83137b1 100644 --- a/crypto/x509/by_file.c +++ b/crypto/x509/by_file.c @@ -62,7 +62,6 @@ #include "internal.h" -#if !defined(OPENSSL_NO_FILESYSTEM) static int by_file_ctrl(X509_LOOKUP *ctx, int cmd, const char *argc, long argl, char **ret); @@ -278,5 +277,3 @@ err: sk_X509_INFO_pop_free(inf, X509_INFO_free); return count; } - -#endif // !OPENSSL_NO_FILESYSTEM diff --git a/crypto/x509/x509_d2.c b/crypto/x509/x509_d2.c index 7db1d2f34..269f56fef 100644 --- a/crypto/x509/x509_d2.c +++ b/crypto/x509/x509_d2.c @@ -57,7 +57,6 @@ #include #include -#if !defined(OPENSSL_NO_FILESYSTEM) int X509_STORE_set_default_paths(X509_STORE *ctx) { X509_LOOKUP *lookup; @@ -107,5 +106,3 @@ int X509_STORE_load_locations(X509_STORE *ctx, const char *file, } return 1; } - -#endif // !OPENSSL_NO_FILESYSTEM diff --git a/include/openssl/base.h b/include/openssl/base.h index 38d64742a..87ffe214b 100644 --- a/include/openssl/base.h +++ b/include/openssl/base.h @@ -108,7 +108,7 @@ extern "C" { // A consumer may use this symbol in the preprocessor to temporarily build // against multiple revisions of BoringSSL at the same time. It is not // recommended to do so for longer than is necessary. -#define BORINGSSL_API_VERSION 24 +#define BORINGSSL_API_VERSION 25 #if defined(BORINGSSL_SHARED_LIBRARY) diff --git a/include/openssl/bio.h b/include/openssl/bio.h index ce139cccf..707a4b155 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -470,11 +470,9 @@ OPENSSL_EXPORT int BIO_get_fd(BIO *bio, int *out_fd); // BIO_s_file returns a BIO_METHOD that wraps a |FILE|. OPENSSL_EXPORT const BIO_METHOD *BIO_s_file(void); -#if !defined(OPENSSL_NO_FILESYSTEM) // BIO_new_file creates a file BIO by opening |filename| with the given mode. // See the |fopen| manual page for details of the mode argument. OPENSSL_EXPORT BIO *BIO_new_file(const char *filename, const char *mode); -#endif // BIO_new_fp creates a new file BIO that wraps the given |FILE|. If // |close_flag| is |BIO_CLOSE|, then |fclose| will be called on |stream| when @@ -490,7 +488,6 @@ OPENSSL_EXPORT int BIO_get_fp(BIO *bio, FILE **out_file); // success and zero otherwise. OPENSSL_EXPORT int BIO_set_fp(BIO *bio, FILE *file, int close_flag); -#if !defined(OPENSSL_NO_FILESYSTEM) // BIO_read_filename opens |filename| for reading and sets the result as the // |FILE| for |bio|. It returns one on success and zero otherwise. The |FILE| // will be closed when |bio| is freed. @@ -510,7 +507,6 @@ OPENSSL_EXPORT int BIO_append_filename(BIO *bio, const char *filename); // as the |FILE| for |bio|. It returns one on success and zero otherwise. The // |FILE| will be closed when |bio| is freed. OPENSSL_EXPORT int BIO_rw_filename(BIO *bio, const char *filename); -#endif // OPENSSL_NO_FILESYSTEM // BIO_tell returns the file offset of |bio|, or a negative number on error or // if |bio| does not support the operation. diff --git a/include/openssl/conf.h b/include/openssl/conf.h index f217e980f..c9027c1db 100644 --- a/include/openssl/conf.h +++ b/include/openssl/conf.h @@ -99,14 +99,12 @@ OPENSSL_EXPORT CONF *NCONF_new(void *method); // NCONF_free frees all the data owned by |conf| and then |conf| itself. OPENSSL_EXPORT void NCONF_free(CONF *conf); -#if !defined(OPENSSL_NO_FILESYSTEM) // NCONF_load parses the file named |filename| and adds the values found to // |conf|. It returns one on success and zero on error. In the event of an // error, if |out_error_line| is not NULL, |*out_error_line| is set to the // number of the line that contained the error. OPENSSL_EXPORT int NCONF_load(CONF *conf, const char *filename, long *out_error_line); -#endif // NCONF_load_bio acts like |NCONF_load| but reads from |bio| rather than from // a named file. diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 4aba1051a..6f35e6bce 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -1195,7 +1195,6 @@ OPENSSL_EXPORT int SSL_use_RSAPrivateKey_ASN1(SSL *ssl, const uint8_t *der, #define SSL_FILETYPE_PEM 1 #define SSL_FILETYPE_ASN1 2 -#if !defined(OPENSSL_NO_FILESYSTEM) OPENSSL_EXPORT int SSL_CTX_use_RSAPrivateKey_file(SSL_CTX *ctx, const char *file, int type); @@ -1218,7 +1217,6 @@ OPENSSL_EXPORT int SSL_use_PrivateKey_file(SSL *ssl, const char *file, // success and zero on failure. OPENSSL_EXPORT int SSL_CTX_use_certificate_chain_file(SSL_CTX *ctx, const char *file); -#endif // !OPENSSL_NO_FILESYSTEM // SSL_CTX_set_default_passwd_cb sets the password callback for PEM-based // convenience functions called on |ctx|. @@ -2653,7 +2651,6 @@ OPENSSL_EXPORT void SSL_CTX_set_cert_store(SSL_CTX *ctx, X509_STORE *store); // SSL_CTX_get_cert_store returns |ctx|'s certificate store. OPENSSL_EXPORT X509_STORE *SSL_CTX_get_cert_store(const SSL_CTX *ctx); -#if !defined(OPENSSL_NO_FILESYSTEM) // SSL_CTX_set_default_verify_paths loads the OpenSSL system-default trust // anchors into |ctx|'s store. It returns one on success and zero on failure. OPENSSL_EXPORT int SSL_CTX_set_default_verify_paths(SSL_CTX *ctx); @@ -2670,7 +2667,6 @@ OPENSSL_EXPORT int SSL_CTX_set_default_verify_paths(SSL_CTX *ctx); OPENSSL_EXPORT int SSL_CTX_load_verify_locations(SSL_CTX *ctx, const char *ca_file, const char *ca_dir); -#endif // !OPENSSL_NO_FILESYSTEM // SSL_get_verify_result returns the result of certificate verification. It is // either |X509_V_OK| or a |X509_V_ERR_*| value. @@ -2832,24 +2828,20 @@ OPENSSL_EXPORT int SSL_add_client_CA(SSL *ssl, X509 *x509); // ownership of |x509|. OPENSSL_EXPORT int SSL_CTX_add_client_CA(SSL_CTX *ctx, X509 *x509); -#if !defined(OPENSSL_NO_FILESYSTEM) // SSL_load_client_CA_file opens |file| and reads PEM-encoded certificates from // it. It returns a newly-allocated stack of the certificate subjects or NULL // on error. Duplicates in |file| are ignored. OPENSSL_EXPORT STACK_OF(X509_NAME) *SSL_load_client_CA_file(const char *file); -#endif // SSL_dup_CA_list makes a deep copy of |list|. It returns the new list on // success or NULL on allocation error. OPENSSL_EXPORT STACK_OF(X509_NAME) *SSL_dup_CA_list(STACK_OF(X509_NAME) *list); -#if !defined(OPENSSL_NO_FILESYSTEM) // SSL_add_file_cert_subjects_to_stack behaves like |SSL_load_client_CA_file| // but appends the result to |out|. It returns one on success or zero on // error. OPENSSL_EXPORT int SSL_add_file_cert_subjects_to_stack(STACK_OF(X509_NAME) *out, const char *file); -#endif // SSL_add_bio_cert_subjects_to_stack behaves like // |SSL_add_file_cert_subjects_to_stack| but reads from |bio|. diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 7ce29180f..033445795 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -2852,10 +2852,8 @@ OPENSSL_EXPORT X509 *X509_STORE_CTX_get0_cert(X509_STORE_CTX *ctx); OPENSSL_EXPORT X509_LOOKUP *X509_STORE_add_lookup(X509_STORE *v, X509_LOOKUP_METHOD *m); -#if !defined(OPENSSL_NO_FILESYSTEM) OPENSSL_EXPORT X509_LOOKUP_METHOD *X509_LOOKUP_hash_dir(void); OPENSSL_EXPORT X509_LOOKUP_METHOD *X509_LOOKUP_file(void); -#endif OPENSSL_EXPORT int X509_STORE_add_cert(X509_STORE *ctx, X509 *x); OPENSSL_EXPORT int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x); @@ -2866,14 +2864,12 @@ OPENSSL_EXPORT int X509_STORE_get_by_subject(X509_STORE_CTX *vs, int type, OPENSSL_EXPORT int X509_LOOKUP_ctrl(X509_LOOKUP *ctx, int cmd, const char *argc, long argl, char **ret); -#if !defined(OPENSSL_NO_FILESYSTEM) OPENSSL_EXPORT int X509_load_cert_file(X509_LOOKUP *ctx, const char *file, int type); OPENSSL_EXPORT int X509_load_crl_file(X509_LOOKUP *ctx, const char *file, int type); OPENSSL_EXPORT int X509_load_cert_crl_file(X509_LOOKUP *ctx, const char *file, int type); -#endif OPENSSL_EXPORT X509_LOOKUP *X509_LOOKUP_new(X509_LOOKUP_METHOD *method); OPENSSL_EXPORT void X509_LOOKUP_free(X509_LOOKUP *ctx); @@ -2882,11 +2878,9 @@ OPENSSL_EXPORT int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type, X509_NAME *name, X509_OBJECT *ret); OPENSSL_EXPORT int X509_LOOKUP_shutdown(X509_LOOKUP *ctx); -#if !defined(OPENSSL_NO_FILESYSTEM) OPENSSL_EXPORT int X509_STORE_load_locations(X509_STORE *ctx, const char *file, const char *dir); OPENSSL_EXPORT int X509_STORE_set_default_paths(X509_STORE *ctx); -#endif OPENSSL_EXPORT int X509_STORE_CTX_get_error(X509_STORE_CTX *ctx); OPENSSL_EXPORT void X509_STORE_CTX_set_error(X509_STORE_CTX *ctx, int s); OPENSSL_EXPORT int X509_STORE_CTX_get_error_depth(X509_STORE_CTX *ctx); diff --git a/ssl/ssl_file.cc b/ssl/ssl_file.cc index 0071153ce..2b596b20a 100644 --- a/ssl/ssl_file.cc +++ b/ssl/ssl_file.cc @@ -203,7 +203,6 @@ int SSL_add_bio_cert_subjects_to_stack(STACK_OF(X509_NAME) *out, BIO *bio) { return add_bio_cert_subjects_to_stack(out, bio, /*allow_empty=*/true); } -#if !defined(OPENSSL_NO_FILESYSTEM) STACK_OF(X509_NAME) *SSL_load_client_CA_file(const char *file) { bssl::UniquePtr in(BIO_new_file(file, "r")); if (in == nullptr) { @@ -545,7 +544,6 @@ end: BIO_free(in); return ret; } -#endif // !OPENSSL_NO_FILESYSTEM void SSL_CTX_set_default_passwd_cb(SSL_CTX *ctx, pem_password_cb *cb) { ctx->default_passwd_callback = cb; diff --git a/ssl/ssl_x509.cc b/ssl/ssl_x509.cc index 5514cf203..e4b3775b3 100644 --- a/ssl/ssl_x509.cc +++ b/ssl/ssl_x509.cc @@ -695,7 +695,6 @@ void SSL_CTX_set_verify_depth(SSL_CTX *ctx, int depth) { X509_VERIFY_PARAM_set_depth(ctx->param, depth); } -#if !defined(OPENSSL_NO_FILESYSTEM) int SSL_CTX_set_default_verify_paths(SSL_CTX *ctx) { check_ssl_ctx_x509_method(ctx); return X509_STORE_set_default_paths(ctx->cert_store); @@ -706,7 +705,6 @@ int SSL_CTX_load_verify_locations(SSL_CTX *ctx, const char *ca_file, check_ssl_ctx_x509_method(ctx); return X509_STORE_load_locations(ctx->cert_store, ca_file, ca_dir); } -#endif // !OPENSSL_NO_FILESYSTEM long SSL_get_verify_result(const SSL *ssl) { check_ssl_x509_method(ssl);