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.

Sign up for GitHub

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

Jump to bottom

gcoap_forward_proxy: CoAPS support #18107

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

gcoap_forward_proxy: CoAPS support #18107

wants to merge 2 commits into from
+53 −25

Conversation

miri64
Copy link
Member

@miri64 miri64 commented May 14, 2022

Contribution description

Allows to deploy the forward proxy using CoAPS / proxying a CoAPS connection.

Testing procedure

Still needs some work, currently, the proxy reports a Bad Option error, when used with CoAPS...

Issues/PRs references

None

@miri64 miri64 added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label May 14, 2022
@github-actions github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: examples Area: Example Applications Area: network Area: Networking Area: sys Area: System labels May 14, 2022
@benpicco benpicco requested a review from janosbrodbeck May 14, 2022 13:38
@miri64 miri64 mentioned this pull request May 16, 2022
Merged
@miri64
Copy link
Member Author

miri64 commented Jul 28, 2022

Ok, I found now some hints on why this is not working:

  1. The number of DTLS peers is fixed to 1 by the gcoap_dtls example. A proxy of course has at least 2 :(
  2. If that is set to 2, I run into the problem, that the proxy runs basically completely in the gcoap thread and tries to use the blocking gcoap_send_req_tl when sending to upstream... It basically stays here in

    RIOT/sys/net/application_layer/gcoap/gcoap.c

    Line 1102 in fb3f1a2

    res = ztimer_msg_receive_timeout(ZTIMER_MSEC, &msg, timeout);
    when trying to establish the handshake, which blocks the gcoap thread. In the meantime the handshake messages from upstream come in, but are not handled in _on_sock_dtls_evt, until the timeout timed out... Maybe an asynchronous version of gcoap_send_req_tl will help here...

@benpicco
Copy link
Contributor

So the idea is that a cllient would connect to the proxy via CoAP and the proxy would forward the connection via CoAPs to the server that it proxies for? That is exactly what we need 😃

@miri64
Copy link
Member Author

miri64 commented Feb 28, 2024

So the idea is that a cllient would connect to the proxy via CoAP and the proxy would forward the connection via CoAPs to the server that it proxies for?

The main idea is to have a CoAPS connection at both ends, but yes, in theory it should also work that you have a (unsecure) CoAP connection at one end and a secure CoAPS connection at the other.

@miri64
Copy link
Member Author

miri64 commented Feb 28, 2024

Maybe the problem above might not be an issue for this use case, but until it is resolved, this can't be merged of course.

@benpicco
Copy link
Contributor

benpicco commented Mar 4, 2024 •
edited
Loading

I'm afraid I figured out why this does not work: In _tl_authenticate() we loop until timeout or a message is received.
This message is supposed to be sent by _on_sock_dtls_evt() which is the event callback for the DTLS socket. That callback is executed by event_loop(&_queue) in the GCoAP thread - the same event loop that handles the proxied request.

That means the event handler will never¹ be executed as _tl_authenticate() is blocking the event handler, we'd either need an async version of the DTLS handshake or move the proxy request to a separate thread.

[1] it will be executed after the timeout, but then it's too late

@miri64
Copy link
Member Author

miri64 commented Mar 4, 2024

Yes, that exactly the conclusion I came to in July according to my comment.

@mariemC mariemC mentioned this pull request Mar 11, 2024
Merged
@miri64 miri64 mentioned this pull request May 15, 2024
Merged
@miri64 miri64 force-pushed the gcoap_forward_proxy/enh/coaps-support branch from def4718 to 78154e3 Compare May 15, 2024 13:19
@github-actions github-actions bot removed the Area: examples Area: Example Applications label May 15, 2024
@miri64
Copy link
Member Author

miri64 commented May 15, 2024

With #20454 and #20554 now merged, this should be much easier to get running. Rebased and adopted to current master.

I only compile-tested so far, if you want to merge, please test functionality rigorously.

Nevertheless, I think this can get out of draft state now.

@miri64 miri64 marked this pull request as ready for review May 15, 2024 13:21
@miri64 miri64 requested a review from PeterKietzmann as a code owner May 15, 2024 13:21
benpicco
benpicco reviewed May 15, 2024
View reviewed changes
sys/include/net/coap.h Outdated Show resolved Hide resolved
@benpicco benpicco requested a review from fabian18 May 15, 2024 13:26
@benpicco
Copy link
Contributor

@mariemC want to give this a try?

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 15, 2024
@miri64 miri64 force-pushed the gcoap_forward_proxy/enh/coaps-support branch from 562f347 to 8181e26 Compare May 15, 2024 13:35
@miri64
Copy link
Member Author

miri64 commented May 15, 2024

Just noticed that I forgot about the new dependency to gcoap_forward_proxy_thread. I added that in the last slew of force pushes.

@riot-ci
Copy link

riot-ci commented May 15, 2024 •
edited
Loading

Murdock results

✔️ PASSED

194e676 gcoap_forward_proxy: add CoAPS support

Success Failures Total Runtime
10195 0 10196 15m:27s

Artifacts

@fabian18
Copy link
Contributor

We need to add DTLS capability to the gcoap example client, don`t we?

@miri64
Copy link
Member Author

miri64 commented May 31, 2024

We need to add DTLS capability to the gcoap example client, don`t we?

examples/gcoap_dlts should already suplly that.

@fabian18
Copy link
Contributor

fabian18 commented Jun 7, 2024

I see what is done in the server_init() function is also done for the client endpoint.

@mariemC
Copy link
Contributor

mariemC commented Jun 7, 2024

So I have tested the PR! and it works fine with Fabian's patch (in the comments he wrote)
I have used examples/gcoap_dtls, I have created two tap interfaces: tap0 for client and tap1 for the proxy and a remote server on my ubuntu machine: coap-server-gnutls -A fdea:dbee:f::1 -h Client_identity -k secretPSK
The client sets the proxy: coap proxy set coaps://[fe80::fccc:bff:fe49:2dfa%7]:5684
The proxy server adds the route to the remote server: nib route add 8 :: fe80::58da:6eff:fe50:5b94 (br0 local link)
These modules need to be added to the Makefile of the example for the server proxy:

  USEMODULE +=gcoap_forward_proxy
  USEMODULE +=gcoap_forward_proxy_thread

I also set DTLS_MAX_BUF to 1400, the message back from the remote server is large and it won't fit, therefore DTLS_MAX_BUF needs to be increased ( I set it incredibly large )
logs on the client:

coap get coaps://[fdea:dbee:f::1]:5684/.well-known/core
2024-06-07 15:07:38,003 # coap get coaps://[fdea:dbee:f::1]:5684/.well-known/core
2024-06-07 15:07:38,004 # gcoap_cli: sending msg ID 3860, 55 bytes
2024-06-07 15:07:38,005 # dtls_prepare_record(): encrypt using TLS_PSK_WITH_AES_128_CCM_8
2024-06-07 15:07:38,007 # nonce: (16 bytes):00000000  DE  77  29  BB  00  01  00  00  00  00  00  05  00  00  00  00
2024-06-07 15:07:38,009 # key: (16 bytes):00000000  15  A8  7C  90  40  7E  2E  C6  2D  30  62  E6  E0  90  9A  42
2024-06-07 15:07:38,010 # message: (71 bytes):00000000  00  01  00  00  00  00  00  05  1A  3D  18  A3  F5  3B  C0  B0
2024-06-07 15:07:38,012 # 00000010  CB  10  19  16  8E  DD  6E  BD  6E  E2  81  93  DD  E8  46  43
2024-06-07 15:07:38,014 # 00000020  D1  D4  21  95  56  9D  1D  F4  E0  66  3F  31  C3  23  B7  3F
2024-06-07 15:07:38,015 # 00000030  67  0A  E2  37  AA  55  89  EB  D0  5E  F4  82  A4  B7  13  20
2024-06-07 15:07:38,016 # 00000040  7C  31  97  59  C2  4A  7D
2024-06-07 15:07:38,017 # -= send header (13 bytes) =-
2024-06-07 15:07:38,018 # 00000000  17  FE  FD  00  01  00  00  00  00  00  05  00  47
2024-06-07 15:07:38,019 # -= send unencrypted (55 bytes) =-
2024-06-07 15:07:38,021 # 00000000  52  01  0F  14  47  AE  DD  16  21  63  6F  61  70  73  3A  2F
2024-06-07 15:07:38,022 # 00000010  2F  5B  66  64  65  61  3A  64  62  65  65  3A  66  3A  3A  31
2024-06-07 15:07:38,023 # 00000020  5D  3A  35  36  38  34  2F  2E  77  65  6C  6C  2D  6B  6E  6F
2024-06-07 15:07:38,023 # 00000030  77  6E  2F  63  6F  72  65
2024-06-07 15:07:38,024 # _on_sock_dtls_evt
2024-06-07 15:07:38,025 # gcoap_cli: no observer for /cli/stats
> 2024-06-07 15:07:38,025 # received message (146 bytes), starting with 'application_data', epoch 1
2024-06-07 15:07:38,026 # got 'application_data' epoch 1 sequence 5 (146 bytes)
2024-06-07 15:07:38,026 # dtls_handle_message: FOUND PEER
2024-06-07 15:07:38,027 # bitfield is f sequence base 3 rseqn 5
2024-06-07 15:07:38,028 # nonce (16 bytes):00000000  D5  BA  31  85  00  01  00  00  00  00  00  05  00  00  00  00
2024-06-07 15:07:38,029 # key (16 bytes):00000000  C4  4C  62  00  15  AB  9F  64  EC  EE  0A  31  C3  F2  94  78
2024-06-07 15:07:38,030 # ciphertext (125 bytes):00000000  93  8E  D6  6A  B4  79  99  C0  03  73  D3  5B  F3  92  15  08
2024-06-07 15:07:38,030 # 00000010  6A  45  53  CC  A3  06  53  24  A6  8C  43  2D  BE  8E  2D  D5
2024-06-07 15:07:38,031 # 00000020  88  14  32  7E  6C  A2  0D  CB  60  91  41  77  DA  27  4A  CF
2024-06-07 15:07:38,031 # 00000030  31  E1  4D  83  A8  06  F0  B8  D1  37  05  E7  C9  F1  C8  F7
2024-06-07 15:07:38,031 # 00000040  84  3B  CE  C1  B8  5D  E4  D7  3D  F5  14  2C  B8  34  48  8D
2024-06-07 15:07:38,032 # 00000050  AA  08  BA  75  D0  93  1A  F8  05  5B  B3  BA  8A  6A  65  13
2024-06-07 15:07:38,032 # 00000060  AC  2F  4A  E4  48  39  9F  AC  DB  F1  63  6E  39  01  31  63
2024-06-07 15:07:38,033 # 00000070  86  64  9C  71  B8  AE  67  B2  49  92  CC  AD  5B
2024-06-07 15:07:38,034 # decrypt_verify(): found 117 bytes cleartext
2024-06-07 15:07:38,034 # cleartext (117 bytes):00000000  52  45  0F  14  47  AE  C1  28  D1  03  69  FF  3C  2F  3E  3B
2024-06-07 15:07:38,035 # 00000010  74  69  74  6C  65  3D  22  47  65  6E  65  72  61  6C  20  49
2024-06-07 15:07:38,035 # 00000020  6E  66  6F  22  3B  63  74  3D  30  2C  3C  2F  74  69  6D  65
2024-06-07 15:07:38,035 # 00000030  3E  3B  69  66  3D  22  63  6C  6F  63  6B  22  3B  72  74  3D
2024-06-07 15:07:38,036 # 00000040  22  74  69  63  6B  73  22  3B  74  69  74  6C  65  3D  22  49
2024-06-07 15:07:38,036 # 00000050  6E  74  65  72  6E  61  6C  20  43  6C  6F  63  6B  22  3B  63
2024-06-07 15:07:38,036 # 00000060  74  3D  30  3B  6F  62  73  2C  3C  2F  61  73  79  6E  63  3E
2024-06-07 15:07:38,037 # 00000070  3B  63  74  3D  30
2024-06-07 15:07:38,037 # update bitfield is 3d new sequence base 5
2024-06-07 15:07:38,037 # -= receive header (13 bytes) =-
2024-06-07 15:07:38,037 # 00000000  17  FE  FD  00  01  00  00  00  00  00  05  00  85
2024-06-07 15:07:38,038 # -= receive unencrypted (117 bytes) =-
2024-06-07 15:07:38,038 # 00000000  52  45  0F  14  47  AE  C1  28  D1  03  69  FF  3C  2F  3E  3B
2024-06-07 15:07:38,038 # 00000010  74  69  74  6C  65  3D  22  47  65  6E  65  72  61  6C  20  49
2024-06-07 15:07:38,039 # 00000020  6E  66  6F  22  3B  63  74  3D  30  2C  3C  2F  74  69  6D  65
2024-06-07 15:07:38,039 # 00000030  3E  3B  69  66  3D  22  63  6C  6F  63  6B  22  3B  72  74  3D
2024-06-07 15:07:38,039 # 00000040  22  74  69  63  6B  73  22  3B  74  69  74  6C  65  3D  22  49
2024-06-07 15:07:38,040 # 00000050  6E  74  65  72  6E  61  6C  20  43  6C  6F  63  6B  22  3B  63
2024-06-07 15:07:38,040 # 00000060  74  3D  30  3B  6F  62  73  2C  3C  2F  61  73  79  6E  63  3E
2024-06-07 15:07:38,040 # 00000070  3B  63  74  3D  30
2024-06-07 15:07:38,040 # ** application data:
2024-06-07 15:07:38,040 # sock_dtls: decrypted message arrived
2024-06-07 15:07:38,041 # _on_sock_dtls_evt
2024-06-07 15:07:38,041 # gcoap: response Success, code 2.05, 105 bytes
2024-06-07 15:07:38,041 # </>;title="General Info";ct=0,</time>;if="clock";rt="ticks";title="Internal Clock";ct=0;obs,</async>;ct=0

@miri64
Copy link
Member Author

miri64 commented Jun 7, 2024

Hey! Just to let you both know: I won't be able to work on this until June 15th. But afterwards, I will review and, if applicable, apply all the patches you provide :-)

@benpicco
Copy link
Contributor

Can we close this now that #20454 is merged?

@miri64
Copy link
Member Author

miri64 commented Jul 29, 2024 •
edited
Loading

Can we close this now that #20454 is merged?

Huh? Proxying should not work without this PR, especially CoAPS<=>CoAPS and CoAP<=>CoAP proxying. CoAPS<=>CoAPS proxying should not work without this PR.

@miri64 miri64 force-pushed the gcoap_forward_proxy/enh/coaps-support branch from 8181e26 to 194e676 Compare July 29, 2024 14:21
@miri64
Copy link
Member Author

miri64 commented Jul 29, 2024

Rebased and finally addressed @fabian18's comments.

@miri64
Copy link
Member Author

miri64 commented Jul 29, 2024

These modules need to be added to the Makefile of the example for the server proxy:

  USEMODULE +=gcoap_forward_proxy
  USEMODULE +=gcoap_forward_proxy_thread

Since examples/gcoap does not contain gcoap_forward_proxy either, that's okay. gcoap_forward_proxy_thread should now be added automagically via sys/Makefile.dep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabian18 fabian18 fabian18 left review comments

@benpicco benpicco benpicco left review comments

@janosbrodbeck janosbrodbeck Awaiting requested review from janosbrodbeck

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann PeterKietzmann is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@miri64 @benpicco @riot-ci @fabian18 @mariemC