Fabo/595 https #601
Fabo/595 https #601
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #601 +/- ##
========================================
Coverage 57.16% 57.16%
========================================
Files 33 33
Lines 1807 1807
========================================
Hits 1033 1033
Misses 693 693
Partials 81 81 |
|
Would you like some help with generating certificates as per the example in #595 |
|
@zmanian I would love your help, if you have some time to spend! :-) |
|
@mappum can you summarize the discussion that took place at full node regarding node child processes and https? |
|
So I feel there are something I need to fix here.
|
| pem.Encode(keyOut, pemBlockForKey(privKey)) | ||
| keyOut.Close() | ||
|
|
||
| validFor := time.Duration(365 * 24 * time.Hour) |
There was a problem hiding this comment.
time.Duration(8760 * time.Hour) // 365 * 24 = 8760
There was a problem hiding this comment.
Why change this if the compiler is just going to simplify it anyway?
There was a problem hiding this comment.
@mappum cool - obviously the compiler would simplify... I take it back! way clearer to leave as is :)
|
Where are we on this? |
|
Question: why don't use something like Nginx to terminate SSL https://docs.nginx.com/nginx/admin-guide/security-controls/terminating-ssl-http/ ? |
| os.IsNotExist(err) | ||
|
|
||
| if err != nil { | ||
| err = generateAndSaveCertificate(exPath) |
There was a problem hiding this comment.
do you use https://letsencrypt.org/ ? or is this just generates raw/unconfirmed certificate?
There was a problem hiding this comment.
These should not be certificates signed by a public certificate authority. They need to be generated locally.
| exPath := filepath.Dir(ex) | ||
|
|
||
| _, err = os.Stat(filepath.Join(exPath, "server.crt")) | ||
| os.IsNotExist(err) |
There was a problem hiding this comment.
why do this if you're not checking for the output (bool)?
| } | ||
| exPath := filepath.Dir(ex) | ||
|
|
||
| _, err = os.Stat(filepath.Join(exPath, "server.crt")) |
There was a problem hiding this comment.
const serverCertificateFile = "server.crt"
|
|
||
| _, err = os.Stat(filepath.Join(exPath, "server.crt")) | ||
| os.IsNotExist(err) | ||
| _, err = os.Stat(filepath.Join(exPath, "server.key")) |
There was a problem hiding this comment.
const serverKeyFile = "server.key"
|
The reason not to use nginx is that TLS is needed to secure the communication between voyager and the LCD rest server and we shouldn't ship nginx with voyager. In an enterprise setting other configuration might make sense and then the rest server can be started with the |
|
Thanks for explaining this. |
|
This code needs a non-trivial rebase and some new features before it can be merged. I'm done traveling for some days and i hopefully can get this ready shortly. |
|
As per conversation with @zmanian this PR will be closed and new one opened based on updated develop branch.. this PR's branch is going to be saved under |
* Don't run ValidateBasic on recheck (#601) (cherry picked from commit de3aaa1) * update changelog --------- Co-authored-by: Dev Ojha <[email protected]> Co-authored-by: Adam Tucker <[email protected]>
Add https to the REST server.
Closes: #595
Feedback welcome!
Should we add an option to set the certificate location?