Skip to content
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

[3.2] DTLS support + optional ENet encryption #35091

Merged
merged 7 commits into from
May 5, 2020

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Jan 13, 2020

DTLS

  • New UDPServer class, support for UDP connected socket (i.e. bound to a specific receive/send address).
  • New PacketPeerDTLS and DTLSServer classes (+ implementation via mbedtls module).
  • See old report for more details.

ENet

  • New implementation of custom godot sockets in thirdparty/enet/godot.cpp to allow optional socket upgrade to DTLS.
  • NetworkedMultiplayerENet now supports optional DTLS encryption via the set_dtls_enabled function. Allows setting key/certificate/verify options.
# server
var peer = NetworkedMultiplayerENet.new()
var key = load("key.key") # Required for server
var cert = load("cert.crt") # Required for server
peer.set_dtls_enabled(true)
peer.set_dtls_key(key)
peer.set_dtls_certificate(cert)
peer.create_server(port, 4)

# client
var peer = NetworkedMultiplayerENet.new()
peer.set_dtls_enabled(true)
peer.set_dtls_verify(true) # Off by default... should we change it?
var cert = load("cert.crt")
peer.set_dtls_certificate(cert)
peer.create_client(host, port) # If verify is on, host must match the one in cert.

Closes #19110

@Faless Faless added this to the 4.0 milestone Jan 13, 2020
@Faless Faless requested review from akien-mga, hpvb and a team as code owners January 13, 2020 17:20
@@ -61,7 +61,7 @@ class NetSocket : public Reference {
virtual Error connect_to_host(IP_Address p_addr, uint16_t p_port) = 0;
virtual Error poll(PollType p_type, int timeout) const = 0;
virtual Error recv(uint8_t *p_buffer, int p_len, int &r_read) = 0;
virtual Error recvfrom(uint8_t *p_buffer, int p_len, int &r_read, IP_Address &r_ip, uint16_t &r_port) = 0;
virtual Error recvfrom(uint8_t *p_buffer, int p_len, int &r_read, IP_Address &r_ip, uint16_t &r_port, bool peek = false) = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be p_peek.

@@ -155,6 +158,9 @@ void register_core_types() {
ClassDB::register_class<StreamPeerTCP>();
ClassDB::register_class<TCP_Server>();
ClassDB::register_class<PacketPeerUDP>();
ClassDB::register_class<UDPServer>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have TCP_Server and UDPServer using different naming conventions. The former should be fixed, breaking compat in 4.0, I guess?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The former should be fixed, breaking compat in 4.0, I guess?

Yeah, I agree, TCP_Server should become TCPServer

@@ -38,6 +38,53 @@ static void my_debug(void *ctx, int level,
fflush(stdout);
}

void SSLContextMbedTLS::print_mbedtls_error(int ret) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be p_ret.

virtual Error bind(IP_Address p_ip, uint16_t p_port) = 0;
virtual Error sendto(const uint8_t *p_buffer, int p_len, int &r_sent, IP_Address p_ip, uint16_t p_port) = 0;
virtual Error recvfrom(uint8_t *p_buffer, int p_len, int &r_read, IP_Address &r_ip, uint16_t &r_port) = 0;
virtual int set_option(ENetSocketOption option, int value) = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p_value.

@Malkverbena
Copy link

Due the fact the Godot 4.0 will take a long tike to became stable again, is there any chance to to see DTLS support and ENet encryption on 3.2 branch?

@Calinou

This comment has been minimized.

@Faless Faless force-pushed the dtls/enet branch 2 times, most recently from 19fddd1 to d3eae91 Compare February 11, 2020 16:32
@Faless
Copy link
Collaborator Author

Faless commented Feb 11, 2020

@akien-mga I think this is ready now. I shall write documentation along with the post (we might want to wait for that, hopefully tomorrow).
It is also potentially cherry-pickable, as it's unlikely to cause regressions except for regular ENet (which is at low risk IMO, but I'll run more tests with this rebase), so we might test it for a while and then merge it with a bit "EXPERIMENTAL" notice in the docs.

@Faless
Copy link
Collaborator Author

Faless commented Feb 11, 2020

meh... I failed the rebase... will update ASAP

@Faless Faless requested a review from a team as a code owner February 16, 2020 00:10
@Faless Faless changed the base branch from master to 3.2 February 17, 2020 11:08
@Faless Faless force-pushed the dtls/enet branch 2 times, most recently from 0616ddc to ffe3fe5 Compare February 17, 2020 11:22
@Faless Faless changed the title DTLS support + optional ENet encryption [3.2] DTLS support + optional ENet encryption Feb 17, 2020
@akien-mga akien-mga modified the milestones: 4.0, 3.2 Feb 18, 2020
UDP sockets can be "connected" to filter packets from a specific source.
In case of a bound socket (e.g. server), a new socket can be created on
the same address/port that will receive all packets that are not
filtered by a more specific socket (e.g. the previously connect socket).

This way, a UDPServer can listen to new packets, and return a new
PacketPeerUDP when receiving one, knowing that is a "new client".
Custom instance implementation via the mbedtls module.
Non-DTLS implementation uses plain NetSocket for performance as before.
@clofresh
Copy link
Contributor

This is awesome! I've always loved enet's simplicity but the lack of encryption was a big negative for it. Would it be possible to extract the server part into a standalone C library so we can make lightweight servers in other languages?

@Faless
Copy link
Collaborator Author

Faless commented Feb 26, 2020

Would it be possible to extract the server part into a standalone C library so we can make lightweight servers in other languages?

Well, not really, the DTLS implementation is in C++ and largely relies on Godot data structures, if you need a C implementation you are probably better off making one from scratch.
If you are fine with C++ you would still need to replace or include quite a few things from core Ref/PoolVector/String/PacketPeer/etc.

@rieniter
Copy link

hello, I can't compile this module from 3.2 version
2020-02-29_162245

@Faless
Copy link
Collaborator Author

Faless commented Feb 29, 2020 via email

@rieniter
Copy link

What do you mean by "from the 3.2 version"? Did you check out my branch or did you try to apply the patch to 3.2? It seems that you are missing some changes. What build options are you using?

On Sat, Feb 29, 2020, 11:08 rieniter @.***> wrote: hello, I can't compile this module from 3.2 version [image: 2020-02-29_162245] https://user-images.githubusercontent.com/34160378/75605513-17819d80-5b16-11ea-8212-11b243c9d8df.png — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#35091?email_source=notifications&email_token=AAM4C3SM7QD4DEEMF3PNQR3RFDPDPA5CNFSM4KGF4N6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENLV72Y#issuecomment-592928747>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAM4C3XYJPTQWPP5F6AYKKDRFDPDPANCNFSM4KGF4N6A .

Hello, thank you for your reply. I'm still a noober, so let me explain what i was done:
I have try to copy your branch to original godot 3.2 and then build it, but it look like have something more custom module error than above.
So, I checked and know that this PR was merged into master branch, and i decide to download only that enet module from master branch and copy to my godot 3.2 branch, now everything look good except above error

@Faless
Copy link
Collaborator Author

Faless commented Feb 29, 2020

I see, it won't work like that, you can either checkout my branch directly (e.g.):
git clone https://github.com/Faless/godot.git -b dtls/enet godot-dtls
Or apply the diff for this PR to the 3.2 code

@rieniter
Copy link

I see, it won't work like that, you can either checkout my branch directly (e.g.):
git clone https://github.com/Faless/godot.git -b dtls/enet godot-dtls
Or apply the diff for this PR to the 3.2 code

Thank you, it worked great !

@Malkverbena
Copy link

I have done lots of tests with DTLS in Godot 3.2. So far everything works pretty well. Is there any way to help on this? any test?? I would love to see PR merged.

@Calinou
Copy link
Member

Calinou commented Apr 30, 2020

@Malkverbena The master branch is currently undergoing refactoring, which means it may take a while for pull requests to be merged (even if this one targets 3.2).

@akien-mga akien-mga merged commit ef715f3 into godotengine:3.2 May 5, 2020
@akien-mga
Copy link
Member

Let's get this merged for 3.2.2-beta2 and tested further.

Thanks!

@extremety1989
Copy link

@akien-mga does it works for beta4?

@akien-mga
Copy link
Member

It's merged since beta2, so it's in beta4 too. Whether it works as you expect it to is for you as a user to ascertain ;)

@extremety1989
Copy link

@akien-mga does not work for me, i used certificate generator for certs, can it cause problemes ?

@extremety1989
Copy link

extremety1989 commented Jun 11, 2020

@akien-mga used same code from this site https://godotengine.org/article/enet-dtls-encryption

@akien-mga
Copy link
Member

Please open an issue with a sample project that can trigger the bug.

@Faless Faless deleted the dtls/enet branch July 13, 2021 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add easy encryption to HighLevelMultiplayer Api.
7 participants