-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
add letsencrypt to Gitea #4189
add letsencrypt to Gitea #4189
Changes from 3 commits
aa3fccb
2771df7
dd01b57
ce3840f
95a4191
c56d4a2
1d0097a
caa2d3a
6bbf48c
42411f8
e5afbb9
6fcb86b
f58c5b4
fd0103f
f09fb9c
75dd8ed
89c5e1c
bfe3769
c195fe1
ceebba5
5c57c62
9169a46
f961204
7a79c2b
60708ca
34b5519
be6b426
793c460
aa80181
77a65fa
d3cbc0c
9112ba2
9042772
3d84f1f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
package cmd | ||
|
||
import ( | ||
"crypto/tls" | ||
"fmt" | ||
"net" | ||
"net/http" | ||
|
@@ -22,6 +23,7 @@ import ( | |
"github.com/Unknwon/com" | ||
context2 "github.com/gorilla/context" | ||
"github.com/urfave/cli" | ||
"golang.org/x/crypto/acme/autocert" | ||
ini "gopkg.in/ini.v1" | ||
) | ||
|
||
|
@@ -71,6 +73,24 @@ func runHTTPRedirector() { | |
} | ||
} | ||
|
||
func runLetsEncrypt(listenAddr, domain string, m http.Handler) error { | ||
certManager := autocert.Manager{ | ||
Prompt: autocert.AcceptTOS, | ||
HostPolicy: autocert.HostWhitelist(domain), | ||
Cache: autocert.DirCache("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. I think this directory should come from 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. Also I think there should be option to specify 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. updated with your changes |
||
} | ||
go http.ListenAndServe(":80", certManager.HTTPHandler(nil)) // all traffic coming into HTTP will be redirect to HTTPS automatically | ||
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. Shouldn't this also only listen on listenAddr ? 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. for the HTTP validation that LetsEncrypt does, their servers need to be able to access port 80 on the server requesting the certificate. 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. Maybe you should just write 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. updated 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. @flufmonster yes but listening only on listenAddr+":80" should be enough since it will be the facing interface also for web access. If I remember listenAddr is by default 0.0.0.0 so by default we have the same comportment but if someone want to listen only to one interface via configuring listenAddr we shouldn't listen on all interface even for LE. 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. Thanks @sapk I've updated this so it only listens on listenAddr instead of 0.0.0.0 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 this should be handled in same code as http to https functionality already built into gitea |
||
// required for letsencrypt validation | ||
server := &http.Server{ | ||
Addr: listenAddr, | ||
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. AFAIK Let's Encrypt enforces port 443 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. Let's Encrypt only enforces a port when requesting the certificate. 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. The HTTP port is not really required, the autocert HTTP handler does only a simple redirect to HTTPS, so this port 443 is enforced by Let's Encrypt. 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. The HTTP port is required. The HTTP handler doesn't just do a simple redirect it also handles the HTTP-01 validation per the acme standard: https://tools.ietf.org/html/draft-ietf-acme-acme-07#section-8.3 and here is the code https://github.com/golang/crypto/blob/master/acme/autocert/autocert.go#L333 I think what you are thinking of is the TLS-SNI challenge which was disabled permanently due to security issues. A new version of TLS only challenge is being worked on but it is still only being discussed on mailing lists. 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. You are right on that, but since you are not defining a fallback handler HTTPS must run on 443: https://github.com/golang/crypto/blob/master/acme/autocert/autocert.go#L323-L326 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've just setup a fallback handler to redirect to AppURL so that the user is redirected to the correct place. |
||
Handler: m, | ||
TLSConfig: &tls.Config{ | ||
GetCertificate: certManager.GetCertificate, | ||
}, | ||
} | ||
return server.ListenAndServeTLS("", "") | ||
} | ||
|
||
func runWeb(ctx *cli.Context) error { | ||
if ctx.IsSet("config") { | ||
setting.CustomConf = ctx.String("config") | ||
|
@@ -147,6 +167,8 @@ func runWeb(ctx *cli.Context) error { | |
go runHTTPRedirector() | ||
} | ||
err = runHTTPS(listenAddr, setting.CertFile, setting.KeyFile, context2.ClearHandler(m)) | ||
case setting.LetsEncrypt: | ||
err = runLetsEncrypt(listenAddr, setting.Domain, context2.ClearHandler(m)) | ||
case setting.FCGI: | ||
listener, err := net.Listen("tcp", listenAddr) | ||
if err != nil { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,7 +82,7 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. | |
|
||
## Server (`server`) | ||
|
||
- `PROTOCOL`: **http**: \[http, https, fcgi, unix\] | ||
- `PROTOCOL`: **http**: \[http, https, fcgi, unix, letsencrypt\] | ||
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. Then if you use 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. updated |
||
- `DOMAIN`: **localhost**: Domain name of this server. | ||
- `ROOT_URL`: **%(PROTOCOL)s://%(DOMAIN)s:%(HTTP\_PORT)s/**: | ||
Overwrite the automatically generated public URL. | ||
|
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.
This HAS to be behind its own setting (
LETSENCRYPT_ACCEPT_TOS
) 😱 People will not read the comments...