Proposal: Include Client Address in PSK Validation for Brute Force Detection

tonisole commented 1 year ago

Description

I am proposing a modification to the current PSK validation mechanism to include the client's address as part of the validation process. This change is crucial for implementing a Brute Force Detection mechanism in our system.

Currently, the PSK validation does not provide any information about the client attempting to connect. This lack of information makes it impossible to detect if a specific IP address is repeatedly trying to discover the Hint/PSK authorization, a common sign of a brute force attack.

var attempts = make(map[string]int) // Map of attempts for each IP address for a Brute Force Protection

PSK: func(hint []byte, addr net.Addr) ([]byte, error) {
    fmt.Printf("Server's hint: %s \n", hint)
    // *************** Brute Force Attack protection ***************
    // Check if the IP address is in the map, and the IP address has exceeded the limit
    if attempts[addr.(*net.UDPAddr).IP.String()] > 5 {
        return nil, fmt.Errorf("too many attempts from this IP address")
    }
    // Here I increment the number of attempts for this IP address
    attempts[addr.(*net.UDPAddr).IP.String()]++

I am using this library in conjunction with the go-coap library, and having the ability to implement this detection mechanism would significantly enhance the security of our system. By including the client's address in the PSK validation, we can monitor and limit the number of attempts from a single IP address, effectively preventing potential brute force attacks.

This draft is intended to open a discussion about the necessity and feasibility of this proposed change. I believe that with this modification, we can make our system more secure and robust against potential threats. I look forward to hearing your thoughts and suggestions on this matter.

Reference issue

No related issue

codecov[bot] commented 1 year ago

Codecov Report

Attention: Patch coverage is 66.66667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 79.98%. Comparing base (adec94a) to head (3e7a2ef).

Files	Patch %	Lines
flight4handler.go	0.00%	0 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #596 +/- ## ========================================== - Coverage 80.02% 79.98% -0.04% ========================================== Files 101 101 Lines 5306 5306 ========================================== - Hits 4246 4244 -2 - Misses 685 687 +2 Partials 375 375 ``` | [Flag](https://app.codecov.io/gh/pion/dtls/pull/596/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | | |---|---|---| | [go](https://app.codecov.io/gh/pion/dtls/pull/596/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | `80.01% <66.66%> (-0.04%)` | :arrow_down: | | [wasm](https://app.codecov.io/gh/pion/dtls/pull/596/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | `63.85% <66.66%> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Sean-Der commented 1 year ago

@at-wat @tonisole We do need to tag a /v3!

@hasheddan can I get a short write up on your changes? I say we do this change proposed here, and yours? I really would like to get pion/dtls stabilized soon :)

Sean-Der commented 5 months ago

Hi @tonisole

Sorry I didn't make more movement on this! I would love to see this get into pion/dtls

What do you think of making it non-PSK specific? I think this would be useful for certificates also?

tonisole commented 5 months ago

Hi @Sean-Der,

Thank you for your feedback and interest in getting this feature into pion/dtls.

I’ve thought about your suggestion and have implemented a more general solution that is easier to implement and provides several benefits:

General Solution: The callback mechanism I’ve implemented is not specific to PSK. It can be used for any type of connection, including certificate-based ones. This makes it versatile and applicable to various scenarios.

Improved Performance: By placing the callback mechanism before the authorization validation, we can speed up the discard process. This means that connection attempts can be rejected early, reducing the load on the server and improving overall performance.

Backwards Compatibility: The best part of this solution is that it can be backported to the current version of DTLS without breaking changes. This ensures that existing implementations remain stable and functional while gaining the benefits of the new feature.

If you agree, I will cancel this Pull Request in favor of the newer https://github.com/pion/dtls/pull/640 , more general, and easier-to-implement solution.

pion / dtls