From: Bodo Möller Date: Tue, 16 Nov 1999 23:15:41 +0000 (+0000) Subject: Store verify_result with sessions to avoid potential security hole. X-Git-Tag: OpenSSL_0_9_5beta1~419 X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=b1fe6ca175bdbb51a064c1e5519b21d80804e7c6;p=oweals%2Fopenssl.git Store verify_result with sessions to avoid potential security hole. --- diff --git a/CHANGES b/CHANGES index 4ccab5772f..289342b987 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,17 @@ Changes between 0.9.4 and 0.9.5 [xx XXX 1999] + *) For servers, store verify_result in SSL_SESSION data structure + (and add it to external session representation). + This is needed when client certificate verifications fails, + but an application-provided verification callback (set by + SSL_CTX_set_cert_verify_callback) allows accepting the session + anyway (i.e. leaves x509_store_ctx->error != X509_V_OK + but returns 1): When the session is reused, we have to set + ssl->verify_result to the appropriate error code to avoid + security holes. + [Bodo Moeller, problem pointed out by Lutz Jaenicke] + *) Fix a bug in the new PKCS#7 code: it didn't consider the case in PKCS7_dataInit() where the signed PKCS7 structure didn't contain any existing data because it was being created. diff --git a/crypto/x509/x509_vfy.h b/crypto/x509/x509_vfy.h index ecfd4cf9ed..39fa056c1a 100644 --- a/crypto/x509/x509_vfy.h +++ b/crypto/x509/x509_vfy.h @@ -234,6 +234,7 @@ struct x509_store_state_st /* X509_STORE_CTX */ X509_LOOKUP_ctrl((x),X509_L_ADD_DIR,(name),(long)(type),NULL) #define X509_V_OK 0 +/* illegal error (for uninitialized values, to avoid X509_V_OK): 1 */ #define X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT 2 #define X509_V_ERR_UNABLE_TO_GET_CRL 3 diff --git a/ssl/s2_clnt.c b/ssl/s2_clnt.c index 1fe8bd627d..956c0280c9 100644 --- a/ssl/s2_clnt.c +++ b/ssl/s2_clnt.c @@ -445,7 +445,7 @@ static int get_server_hello(SSL *s) CRYPTO_w_unlock(CRYPTO_LOCK_X509); #else s->session->peer = s->session->sess_cert->peer_key->x509; - /* peer_key->x509 has been set by ssl2_set_certificate. */ + /* peer_key->x509 has been set by ssl2_set_certificate. */ CRYPTO_add(&s->session->peer->references, 1, CRYPTO_LOCK_X509); #endif diff --git a/ssl/s2_srvr.c b/ssl/s2_srvr.c index 9aeedef55f..f8f1ba76d0 100644 --- a/ssl/s2_srvr.c +++ b/ssl/s2_srvr.c @@ -921,6 +921,7 @@ static int request_certificate(SSL *s) X509_free(s->session->peer); s->session->peer=x509; CRYPTO_add(&x509->references,1,CRYPTO_LOCK_X509); + s->session->verify_result = s->verify_result; ret=1; goto end; } diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 961a2ca10c..dd3b149a89 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c @@ -1627,6 +1627,7 @@ static int ssl3_get_client_certificate(SSL *s) if (s->session->peer != NULL) /* This should not be needed */ X509_free(s->session->peer); s->session->peer=sk_X509_shift(sk); + s->session->verify_result = s->verify_result; /* With the current implementation, sess_cert will always be NULL * when we arrive here. */ diff --git a/ssl/ssl.h b/ssl/ssl.h index 3f7cdd10dd..f888530625 100644 --- a/ssl/ssl.h +++ b/ssl/ssl.h @@ -215,7 +215,8 @@ typedef struct ssl_method_st * Timeout [ 2 ] EXPLICIT INTEGER, -- optional Timeout ins seconds * Peer [ 3 ] EXPLICIT X509, -- optional Peer Certificate * Session_ID_context [ 4 ] EXPLICIT OCTET_STRING, -- the Session ID context - * Compression [5] IMPLICIT ASN1_OBJECT -- compression OID XXXXX + * Verify_result [ 5 ] EXPLICIT INTEGER -- X509_V_... code for `Peer' + * Compression [6] IMPLICIT ASN1_OBJECT -- compression OID XXXXX * } * Look in ssl/ssl_asn1.c for more details * I'm using EXPLICIT tags so I can read the damn things using asn1parse :-). @@ -249,6 +250,9 @@ typedef struct ssl_session_st * (the latter is not enough as sess_cert is not retained * in the external representation of sessions, see ssl_asn1.c). */ X509 *peer; + /* when app_verify_callback accepts a session where the peer's certificate + * is not ok, we must remember the error for session reuse: */ + long verify_result; /* only for servers */ int references; long timeout; diff --git a/ssl/ssl_asn1.c b/ssl/ssl_asn1.c index 0f6a0884e4..d77b0f26db 100644 --- a/ssl/ssl_asn1.c +++ b/ssl/ssl_asn1.c @@ -60,6 +60,7 @@ #include #include #include +#include #include "ssl_locl.h" typedef struct ssl_session_asn1_st @@ -73,14 +74,15 @@ typedef struct ssl_session_asn1_st ASN1_OCTET_STRING key_arg; ASN1_INTEGER time; ASN1_INTEGER timeout; + ASN1_INTEGER verify_result; } SSL_SESSION_ASN1; int i2d_SSL_SESSION(SSL_SESSION *in, unsigned char **pp) { #define LSIZE2 (sizeof(long)*2) - int v1=0,v2=0,v3=0,v4=0; + int v1=0,v2=0,v3=0,v4=0,v5=0; unsigned char buf[4],ibuf1[LSIZE2],ibuf2[LSIZE2]; - unsigned char ibuf3[LSIZE2],ibuf4[LSIZE2]; + unsigned char ibuf3[LSIZE2],ibuf4[LSIZE2],ibuf5[LSIZE2]; long l; SSL_SESSION_ASN1 a; M_ASN1_I2D_vars(in); @@ -156,6 +158,14 @@ int i2d_SSL_SESSION(SSL_SESSION *in, unsigned char **pp) ASN1_INTEGER_set(&(a.timeout),in->timeout); } + if (in->verify_result != X509_V_OK) + { + a.verify_result.length=LSIZE2; + a.verify_result.type=V_ASN1_INTEGER; + a.verify_result.data=ibuf5; + ASN1_INTEGER_set(&a.verify_result,in->verify_result); + } + M_ASN1_I2D_len(&(a.version), i2d_ASN1_INTEGER); M_ASN1_I2D_len(&(a.ssl_version), i2d_ASN1_INTEGER); M_ASN1_I2D_len(&(a.cipher), i2d_ASN1_OCTET_STRING); @@ -170,6 +180,8 @@ int i2d_SSL_SESSION(SSL_SESSION *in, unsigned char **pp) if (in->peer != NULL) M_ASN1_I2D_len_EXP_opt(in->peer,i2d_X509,3,v3); M_ASN1_I2D_len_EXP_opt(&a.session_id_context,i2d_ASN1_OCTET_STRING,4,v4); + if (in->verify_result != X509_V_OK) + M_ASN1_I2D_len_EXP_opt(&(a.verify_result),i2d_ASN1_INTEGER,5,v5); M_ASN1_I2D_seq_total(); @@ -188,7 +200,8 @@ int i2d_SSL_SESSION(SSL_SESSION *in, unsigned char **pp) M_ASN1_I2D_put_EXP_opt(in->peer,i2d_X509,3,v3); M_ASN1_I2D_put_EXP_opt(&a.session_id_context,i2d_ASN1_OCTET_STRING,4, v4); - + if (in->verify_result != X509_V_OK) + M_ASN1_I2D_put_EXP_opt(&a.verify_result,i2d_ASN1_INTEGER,5,v5); M_ASN1_I2D_finish(); } @@ -322,6 +335,15 @@ SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, unsigned char **pp, else ret->sid_ctx_length=0; + ai.length=0; + M_ASN1_D2I_get_EXP_opt(aip,d2i_ASN1_INTEGER,5); + if (ai.data != NULL) + { + ret->verify_result=ASN1_INTEGER_get(aip); + Free(ai.data); ai.data=NULL; ai.length=0; + } + else + ret->verify_result=X509_V_OK; + M_ASN1_D2I_Finish(a,SSL_SESSION_free,SSL_F_D2I_SSL_SESSION); } - diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 4dddf627cd..57ee7eb3c5 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -112,6 +112,7 @@ SSL_SESSION *SSL_SESSION_new(void) } memset(ss,0,sizeof(SSL_SESSION)); + ss->verify_result = 1; /* avoid 0 (= X509_V_OK) just in case */ ss->references=1; ss->timeout=60*5+4; /* 5 minute timeout by default */ ss->time=time(NULL); @@ -190,6 +191,7 @@ int ssl_get_new_session(SSL *s, int session) ss->sid_ctx_length=s->sid_ctx_length; s->session=ss; ss->ssl_version=s->version; + ss->verify_result = X509_V_OK; return(1); } @@ -320,6 +322,7 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len) if (s->session != NULL) SSL_SESSION_free(s->session); s->session=ret; + s->verify_result = s->session->verify_result; return(1); err: diff --git a/ssl/ssltest.c b/ssl/ssltest.c index 7d3c31dfae..2e373a8f37 100644 --- a/ssl/ssltest.c +++ b/ssl/ssltest.c @@ -398,6 +398,11 @@ bad: SSL_CTX_set_verify(c_ctx,SSL_VERIFY_PEER, verify_callback); } + + { + int session_id_context = 0; + SSL_CTX_set_session_id_context(s_ctx, (void *)&session_id_context, sizeof session_id_context); + } c_ssl=SSL_new(c_ctx); s_ssl=SSL_new(s_ctx);