-
Notifications
You must be signed in to change notification settings - Fork 45
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.
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
Use SSL by default #122
Use SSL by default #122
Changes from 3 commits
ebe2b65
2d4aaf5
e064b5f
d6159a6
7825817
4143376
b59df6f
f963a91
af0f5c8
6b12dc1
f67cb25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
server { | ||
listen 80 default; | ||
server_name rmt; | ||
access_log /var/log/nginx/rmt_http_access.log; | ||
error_log /var/log/nginx/rmt_http_error.log; | ||
root /usr/share/rmt/public; | ||
|
||
location / { | ||
autoindex on; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
upstream rmt { | ||
server localhost:4224; | ||
} | ||
|
||
server { | ||
listen 443 ssl; | ||
server_name rmt; | ||
|
||
access_log /var/log/nginx/rmt_https_access.log; | ||
error_log /var/log/nginx/rmt_https_error.log; | ||
root /usr/share/rmt/public; | ||
|
||
ssl_certificate /usr/share/rmt/ssl/rmt-server.crt; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
ssl_certificate_key /usr/share/rmt/ssl/rmt-server.key; | ||
ssl_protocols TLSv1.2 TLSv1.3; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK! 👍 |
||
|
||
location / { | ||
try_files $uri/index.html $uri.html $uri @rmt_app; | ||
autoindex off; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is it off on https? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I've disabled autoindex for plain HTTP instead. |
||
} | ||
|
||
location /repo { | ||
autoindex on; | ||
} | ||
|
||
location @rmt_app { | ||
proxy_pass http://rmt; | ||
proxy_redirect off; | ||
proxy_read_timeout 600; | ||
|
||
proxy_set_header Host $http_host; | ||
proxy_set_header X-Forwarded-Proto $scheme; | ||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; | ||
proxy_set_header X-Forwarded-Ssl on; | ||
proxy_set_header X-Real-IP $remote_addr; | ||
} | ||
|
||
# An alias to RMT CA certificate, so that it can be downloaded to client machines. | ||
location /rmt.crt { | ||
alias /usr/share/rmt/ssl/rmt-ca.crt; | ||
} | ||
} |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not white list the
tools/clientSetup4RMT.sh
only and redirect everything else to https?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.