search

ktls / af_ktls

Linux Kernel TLS/DTLS Module
GNU General Public License v2.0
159 stars 25 forks source link

Fixed dangerous cast. #60

Closed lancerchao closed 8 years ago

lancerchao commented 8 years ago

Taken from OpenSSL

nmav commented 8 years ago

LGTM

fridex commented 8 years ago

Looks good to me as well. Cannot merge now because of tls_err_abort() which is introduced in #62. Will do the review.

djwatson commented 8 years ago

LGTM. Let's merge this and just rebase #62 on top?

fridex commented 8 years ago

The static keyword was removed. I would merge it without overflow check and leave it for the next patch?

I'm not sure about the overflow handling like implemented here from user space POV. The user space has to trigger rehandshake. There needs to be checked how GnuTLS and OpenSSL behaves in this situation.

IIRC, GnuTLS does handshake explicitly whereas OpenSSL implicitly/transparently during communication. I'm not sure if we trigger rehandshake just by propagating an error this way.

djwatson commented 8 years ago

Ah I see. Yea I would merge without overflow and open a separate issue for overflow.

nmav commented 8 years ago

I'm not sure about the overflow handling like implemented here from user space POV. The user space has to trigger rehandshake. There needs to be checked how GnuTLS and OpenSSL behaves in this situation.

The overflow check must be present as a last defense to protect userspace. If userspace continues sending packets after an overflow occurred that will allow the decryption of the transmitted data, due to the way CTR mode (used by GCM) operates. Both openssl and gnutls do overflow checks, so applications which use them and switch to af_ktls would expect the same.

fridex commented 8 years ago

The overflow check must be present as a last defense to protect userspace. If userspace continues sending packets after an overflow occurred that will allow the decryption of the transmitted data, due to the way CTR mode (used by GCM) operates. Both openssl and gnutls do overflow checks, so applications which use them and switch to af_ktls would expect the same.

OK, so there should be present checks in OpenSSL/GnuTLS if the overflow occurs in AF_KTLS and handling would be switched back to OpenSSL/GnuTLS - so we could merge this after #62 due to tls_err_abort()