diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c index 451a3c24b2..1d3236f196 100644 --- a/ssl/d1_pkt.c +++ b/ssl/d1_pkt.c @@ -329,7 +329,6 @@ static int dtls1_process_buffered_records(SSL *s) { static int dtls1_process_record(SSL *s) { int al; - int enc_err; SSL3_RECORD *rr; rr = &(s->s3->rrec); @@ -357,23 +356,12 @@ static int dtls1_process_record(SSL *s) { /* decrypt in place in 'rr->input' */ rr->data = rr->input; - enc_err = s->enc_method->enc(s, 0); - /* enc_err is: - * 0: (in non-constant time) if the record is publically invalid. - * 1: if the padding is valid - * -1: if the padding is invalid */ - if (enc_err == 0) { + if (!s->enc_method->enc(s, 0)) { /* For DTLS we simply ignore bad packets. */ rr->length = 0; s->packet_length = 0; goto err; } - if (enc_err < 0) { - /* decryption failed, silently discard message */ - rr->length = 0; - s->packet_length = 0; - goto err; - } if (rr->length > SSL3_RT_MAX_PLAIN_LENGTH) { al = SSL_AD_RECORD_OVERFLOW; @@ -1171,7 +1159,7 @@ static int do_dtls1_write(SSL *s, int type, const uint8_t *buf, wr->data = p; wr->length += eivlen; - if (s->enc_method->enc(s, 1) < 1) { + if (!s->enc_method->enc(s, 1)) { goto err; } diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index f51e8295c7..4263cb03e1 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -268,7 +268,7 @@ int ssl3_read_n(SSL *s, int n, int max, int extend) { /* used only by ssl3_read_bytes */ static int ssl3_get_record(SSL *s) { int ssl_major, ssl_minor, al; - int enc_err, n, i, ret = -1; + int n, i, ret = -1; SSL3_RECORD *rr; uint8_t *p; short version; @@ -373,22 +373,7 @@ static int ssl3_get_record(SSL *s) { /* decrypt in place in 'rr->input' */ rr->data = rr->input; - enc_err = s->enc_method->enc(s, 0); - /* enc_err is: - * 0: (in non-constant time) if the record is publically invalid. - * 1: if the padding is valid - * -1: if the padding is invalid */ - if (enc_err == 0) { - al = SSL_AD_DECRYPTION_FAILED; - OPENSSL_PUT_ERROR(SSL, ssl3_get_record, SSL_R_BLOCK_CIPHER_PAD_IS_WRONG); - goto f_err; - } - if (enc_err < 0) { - /* A separate 'decryption_failed' alert was introduced with TLS 1.0, SSL - * 3.0 only has 'bad_record_mac'. But unless a decryption failure is - * directly visible from the ciphertext anyway, we should not reveal which - * kind of error occured – this might become visible to an attacker (e.g. - * via a logfile) */ + if (!s->enc_method->enc(s, 0)) { al = SSL_AD_BAD_RECORD_MAC; OPENSSL_PUT_ERROR(SSL, ssl3_get_record, SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC); @@ -625,7 +610,7 @@ static int do_ssl3_write(SSL *s, int type, const uint8_t *buf, unsigned int len, wr->data = p; wr->length += eivlen; - if (s->enc_method->enc(s, 1) < 1) { + if (!s->enc_method->enc(s, 1)) { goto err; } diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index 10d940fe5d..014bc88ffe 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c @@ -565,14 +565,7 @@ int tls1_setup_key_block(SSL *s) { } /* tls1_enc encrypts/decrypts the record in |s->wrec| / |s->rrec|, - * respectively. - * - * Returns: - * 0: (in non-constant time) if the record is publically invalid (i.e. too - * short etc). - * 1: if the record's padding is valid / the encryption was successful. - * -1: if the record's padding/AEAD-authenticator is invalid or, if sending, - * an internal error occured. */ + * respectively. It returns one on success and zero on failure. */ int tls1_enc(SSL *s, int send) { SSL3_RECORD *rec; const SSL_AEAD_CTX *aead; @@ -586,6 +579,7 @@ int tls1_enc(SSL *s, int send) { } if (s->session == NULL || aead == NULL) { + /* Handle the initial NULL cipher. */ memmove(rec->data, rec->input, rec->length); rec->input = rec->data; return 1; @@ -623,7 +617,7 @@ int tls1_enc(SSL *s, int send) { if (aead->fixed_nonce_len + aead->variable_nonce_len > sizeof(nonce)) { OPENSSL_PUT_ERROR(SSL, tls1_enc, ERR_R_INTERNAL_ERROR); - return -1; /* internal error - should never happen. */ + return 0; } memcpy(nonce, aead->fixed_nonce, aead->fixed_nonce_len); @@ -639,14 +633,14 @@ int tls1_enc(SSL *s, int send) { if (aead->random_variable_nonce) { assert(aead->variable_nonce_included_in_record); if (!RAND_bytes(nonce + nonce_used, aead->variable_nonce_len)) { - return -1; + return 0; } } else { /* When sending we use the sequence number as the variable part of the * nonce. */ if (aead->variable_nonce_len != 8) { OPENSSL_PUT_ERROR(SSL, tls1_enc, ERR_R_INTERNAL_ERROR); - return -1; + return 0; } memcpy(nonce + nonce_used, ad, aead->variable_nonce_len); } @@ -669,7 +663,7 @@ int tls1_enc(SSL *s, int send) { if (!EVP_AEAD_CTX_seal(&aead->ctx, out + eivlen, &n, len + aead->tag_len, nonce, nonce_used, in + eivlen, len, ad, ad_len)) { - return -1; + return 0; } if (aead->variable_nonce_included_in_record) { @@ -681,7 +675,7 @@ int tls1_enc(SSL *s, int send) { if (rec->data != rec->input) { OPENSSL_PUT_ERROR(SSL, tls1_enc, ERR_R_INTERNAL_ERROR); - return -1; /* internal error - should never happen. */ + return 0; } out = in = rec->input; @@ -711,7 +705,7 @@ int tls1_enc(SSL *s, int send) { if (!EVP_AEAD_CTX_open(&aead->ctx, out, &n, rec->length, nonce, nonce_used, in, len, ad, ad_len)) { - return -1; + return 0; } rec->data = rec->input = out;