Use SSL by default #122
Use SSL by default #122
Conversation
package/nginx-https.conf
Outdated
| @@ -10,7 +10,7 @@ server { | |||
| error_log /var/log/nginx/rmt_https_error.log; | |||
| root /usr/share/rmt/public; | |||
|
|
|||
| ssl_certificate /usr/share/rmt/ssl/rmt-server.pem; | |||
| ssl_certificate /usr/share/rmt/ssl/rmt-server.crt; | |||
There was a problem hiding this comment.
@ikapelyukhin we are suggesting users of nginx to switch to TLS1.2 only. See line 99
Would that make sense to align RMT config?
There was a problem hiding this comment.
I'm not sure if TLSv1.2 would work with openssl version that is on SLES12 installation media, but for all modern use cases -- sure, it makes sense.
package/nginx-https.conf
Outdated
| @@ -12,6 +12,7 @@ server { | |||
|
|
|||
| ssl_certificate /usr/share/rmt/ssl/rmt-server.crt; | |||
| ssl_certificate_key /usr/share/rmt/ssl/rmt-server.key; | |||
| ssl_protocols TLSv1.2 TLSv1.3; | |||
There was a problem hiding this comment.
@kalabiyau I've thrown in TLSv1.3 as well, just for the future. Should the cipher suite also be modified?
There was a problem hiding this comment.
I think default still makes sense and there was no specific advise from security team on that bit. So, I suggest we keep it this way for now.
# ssl_ciphers HIGH:!aNULL:!MD5;
ikapelyukhin
force-pushed
the
ssl_packaging_changes
branch
from
5e8c2f5
to
94b6ffe
Compare
ikapelyukhin
force-pushed
the
ssl_packaging_changes
branch
from
94b6ffe
to
d6159a6
Compare
To make sure there is no conflict when migrating from SMT.
public/tools/clientSetup4RMT.sh
Outdated
|
|
||
| function usage() | ||
| { | ||
| if [ -n "$1" ] ; then |
There was a problem hiding this comment.
What is this? Is this required?
There was a problem hiding this comment.
Should print the first argument to STDERR if one is present, but yeah -- it seems like it isn't needed, considering that no arguments are allowed.
| exit 1 | ||
| fi | ||
|
|
||
| if [ ! -x $CP ]; then |
There was a problem hiding this comment.
Are the checks for cp and cat really required? Since they are in coreutils and I can not imagine a generic system without this package
There was a problem hiding this comment.
It's a bit too paranoid for my taste, but this script is borrowed from SMT package with minimal changes (older SLE11 openssl stuff and suse_register references removed) -- I don't see a reason to change this.
We are considering either moving this script to SUSEConnect package or implementing a feature in SUSEConnect itself to allow for importing certificates.
There was a problem hiding this comment.
Yes true. Ok than. But its somewhat funny cp is checked but mktemp is ok too use without checking even its also in coreutils. Anyway a connect implementation is preferable I guess.
| fi | ||
| fi | ||
|
|
||
| if [ ! -x $RM ]; then |
There was a problem hiding this comment.
same here; rm and chmod both also in coreutils
| access_log /var/log/nginx/rmt_http_access.log; | ||
| error_log /var/log/nginx/rmt_http_error.log; | ||
| root /usr/share/rmt/public; | ||
|
|
There was a problem hiding this comment.
Why not white list the tools/clientSetup4RMT.sh only and redirect everything else to https?
There was a problem hiding this comment.
is the clientSetup4RMT.sh the only usecase for plain http, or should the repository data also be available over http?
There was a problem hiding this comment.
@felixsch What do you mean specifically? It doesn't do a redirect at the moment, but otherwise that's what it does.
@digitaltom That's up for discussion. SMT does allow access to RPMs over plain HTTP. If you want to learn how many users do that instead of using SUSEConnect -- we could disable access over HTTP and see how many users complain 😬
There was a problem hiding this comment.
@digitaltom actually asked the question behind my statement. If no access to HTTP should be allowed, why not redirect to HTTPS everything except the clientSetup4RMT.sh.
There was a problem hiding this comment.
@kalabiyau Do you have any thoughts on this? Should the RPMs be accessible over plain HTTP?
SMT serves RPMs over HTTP, at the moment RMT does the same -- only the API is restricted to HTTPS.
There was a problem hiding this comment.
@ikapelyukhin as you described to me that atm we have:
registration would by default offer https:// urls
and plain access is allowed to the content via http://
I believe it is a sane setup.
|
|
||
| location / { | ||
| try_files $uri/index.html $uri.html $uri @rmt_app; | ||
| autoindex off; |
There was a problem hiding this comment.
Actually, I guess it should be off for plain HTTP version. It should list only repos and maybe tools directories.
There was a problem hiding this comment.
OK, I've disabled autoindex for plain HTTP instead.
ikapelyukhin
force-pushed
the
ssl_packaging_changes
branch
from
6a4c7fb
to
f67cb25
Compare
Now RMT would serve
clientSetup4RMT.shscript at http://rmt-hostname/tools/clientSetup4RMT.shCA certificate will be available at https://rmt-hostname/rmt.crt