Add TLS support #102
Add TLS support #102
Conversation
Codecov Report
@@ Coverage Diff @@
## development #102 +/- ##
===============================================
+ Coverage 88.20% 89.75% +1.55%
===============================================
Files 30 34 +4
Lines 4789 5281 +492
===============================================
+ Hits 4224 4740 +516
+ Misses 565 541 -24
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
src/keria/app/agenting.py
Outdated
| @@ -108,6 +110,31 @@ def setup(name, bran, adminPort, bootPort, base='', httpPort=None, configFile=No | |||
| return doers | |||
|
|
|||
|
|
|||
| def createHttpServer(port, app, keypath, certpath, cafilepath): | |||
There was a problem hiding this comment.
This method probably needs a test to get the patch coverage up
| @@ -93,7 +95,7 @@ def setup(name, bran, adminPort, bootPort, base='', httpPort=None, configFile=No | |||
| ending.loadEnds(agency=agency, app=happ) | |||
| indirecting.loadEnds(agency=agency, app=happ) | |||
|
|
|||
| server = http.Server(port=httpPort, app=happ) | |||
| server = createHttpServer(httpPort, happ, keypath, certpath, cafilepath) | |||
There was a problem hiding this comment.
If you are going to change this to HTTPS, you will need to change the way you expose the OOBIs so that they are also HTTPs. Otherwise others will not be able to resolve the oobis for this Agent.
This is brings to mind that we might need multiple cert options because users will probably want to expose this endpoint to the world on one domain name and expose the Admin API on another domain name. Heck, we may need 3.
There was a problem hiding this comment.
Does Rodolfo's PR #91 address this need?
There was a problem hiding this comment.
No it does not. You will have to update OobiResourceEnd.on_get to change what scheme (currently hardcoded to http) is used when generating OOBIs, based on the configuration of these flags. Endpoints that are returned when OOBIs are resolved are controlled by configuration files so they are fine, but not OOBIs themselves.
There was a problem hiding this comment.
I just hit this problem. I'm getting a socket.gaierror: [Errno -2] Name does not resolve when resolving an HTTPS OOBI.
I'll get a PR submitted on this ASAP.
There was a problem hiding this comment.
I was able to use the changes from #91 for a fully HTTPS KERIA. I haven't yet tried switching my witness over to HTTPS. That's my next task.
There was a problem hiding this comment.
You still have not address my change request. That change needs to be made before this PR can be accepted.
There was a problem hiding this comment.
Do you want to merge #103 and have me re-base this on development or should I just pull those changes into this branch?
There was a problem hiding this comment.
#103 is a mess at the moment, pulling in old changes or something. And tests are broken in it.
There was a problem hiding this comment.
Ok, I'll pull just the changes I need into this PR.
There was a problem hiding this comment.
Those changes are now committed to this PR.
|
To get the patch coverage up higher I need to add test coverage for the I could pull in test certificates like Sam did in the hio library here: https://github.com/ioflo/hio/blob/master/tests/core/tcp/test_tcp.py#L657 If you'd like me to do that let me know and I will. Otherwise the overall test coverage appears to be increasing, yet the patch coverage is still around 50%. |
|
seems that the codecov reports that is shown at the top of the PR gives different results that the one used to enforce the checks. |
|
I close PR #103 in favor of this one that includes all the changes |
|
Is anything more needed for this PR to be merged? |
This adds createHttpServer to create an HTTPS server with
hio.core.tcp.ServerTls.