On Mon, Aug 15, 2016 at 11:33:18PM +1000, Joel Sing wrote:
> On Monday 15 August 2016 13:04:43 Reyk Floeter wrote:
> > On Sat, Aug 13, 2016 at 02:57:14AM +1000, Joel Sing wrote:
> > > The following diff makes httpd stricter with respect to TLS configuration:
> > > 
> > > - Do not allow TLS and non-TLS to be configured on the same port.
> > > - Do not allow TLS options to be specified without a TLS listener.
> > > - Ensure that TLS options are the same when a server is specified on the
> > > 
> > >   same address/port.
> > > 
> > > Currently, these configurations are permitted but do not work as intended.
> > > 
> > > This also factors out (and reuses) the server matching code that was
> > > already duplicated.
> > > 
> > > ok?
> > 
> > - I think server_match() and server_tls_cmp() can both live in
> > server.c (server_match() somewhere close to server_foreach() - this
> > match function can be used for at least one other case outside of
> > parse.y).
> 
> I've moved server_tls_cmp() to server.c, however I'm not sure it makes sense 
> for server_match() since it operates on conf (a global declared in parse.y) 
> and I do not see any identical matching (the closest seems to be the code in 
> server_privinit(), but it still differs).
>  

you can use env-> instead of conf-> there.

I think server_privinit() could be reworked to use it.

In other words: I don't think that server_match() is something that is
specific to the parser, so it shouldn't be in the parse.y file.

Reyk

> > - As discussed before, for consistency with the config, please use
> > "tls" instead of "TLS" in the log messages.
> 
> Agreed, I'll handle this in a separate commit.
>  
> > FYI, The SNI diff doesn't like the tls_cert_file and tls_key_file
> > checks in server_tls_cmp(), as they now become valid, but they can be
> > removed/changed later.
> 
> Yes, they are independent diffs and we'll need to relax the restrictions 
> slightly when we enable SNI.
> 
> > Otherwise OK reyk@
> 
> Thanks.

-- 

Reply via email to