{"schema":"libjg2-1", "vpath":"/git/", "avatar":"/git/avatar/", "alang":"", "gen_ut":1752648765, "reponame":"openssl", "desc":"OpenSSL", "owner": { "name": "Andy Green", "email": "andy@warmcat.com", "md5": "c50933ca2aa61e0fe2c43d46bb6b59cb" },"url":"https://warmcat.com/repo/openssl", "f":3, "items": [ {"schema":"libjg2-1", "cid":"e063d77aa132197f5c518b64a439f038", "commit": {"type":"commit", "time": 1512394308, "time_ofs": 0, "oid_tree": { "oid": "f7d23d758f00a0a107a2b740859dc4857326b55c", "alias": []}, "oid":{ "oid": "29bfd5b79a1ec2be8bea307ffb50810fce11915c", "alias": []}, "msg": "Add some more cleanups", "sig_commit": { "git_time": { "time": 1512394308, "offset": 0 }, "name": "Matt Caswell", "email": "matt@openssl.org", "md5": "10f7b441a32d5790efad9fc68cae4af2" }, "sig_author": { "git_time": { "time": 1511442702, "offset": 0 }, "name": "Matt Caswell", "email": "matt@openssl.org", "md5": "10f7b441a32d5790efad9fc68cae4af2" }}, "body": "Add some more cleanups\n\nFollow up from the conversion to use SSLfatal() in the state machine to\nclean things up a bit more.\n\n[extended tests]\n\nReviewed-by: Richard Levitte \u003clevitte@openssl.org\u003e\n(Merged from https://github.com/openssl/openssl/pull/4778)\n" , "diff": "diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt\nindex 1220d3a..932fc46 100644\n--- a/crypto/err/openssl.txt\n+++ b/crypto/err/openssl.txt\n@@ -1079,7 +1079,9 @@ SSL_F_SSL_CERT_NEW:162:ssl_cert_new\n SSL_F_SSL_CERT_SET0_CHAIN:340:ssl_cert_set0_chain\n SSL_F_SSL_CHECK_PRIVATE_KEY:163:SSL_check_private_key\n SSL_F_SSL_CHECK_SERVERHELLO_TLSEXT:280:*\n+SSL_F_SSL_CHECK_SRP_EXT_CLIENTHELLO:606:ssl_check_srp_ext_ClientHello\n SSL_F_SSL_CHECK_SRVR_ECC_CERT_AND_ALG:279:ssl_check_srvr_ecc_cert_and_alg\n+SSL_F_SSL_CHOOSE_CLIENT_VERSION:607:ssl_choose_client_version\n SSL_F_SSL_CIPHER_LIST_TO_BYTES:425:ssl_cipher_list_to_bytes\n SSL_F_SSL_CIPHER_PROCESS_RULESTR:230:ssl_cipher_process_rulestr\n SSL_F_SSL_CIPHER_STRENGTH_SORT:231:ssl_cipher_strength_sort\ndiff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h\nindex 6662f66..ef6b9dd 100644\n--- a/include/openssl/sslerr.h\n+++ b/include/openssl/sslerr.h\n@@ -132,7 +132,9 @@ int ERR_load_SSL_strings(void);\n # define SSL_F_SSL_CERT_SET0_CHAIN 340\n # define SSL_F_SSL_CHECK_PRIVATE_KEY 163\n # define SSL_F_SSL_CHECK_SERVERHELLO_TLSEXT 280\n+# define SSL_F_SSL_CHECK_SRP_EXT_CLIENTHELLO 606\n # define SSL_F_SSL_CHECK_SRVR_ECC_CERT_AND_ALG 279\n+# define SSL_F_SSL_CHOOSE_CLIENT_VERSION 607\n # define SSL_F_SSL_CIPHER_LIST_TO_BYTES 425\n # define SSL_F_SSL_CIPHER_PROCESS_RULESTR 230\n # define SSL_F_SSL_CIPHER_STRENGTH_SORT 231\ndiff --git a/ssl/ssl_err.c b/ssl/ssl_err.c\nindex b9bef5a..7710ef0 100644\n--- a/ssl/ssl_err.c\n+++ b/ssl/ssl_err.c\n@@ -180,8 +180,12 @@ static const ERR_STRING_DATA SSL_str_functs[] \u003d {\n {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CHECK_PRIVATE_KEY, 0),\n \u0022SSL_check_private_key\u0022},\n {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CHECK_SERVERHELLO_TLSEXT, 0), \u0022\u0022},\n+ {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CHECK_SRP_EXT_CLIENTHELLO, 0),\n+ \u0022ssl_check_srp_ext_ClientHello\u0022},\n {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CHECK_SRVR_ECC_CERT_AND_ALG, 0),\n \u0022ssl_check_srvr_ecc_cert_and_alg\u0022},\n+ {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CHOOSE_CLIENT_VERSION, 0),\n+ \u0022ssl_choose_client_version\u0022},\n {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CIPHER_LIST_TO_BYTES, 0),\n \u0022ssl_cipher_list_to_bytes\u0022},\n {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_CIPHER_PROCESS_RULESTR, 0),\ndiff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h\nindex 020d774..c812eef 100644\n--- a/ssl/ssl_locl.h\n+++ b/ssl/ssl_locl.h\n@@ -2266,8 +2266,7 @@ __owur int ssl_check_version_downgrade(SSL *s);\n __owur int ssl_set_version_bound(int method_version, int version, int *bound);\n __owur int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello,\n DOWNGRADE *dgrd);\n-__owur int ssl_choose_client_version(SSL *s, int version, int checkdgrd,\n- int *al);\n+__owur int ssl_choose_client_version(SSL *s, int version, int checkdgrd);\n int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version);\n \n __owur long tls1_default_timeout(void);\ndiff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c\nindex f4aa98e..464a5ef 100644\n--- a/ssl/statem/extensions.c\n+++ b/ssl/statem/extensions.c\n@@ -439,12 +439,11 @@ int extension_is_relevant(SSL *s, unsigned int extctx, unsigned int thisctx)\n /*\n * Gather a list of all the extensions from the data in |packet]. |context|\n * tells us which message this extension is for. The raw extension data is\n- * stored in |*res| on success. In the event of an error the alert type to use\n- * is stored in |*al|. We don't actually process the content of the extensions\n- * yet, except to check their types. This function also runs the initialiser\n- * functions for all known extensions if |init| is nonzero (whether we have\n- * collected them or not). If successful the caller is responsible for freeing\n- * the contents of |*res|.\n+ * stored in |*res| on success. We don't actually process the content of the\n+ * extensions yet, except to check their types. This function also runs the\n+ * initialiser functions for all known extensions if |init| is nonzero (whether\n+ * we have collected them or not). If successful the caller is responsible for\n+ * freeing the contents of |*res|.\n *\n * Per http://tools.ietf.org/html/rfc5246#section-7.4.1.4, there may not be\n * more than one extension of the same type in a ClientHello or ServerHello.\n@@ -579,9 +578,8 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,\n * given |context| and the parser has not already been run. If this is for a\n * Certificate message, then we also provide the parser with the relevant\n * Certificate |x| and its position in the |chainidx| with 0 being the first\n- * Certificate. Returns 1 on success or 0 on failure. In the event of a failure\n- * |*al| is populated with a suitable alert code. If an extension is not present\n- * this counted as success.\n+ * Certificate. Returns 1 on success or 0 on failure. If an extension is not\n+ * present this counted as success.\n */\n int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context,\n RAW_EXTENSION *exts, X509 *x, size_t chainidx)\n@@ -631,8 +629,7 @@ int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context,\n * finalisation for all extensions at the end if |fin| is nonzero, whether we\n * collected them or not. Returns 1 for success or 0 for failure. If we are\n * working on a Certificate message then we also pass the Certificate |x| and\n- * its position in the |chainidx|, with 0 being the first certificate. On\n- * failure, |*al| is populated with a suitable alert code.\n+ * its position in the |chainidx|, with 0 being the first certificate.\n */\n int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, X509 *x,\n size_t chainidx, int fin)\n@@ -782,8 +779,7 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,\n * Built in extension finalisation and initialisation functions. All initialise\n * or finalise the associated extension type for the given |context|. For\n * finalisers |sent| is set to 1 if we saw the extension during parsing, and 0\n- * otherwise. These functions return 1 on success or 0 on failure. In the event\n- * of a failure then |*al| is populated with a suitable error code.\n+ * otherwise. These functions return 1 on success or 0 on failure.\n */\n \n static int final_renegotiate(SSL *s, unsigned int context, int sent)\ndiff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c\nindex ca1cef5..c626ba6 100644\n--- a/ssl/statem/extensions_srvr.c\n+++ b/ssl/statem/extensions_srvr.c\n@@ -400,8 +400,7 @@ int tls_parse_ctos_npn(SSL *s, PACKET *pkt, unsigned int context, X509 *x,\n \n /*\n * Save the ALPN extension in a ClientHello.|pkt| holds the contents of the ALPN\n- * extension, not including type and length. |al| is a pointer to the alert\n- * value to send in the event of a failure. Returns: 1 on success, 0 on error.\n+ * extension, not including type and length. Returns: 1 on success, 0 on error.\n */\n int tls_parse_ctos_alpn(SSL *s, PACKET *pkt, unsigned int context, X509 *x,\n size_t chainidx)\n@@ -523,7 +522,6 @@ int tls_parse_ctos_etm(SSL *s, PACKET *pkt, unsigned int context, X509 *x,\n /*\n * Process a psk_kex_modes extension received in the ClientHello. |pkt| contains\n * the raw PACKET data for the extension. Returns 1 on success or 0 on failure.\n- * If a failure occurs then |*al| is set to an appropriate alert value.\n */\n int tls_parse_ctos_psk_kex_modes(SSL *s, PACKET *pkt, unsigned int context,\n X509 *x, size_t chainidx)\n@@ -554,7 +552,6 @@ int tls_parse_ctos_psk_kex_modes(SSL *s, PACKET *pkt, unsigned int context,\n /*\n * Process a key_share extension received in the ClientHello. |pkt| contains\n * the raw PACKET data for the extension. Returns 1 on success or 0 on failure.\n- * If a failure occurs then |*al| is set to an appropriate alert value.\n */\n int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,\n size_t chainidx)\ndiff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c\nindex 038fac9..48e6a0a 100644\n--- a/ssl/statem/statem_clnt.c\n+++ b/ssl/statem/statem_clnt.c\n@@ -1310,11 +1310,10 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)\n PACKET session_id, extpkt;\n size_t session_id_len;\n const unsigned char *cipherchars;\n- int al \u003d SSL_AD_INTERNAL_ERROR;\n unsigned int compression;\n unsigned int sversion;\n unsigned int context;\n- int protverr, discard;\n+ int discard;\n RAW_EXTENSION *extensions \u003d NULL;\n #ifndef OPENSSL_NO_COMP\n SSL_COMP *comp;\n@@ -1338,10 +1337,8 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)\n * Must be done after reading the random data so we can check for the\n * TLSv1.3 downgrade sentinels\n */\n- protverr \u003d ssl_choose_client_version(s, sversion, 1, \u0026al);\n- if (protverr !\u003d 0) {\n- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,\n- protverr);\n+ if (!ssl_choose_client_version(s, sversion, 1)) {\n+ /* SSLfatal() already called */\n goto err;\n }\n \ndiff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c\nindex 65c3aa3..b8e094b 100644\n--- a/ssl/statem/statem_lib.c\n+++ b/ssl/statem/statem_lib.c\n@@ -1740,11 +1740,10 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd)\n * @s: client SSL handle.\n * @version: The proposed version from the server's HELLO.\n * @checkdgrd: Whether to check the downgrade sentinels in the server_random\n- * @al: Where to store any alert value that may be generated\n *\n- * Returns 0 on success or an SSL error reason number on failure.\n+ * Returns 1 on success or 0 on error.\n */\n-int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)\n+int ssl_choose_client_version(SSL *s, int version, int checkdgrd)\n {\n const version_info *vent;\n const version_info *table;\n@@ -1755,15 +1754,18 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)\n version \u003d TLS1_3_VERSION;\n \n if (s-\u003ehello_retry_request \u0026\u0026 version !\u003d TLS1_3_VERSION) {\n- *al \u003d SSL_AD_PROTOCOL_VERSION;\n- return SSL_R_WRONG_SSL_VERSION;\n+ SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION,\n+ SSL_R_WRONG_SSL_VERSION);\n+ return 0;\n }\n \n switch (s-\u003emethod-\u003eversion) {\n default:\n if (version !\u003d s-\u003eversion) {\n- *al \u003d SSL_AD_PROTOCOL_VERSION;\n- return SSL_R_WRONG_SSL_VERSION;\n+ SSLfatal(s, SSL_AD_PROTOCOL_VERSION,\n+ SSL_F_SSL_CHOOSE_CLIENT_VERSION,\n+ SSL_R_WRONG_SSL_VERSION);\n+ return 0;\n }\n /*\n * If this SSL handle is not from a version flexible method we don't\n@@ -1772,7 +1774,7 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)\n * versions they don't want. If not, then easy to fix, just return\n * ssl_method_error(s, s-\u003emethod)\n */\n- return 0;\n+ return 1;\n case TLS_ANY_VERSION:\n table \u003d tls_version_table;\n break;\n@@ -1795,8 +1797,9 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)\n err \u003d ssl_method_error(s, method);\n if (err !\u003d 0) {\n if (version \u003d\u003d vent-\u003eversion) {\n- *al \u003d SSL_AD_PROTOCOL_VERSION;\n- return err;\n+ SSLfatal(s, SSL_AD_PROTOCOL_VERSION,\n+ SSL_F_SSL_CHOOSE_CLIENT_VERSION, err);\n+ return 0;\n }\n \n continue;\n@@ -1815,8 +1818,10 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)\n s-\u003es3-\u003eserver_random + SSL3_RANDOM_SIZE\n - sizeof(tls12downgrade),\n sizeof(tls12downgrade)) \u003d\u003d 0) {\n- *al \u003d SSL_AD_ILLEGAL_PARAMETER;\n- return SSL_R_INAPPROPRIATE_FALLBACK;\n+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,\n+ SSL_F_SSL_CHOOSE_CLIENT_VERSION,\n+ SSL_R_INAPPROPRIATE_FALLBACK);\n+ return 0;\n }\n } else if (!SSL_IS_DTLS(s)\n \u0026\u0026 version \u003c TLS1_2_VERSION\n@@ -1825,8 +1830,10 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)\n s-\u003es3-\u003eserver_random + SSL3_RANDOM_SIZE\n - sizeof(tls11downgrade),\n sizeof(tls11downgrade)) \u003d\u003d 0) {\n- *al \u003d SSL_AD_ILLEGAL_PARAMETER;\n- return SSL_R_INAPPROPRIATE_FALLBACK;\n+ SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,\n+ SSL_F_SSL_CHOOSE_CLIENT_VERSION,\n+ SSL_R_INAPPROPRIATE_FALLBACK);\n+ return 0;\n }\n }\n }\n@@ -1834,11 +1841,12 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)\n \n s-\u003emethod \u003d method;\n s-\u003eversion \u003d version;\n- return 0;\n+ return 1;\n }\n \n- *al \u003d SSL_AD_PROTOCOL_VERSION;\n- return SSL_R_UNSUPPORTED_PROTOCOL;\n+ SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION,\n+ SSL_R_UNSUPPORTED_PROTOCOL);\n+ return 0;\n }\n \n /*\ndiff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c\nindex 7e6aa94..0ba7f89 100644\n--- a/ssl/statem/statem_srvr.c\n+++ b/ssl/statem/statem_srvr.c\n@@ -1102,11 +1102,11 @@ WORK_STATE ossl_statem_server_post_process_message(SSL *s, WORK_STATE wst)\n }\n \n #ifndef OPENSSL_NO_SRP\n-static int ssl_check_srp_ext_ClientHello(SSL *s, int *al)\n+/* Returns 1 on success, 0 for retryable error, -1 for fatal error */\n+static int ssl_check_srp_ext_ClientHello(SSL *s)\n {\n- int ret \u003d SSL_ERROR_NONE;\n-\n- *al \u003d SSL_AD_UNRECOGNIZED_NAME;\n+ int ret;\n+ int al \u003d SSL_AD_UNRECOGNIZED_NAME;\n \n if ((s-\u003es3-\u003etmp.new_cipher-\u003ealgorithm_mkey \u0026 SSL_kSRP) \u0026\u0026\n (s-\u003esrp_ctx.TLS_ext_srp_username_callback !\u003d NULL)) {\n@@ -1115,13 +1115,24 @@ static int ssl_check_srp_ext_ClientHello(SSL *s, int *al)\n * RFC 5054 says SHOULD reject, we do so if There is no srp\n * login name\n */\n- ret \u003d SSL3_AL_FATAL;\n- *al \u003d SSL_AD_UNKNOWN_PSK_IDENTITY;\n+ SSLfatal(s, SSL_AD_UNKNOWN_PSK_IDENTITY,\n+ SSL_F_SSL_CHECK_SRP_EXT_CLIENTHELLO,\n+ SSL_R_PSK_IDENTITY_NOT_FOUND);\n+ return -1;\n } else {\n- ret \u003d SSL_srp_server_param_with_username(s, al);\n+ ret \u003d SSL_srp_server_param_with_username(s, \u0026al);\n+ if (ret \u003c 0)\n+ return 0;\n+ if (ret \u003d\u003d SSL3_AL_FATAL) {\n+ SSLfatal(s, al, SSL_F_SSL_CHECK_SRP_EXT_CLIENTHELLO,\n+ al \u003d\u003d SSL_AD_UNKNOWN_PSK_IDENTITY\n+ ? SSL_R_PSK_IDENTITY_NOT_FOUND\n+ : SSL_R_CLIENTHELLO_TLSEXT);\n+ return -1;\n+ }\n }\n }\n- return ret;\n+ return 1;\n }\n #endif\n \n@@ -1986,7 +1997,7 @@ static int tls_handle_status_request(SSL *s)\n \n /*\n * Call the alpn_select callback if needed. Upon success, returns 1.\n- * Upon failure, returns 0 and sets |*al| to the appropriate fatal alert.\n+ * Upon failure, returns 0.\n */\n int tls_handle_alpn(SSL *s)\n {\n@@ -2058,7 +2069,6 @@ int tls_handle_alpn(SSL *s)\n \n WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)\n {\n- int al \u003d SSL_AD_HANDSHAKE_FAILURE;\n const SSL_CIPHER *cipher;\n \n if (wst \u003d\u003d WORK_MORE_A) {\n@@ -2158,24 +2168,15 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)\n #ifndef OPENSSL_NO_SRP\n if (wst \u003d\u003d WORK_MORE_C) {\n int ret;\n- if ((ret \u003d ssl_check_srp_ext_ClientHello(s, \u0026al)) \u003c 0) {\n+ if ((ret \u003d ssl_check_srp_ext_ClientHello(s)) \u003d\u003d 0) {\n /*\n * callback indicates further work to be done\n */\n s-\u003erwstate \u003d SSL_X509_LOOKUP;\n return WORK_MORE_C;\n }\n- if (ret !\u003d SSL_ERROR_NONE) {\n- /*\n- * This is not really an error but the only means to for\n- * a client to detect whether srp is supported.\n- */\n- if (al !\u003d TLS1_AD_UNKNOWN_PSK_IDENTITY)\n- SSLfatal(s, al, SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,\n- SSL_R_CLIENTHELLO_TLSEXT);\n- else\n- SSLfatal(s, al, SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,\n- SSL_R_PSK_IDENTITY_NOT_FOUND);\n+ if (ret \u003c 0) {\n+ /* SSLfatal() already called */\n goto err;\n }\n }\n","s":{"c":1752648765,"u": 52715}} ],"g": 54413,"chitpc": 0,"ehitpc": 0,"indexed":0 , "ab": 0, "si": 0, "db":0, "di":0, "sat":0, "lfc": "0000"}