-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate DTLS message before decryption #59
base: master
Are you sure you want to change the base?
Conversation
Currently, 1 test is failing and I don't know the reason for it. |
data class VerificationResult(val isValid: Boolean, val message: String) | ||
|
||
fun verifyRecord(encBuffer: ByteBuffer): VerificationResult { | ||
val buffer = clone(encBuffer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use Memory
class for copying encBuffer into. And remember to release it.
Copying function could be:
val intermediateBuffer: ByteBuffer = memory.getByteBuffer(0, encBuffer.remaining().toLong())
intermediateBuffer.put(this)
Point is to use put
function from ByteBuffer class. Create it in BytesExtensions
(remember about unit tests ;) )
} | ||
} | ||
|
||
private fun clone(original: ByteBuffer): ByteBuffer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there is already copy extension function, so lets improve that one.
fun verifyRecord(encBuffer: ByteBuffer): VerificationResult { | ||
val buffer = clone(encBuffer) | ||
val result = MbedtlsApi.mbedtls_ssl_check_record(sslContext, buffer, buffer.remaining()) | ||
return if (result == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should also check for MBEDTLS_ERR_SSL_UNEXPECTED_RECORD
return if (result == 0) { | |
return if (result == 0 || MBEDTLS_ERR_SSL_UNEXPECTED_RECORD) { |
@@ -78,6 +78,7 @@ internal object MbedtlsApi { | |||
external fun mbedtls_ssl_get_peer_cid(sslContext: Pointer, enabled: Pointer, peerCid: Pointer, peerCidLen: Pointer): Int | |||
external fun mbedtls_ssl_context_save(sslContext: Pointer, buf: ByteArray, bufLen: Int, outputLen: ByteArray): Int | |||
external fun mbedtls_ssl_context_load(sslContext: Pointer, buf: ByteArray, len: Int): Int | |||
external fun mbedtls_ssl_check_record(sslContext: Pointer, buf: ByteBuffer, bufLen: Int): Int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets use Memory
class instead of ByteBuffer, mainly because we need to copy.
external fun mbedtls_ssl_check_record(sslContext: Pointer, buf: ByteBuffer, bufLen: Int): Int | |
external fun mbedtls_ssl_check_record(sslContext: Pointer, buf: Memory, bufLen: Int): Int |
@@ -123,6 +124,17 @@ class SslContextTest { | |||
assertEquals("perse", serverSession.decrypt(encryptedDtls2, noSend).decodeToString()) | |||
} | |||
|
|||
@Test | |||
fun `should verify session is valid authentic and decrypt`() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create unit test that will verifyRecord as false
@@ -164,6 +164,27 @@ class SslSession internal constructor( | |||
plainBuffer.limit(size + plainBuffer.position()) | |||
} | |||
|
|||
data class VerificationResult(val isValid: Boolean, val message: String) | |||
|
|||
fun verifyRecord(encBuffer: ByteBuffer): VerificationResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, should we name it checkRecord
to be in line with mbedtls naming?
@@ -164,6 +164,27 @@ class SslSession internal constructor( | |||
plainBuffer.limit(size + plainBuffer.position()) | |||
} | |||
|
|||
data class VerificationResult(val isValid: Boolean, val message: String) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think here we could use sealed interface
with two implementing classes, for example: Valid
ond Invalid
Place is at the bottom of this class.
@@ -276,6 +276,11 @@ class DtlsServer( | |||
fun decrypt(encPacket: ByteBuffer): ReceiveResult { | |||
scheduledTask.cancel(false) | |||
try { | |||
val (isValid, message) = ctx.verifyRecord(encPacket) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not need to check record before each decrypt. Only when loading session.
Problem:
Validate DTLS message before decryption
Why?
Check whether a buffer contains a valid and authentic record that has not been seen before.
Solution:
Use a function to check validity before decryption, Link to mbedtls