Skip to content

Commit

Permalink
caddytls: Add Caddyfile support for on-demand permission module (close
Browse files Browse the repository at this point in the history
  • Loading branch information
mholt committed Apr 22, 2024
1 parent 9f97df2 commit 6a02999
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 8 deletions.
25 changes: 25 additions & 0 deletions caddyconfig/httpcaddyfile/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,34 @@ func parseOptOnDemand(d *caddyfile.Dispenser, _ any) (any, error) {
if ond == nil {
ond = new(caddytls.OnDemandConfig)
}
if ond.PermissionRaw != nil {
return nil, d.Err("on-demand TLS permission module (or 'ask') already specified")
}
perm := caddytls.PermissionByHTTP{Endpoint: d.Val()}
ond.PermissionRaw = caddyconfig.JSONModuleObject(perm, "module", "http", nil)

case "permission":
if !d.NextArg() {
return nil, d.ArgErr()
}
if ond == nil {
ond = new(caddytls.OnDemandConfig)
}
if ond.PermissionRaw != nil {
return nil, d.Err("on-demand TLS permission module (or 'ask') already specified")
}
modName := d.Val()
modID := "tls.permission." + modName
unm, err := caddyfile.UnmarshalModule(d, modID)
if err != nil {
return nil, err
}
perm, ok := unm.(caddytls.OnDemandPermission)
if !ok {
return nil, d.Errf("module %s (%T) is not an on-demand TLS permission module", modID, unm)
}
ond.PermissionRaw = caddyconfig.JSONModuleObject(perm, "module", modName, nil)

case "interval":
if !d.NextArg() {
return nil, d.ArgErr()
Expand Down
10 changes: 2 additions & 8 deletions caddyconfig/httploader.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,19 +181,13 @@ func (hl HTTPLoader) makeClient(ctx caddy.Context) (*http.Client, error) {
if err != nil {
return nil, fmt.Errorf("getting server identity credentials: %v", err)
}
if tlsConfig == nil {
tlsConfig = new(tls.Config)
}
tlsConfig.Certificates = certs
tlsConfig = &tls.Config{Certificates: certs}

Check failure on line 184 in caddyconfig/httploader.go

View workflow job for this annotation

GitHub Actions / lint (linux)

G402: TLS MinVersion too low. (gosec)
} else if hl.TLS.ClientCertificateFile != "" && hl.TLS.ClientCertificateKeyFile != "" {
cert, err := tls.LoadX509KeyPair(hl.TLS.ClientCertificateFile, hl.TLS.ClientCertificateKeyFile)
if err != nil {
return nil, err
}
if tlsConfig == nil {
tlsConfig = new(tls.Config)
}
tlsConfig.Certificates = []tls.Certificate{cert}
tlsConfig = &tls.Config{Certificates: []tls.Certificate{cert}}

Check failure on line 190 in caddyconfig/httploader.go

View workflow job for this annotation

GitHub Actions / lint (linux)

G402: TLS MinVersion too low. (gosec)
}

// trusted server certs
Expand Down
12 changes: 12 additions & 0 deletions modules/caddytls/ondemand.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"go.uber.org/zap"

"github.com/caddyserver/caddy/v2"
"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
)

func init() {
Expand Down Expand Up @@ -117,6 +118,17 @@ func (PermissionByHTTP) CaddyModule() caddy.ModuleInfo {
}
}

// UnmarshalCaddyfile implements caddyfile.Unmarshaler.
func (p *PermissionByHTTP) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
if !d.Next() {
return nil
}
if !d.AllArgs(&p.Endpoint) {
return d.ArgErr()
}
return nil
}

func (p *PermissionByHTTP) Provision(ctx caddy.Context) error {
p.logger = ctx.Logger()
p.replacer = caddy.NewReplacer()
Expand Down

2 comments on commit 6a02999

@mohammed90
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mholt, I believe this is false-positive, but the gosec devs disagree: securego/gosec#1054. I disagree with them, but you're the expert 🤷🏼

@mholt
Copy link
Member Author

@mholt mholt commented on 6a02999 Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mohammed90 Hmm, yes, this is a false positive (the change is purely syntactic, not a logic change) and I disagree with them. In fact I think their conclusion is more dangerous. I'll comment on that issue...

Please sign in to comment.