Move error-on-empty-cipherlist into ssl_create_cipher_list().
It's more consistent to have the helper function do the check that
its every caller already performs. This removes the error code
SSL_R_LIBRARY_HAS_NO_CIPHERS in favor of SSL_R_NO_CIPHER_MATCH.
Change-Id: I522239770dcb881d33d54616af386142ae41b29f
Reviewed-on: https://boringssl-review.googlesource.com/13964
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index 4b980b2..2bd50c8 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -73,7 +73,6 @@
SSL,160,INVALID_SSL_SESSION
SSL,161,INVALID_TICKET_KEYS_LENGTH
SSL,162,LENGTH_MISMATCH
-SSL,163,LIBRARY_HAS_NO_CIPHERS
SSL,164,MISSING_EXTENSION
SSL,258,MISSING_KEY_SHARE
SSL,165,MISSING_RSA_CERTIFICATE
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 62066b0..2b57244 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -4396,7 +4396,6 @@
#define SSL_R_INVALID_SSL_SESSION 160
#define SSL_R_INVALID_TICKET_KEYS_LENGTH 161
#define SSL_R_LENGTH_MISMATCH 162
-#define SSL_R_LIBRARY_HAS_NO_CIPHERS 163
#define SSL_R_MISSING_EXTENSION 164
#define SSL_R_MISSING_RSA_CERTIFICATE 165
#define SSL_R_MISSING_TMP_DH_KEY 166
diff --git a/ssl/internal.h b/ssl/internal.h
index 76f05bd..b8d3211 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -226,14 +226,14 @@
/* ssl_create_cipher_list evaluates |rule_str| according to the ciphers in
* |ssl_method|. It sets |*out_cipher_list| to a newly-allocated
- * |ssl_cipher_preference_list_st| containing the result. It returns
- * |(*out_cipher_list)->ciphers| on success and NULL on failure. If |strict| is
- * true, nonsense will be rejected. If false, nonsense will be silently
- * ignored. */
-STACK_OF(SSL_CIPHER) *
-ssl_create_cipher_list(const SSL_PROTOCOL_METHOD *ssl_method,
- struct ssl_cipher_preference_list_st **out_cipher_list,
- const char *rule_str, int strict);
+ * |ssl_cipher_preference_list_st| containing the result. It returns 1 on
+ * success and 0 on failure. If |strict| is true, nonsense will be rejected. If
+ * false, nonsense will be silently ignored. An empty result is considered an
+ * error regardless of |strict|. */
+int ssl_create_cipher_list(
+ const SSL_PROTOCOL_METHOD *ssl_method,
+ struct ssl_cipher_preference_list_st **out_cipher_list,
+ const char *rule_str, int strict);
/* ssl_cipher_get_value returns the cipher suite id of |cipher|. */
uint16_t ssl_cipher_get_value(const SSL_CIPHER *cipher);
diff --git a/ssl/ssl_cipher.c b/ssl/ssl_cipher.c
index 4a7459f..dc9cc2a 100644
--- a/ssl/ssl_cipher.c
+++ b/ssl/ssl_cipher.c
@@ -1254,10 +1254,10 @@
return 1;
}
-STACK_OF(SSL_CIPHER) *
-ssl_create_cipher_list(const SSL_PROTOCOL_METHOD *ssl_method,
- struct ssl_cipher_preference_list_st **out_cipher_list,
- const char *rule_str, int strict) {
+int ssl_create_cipher_list(
+ const SSL_PROTOCOL_METHOD *ssl_method,
+ struct ssl_cipher_preference_list_st **out_cipher_list,
+ const char *rule_str, int strict) {
STACK_OF(SSL_CIPHER) *cipherstack = NULL;
CIPHER_ORDER *co_list = NULL, *head = NULL, *tail = NULL, *curr;
uint8_t *in_group_flags = NULL;
@@ -1266,7 +1266,7 @@
/* Return with error if nothing to do. */
if (rule_str == NULL || out_cipher_list == NULL) {
- return NULL;
+ return 0;
}
/* Now we have to collect the available ciphers from the compiled in ciphers.
@@ -1275,7 +1275,7 @@
co_list = OPENSSL_malloc(sizeof(CIPHER_ORDER) * kCiphersLen);
if (co_list == NULL) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
- return NULL;
+ return 0;
}
ssl_cipher_collect_ciphers(ssl_method, co_list, &head, &tail);
@@ -1377,6 +1377,11 @@
OPENSSL_free(co_list); /* Not needed any longer */
co_list = NULL;
+ if (sk_SSL_CIPHER_num(cipherstack) == 0) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHER_MATCH);
+ goto err;
+ }
+
pref_list = OPENSSL_malloc(sizeof(struct ssl_cipher_preference_list_st));
if (!pref_list) {
goto err;
@@ -1395,7 +1400,7 @@
*out_cipher_list = pref_list;
pref_list = NULL;
- return cipherstack;
+ return 1;
err:
OPENSSL_free(co_list);
@@ -1405,7 +1410,7 @@
OPENSSL_free(pref_list->in_group_flags);
}
OPENSSL_free(pref_list);
- return NULL;
+ return 0;
}
uint32_t SSL_CIPHER_get_id(const SSL_CIPHER *cipher) { return cipher->id; }
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index b207e92..6e0ece9 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -271,11 +271,7 @@
goto err;
}
- ssl_create_cipher_list(ret->method, &ret->cipher_list,
- SSL_DEFAULT_CIPHER_LIST, 1 /* strict */);
- if (ret->cipher_list == NULL ||
- sk_SSL_CIPHER_num(ret->cipher_list->ciphers) <= 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_LIBRARY_HAS_NO_CIPHERS);
+ if (!SSL_CTX_set_strict_cipher_list(ret, SSL_DEFAULT_CIPHER_LIST)) {
goto err2;
}
@@ -1492,71 +1488,23 @@
}
int SSL_CTX_set_cipher_list(SSL_CTX *ctx, const char *str) {
- STACK_OF(SSL_CIPHER) *cipher_list =
- ssl_create_cipher_list(ctx->method, &ctx->cipher_list, str,
- 0 /* not strict */);
- if (cipher_list == NULL) {
- return 0;
- }
-
- /* |ssl_create_cipher_list| may succeed but return an empty cipher list. */
- if (sk_SSL_CIPHER_num(cipher_list) == 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHER_MATCH);
- return 0;
- }
-
- return 1;
+ return ssl_create_cipher_list(ctx->method, &ctx->cipher_list, str,
+ 0 /* not strict */);
}
int SSL_CTX_set_strict_cipher_list(SSL_CTX *ctx, const char *str) {
- STACK_OF(SSL_CIPHER) *cipher_list =
- ssl_create_cipher_list(ctx->method, &ctx->cipher_list, str,
- 1 /* strict */);
- if (cipher_list == NULL) {
- return 0;
- }
-
- /* |ssl_create_cipher_list| may succeed but return an empty cipher list. */
- if (sk_SSL_CIPHER_num(cipher_list) == 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHER_MATCH);
- return 0;
- }
-
- return 1;
+ return ssl_create_cipher_list(ctx->method, &ctx->cipher_list, str,
+ 1 /* strict */);
}
int SSL_set_cipher_list(SSL *ssl, const char *str) {
- STACK_OF(SSL_CIPHER) *cipher_list =
- ssl_create_cipher_list(ssl->ctx->method, &ssl->cipher_list, str,
- 0 /* not strict */);
- if (cipher_list == NULL) {
- return 0;
- }
-
- /* |ssl_create_cipher_list| may succeed but return an empty cipher list. */
- if (sk_SSL_CIPHER_num(cipher_list) == 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHER_MATCH);
- return 0;
- }
-
- return 1;
+ return ssl_create_cipher_list(ssl->ctx->method, &ssl->cipher_list, str,
+ 0 /* not strict */);
}
int SSL_set_strict_cipher_list(SSL *ssl, const char *str) {
- STACK_OF(SSL_CIPHER) *cipher_list =
- ssl_create_cipher_list(ssl->ctx->method, &ssl->cipher_list, str,
- 1 /* strict */);
- if (cipher_list == NULL) {
- return 0;
- }
-
- /* |ssl_create_cipher_list| may succeed but return an empty cipher list. */
- if (sk_SSL_CIPHER_num(cipher_list) == 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHER_MATCH);
- return 0;
- }
-
- return 1;
+ return ssl_create_cipher_list(ssl->ctx->method, &ssl->cipher_list, str,
+ 1 /* strict */);
}
const char *SSL_get_servername(const SSL *ssl, const int type) {
