mediamtx icon indicating copy to clipboard operation
mediamtx copied to clipboard

feat: added TLS encryption support to API calls (HTTPS)

Open eravellaSC opened this issue 1 year ago • 5 comments

Solving #2491, I added support for TLS-encryption for the HTTP API calls. The gin server was already set up to do so, the only thing missing were the conf paths.

eravellaSC avatar Nov 07 '23 09:11 eravellaSC

Codecov Report

Merging #2658 (062e3a2) into main (5dae270) will decrease coverage by 0.10%. Report is 1 commits behind head on main. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2658      +/-   ##
==========================================
- Coverage   59.41%   59.31%   -0.10%     
==========================================
  Files         144      144              
  Lines       15238    15238              
==========================================
- Hits         9053     9038      -15     
- Misses       5549     5560      +11     
- Partials      636      640       +4     
Files Coverage Δ
internal/conf/conf.go 65.17% <ø> (ø)
internal/core/api.go 66.29% <100.00%> (ø)

... and 5 files with indirect coverage changes

:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

codecov[bot] avatar Nov 07 '23 18:11 codecov[bot]

I double checked the failing tests and coverage reports and they mention files and checks that I have not touched. It's a very small modification, and the codebase was already ready to accomodate this simple addon. Is the coverage slowing down the review process? I can dig deeper if it's needed.

eravellaSC avatar Nov 29 '23 15:11 eravellaSC

I am sorry to bother directly, @aler9, but I'd like to know if there is interest in adding this feature. If not I can close the PR.

eravellaSC avatar Jan 08 '24 09:01 eravellaSC

Hello @eravellaSC, of course there's interest, but the problem is that currently the API is missing a strong authentication system, therefore it's meant to be used either in a closed environment or behind a reverse proxy which provides authentication and TLS termination.

If i merge this feature without providing authentication, there's risk of giving the impression that the API is safe to use.

Furthermore, nowadays valid TLS certificates are generated by authorities that periodically query the corresponding domain through HTTP, and that would mean that some users may try to expose the API publicly. Since it's easy to scan the internet and detect running instances of MediaMTX, this would mean creating a Botnet.

Anyway, the authentication issue will be solved soon.

aler9 avatar Jan 08 '24 10:01 aler9

@aler9 Thanks for the update, I didn't think of that in that way. I'll wait for the auth to be merged then.

eravellaSC avatar Jan 08 '24 10:01 eravellaSC

replaced by #3280

aler9 avatar Apr 21 '24 15:04 aler9

This issue is mentioned in release v1.8.0 🚀 Check out the entire changelog by clicking here

github-actions[bot] avatar Apr 21 '24 15:04 github-actions[bot]