On 19/03/2013, at 7:56 PM, Reyk Floeter <[email protected]> wrote:
> On Tue, Mar 19, 2013 at 05:57:16PM +1000, David Gwynne wrote:
>> this lets the code that picks the filenames to use for certificates
>> fall through to using the services name, instead of just the ip
>> addresses of the service.
>>
>> eg, if i have this in relayd.conf:
>>
>> relay "sslnews.eait.uq.edu.au" {
>> listen on 0.0.0.0 port 563 ssl
>> forward to <news> port 119 check send "" expect "200 *"
>> protocol "sslencap"
>> }
>>
>> i can have this on disk:
>>
>> /etc/ssl/private/sslnews.eait.uq.edu.au.key
>> /etc/ssl/sslnews.eait.uq.edu.au.crt
>>
>> and it works(tm).
>>
>> it makes it easier to separate the service (relayd) from the hosts
>> underlying configuration. imagine a pool of boxes doing ssl offloading
>> with a centrally managed relayd.conf.
>>
>> ok?
>>
>
> "better" is a definition based on your setup - using the ip-based
> scheme allows to use the same cert+key files for multiple relays
> running on the same ip but a different port. this is actually also
> very common and better for 50% of the other users :) but as long as
> you keep the current behavior and check the ip-based keys / certs
> first, like your diff does, it should be ok.
yeah, the port and host certs are preferred over the name ones.
in my mind the best solution would be to require the user to explicitly specify
the files in the configuration.
> but please wait with this diff for three reasons:
> - it conflicts with the bigger ssl inspection diff that should go in
> first (in next few days).
ok.
> - we need to look at SNI, it is highly demanded by many, and this
> might require adjustments to the configuration logic as well.
specifying multiple certs by name would make sense for that.
> - i would like to review the diff more carefully but i'll fly home in
> a few hours and de-jetlag afterwards before i'm able to do it.
while i was driving home i realised that i leak memory if a port/host cert
exists but its key doesnt. so yeah, it could do with some tweaks.
dlg
>
> reyk
>
>> Index: relay.c
>> ===================================================================
>> RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
>> retrieving revision 1.164
>> diff -u -p -r1.164 relay.c
>> --- relay.c 10 Mar 2013 23:32:53 -0000 1.164
>> +++ relay.c 19 Mar 2013 07:49:28 -0000
>> @@ -42,6 +42,7 @@
>> #include <pwd.h>
>> #include <event.h>
>> #include <fnmatch.h>
>> +#include <netdb.h>
>>
>> #include <openssl/ssl.h>
>>
>> @@ -81,6 +82,7 @@ void relay_ssl_readcb(int, short, void
>> void relay_ssl_writecb(int, short, void *);
>>
>> char *relay_load_file(const char *, off_t *);
>> +int relay_load_certfile(struct relay *, const char *);
>> static __inline int
>> relay_proto_cmp(struct protonode *, struct protonode *);
>> extern void bufferevent_read_pressure_cb(struct evbuffer *, size_t,
>> @@ -2352,10 +2354,38 @@ relay_load_file(const char *name, off_t
>> }
>>
>> int
>> +relay_load_certfile(struct relay *rlay, const char *cert)
>> +{
>> + char file[PATH_MAX];
>> +
>> + if (snprintf(file, sizeof(file),
>> + "/etc/ssl/%s.crt", cert) == -1)
>> + return (-1);
>> +
>> + if ((rlay->rl_ssl_cert = relay_load_file(file,
>> + &rlay->rl_conf.ssl_cert_len)) == NULL)
>> + return (-1);
>> +
>> + log_debug("%s: using certificate %s", __func__, file);
>> +
>> + if (snprintf(file, sizeof(file),
>> + "/etc/ssl/private/%s.key", cert) == -1)
>> + return -1;
>> +
>> + if ((rlay->rl_ssl_key = relay_load_file(file,
>> + &rlay->rl_conf.ssl_key_len)) == NULL)
>> + return (-1);
>> +
>> + log_debug("%s: using private key %s", __func__, file);
>> +
>> + return (0);
>> +}
>> +
>> +int
>> relay_load_certfiles(struct relay *rlay)
>> {
>> char certfile[PATH_MAX];
>> - char hbuf[sizeof("ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255")];
>> + char hbuf[NI_MAXHOST];
>> struct protocol *proto = rlay->rl_proto;
>> int useport = htons(rlay->rl_conf.port);
>>
>> @@ -2372,36 +2402,19 @@ relay_load_certfiles(struct relay *rlay)
>> if (print_host(&rlay->rl_conf.ss, hbuf, sizeof(hbuf)) == NULL)
>> return (-1);
>>
>> - if (snprintf(certfile, sizeof(certfile),
>> - "/etc/ssl/%s:%u.crt", hbuf, useport) == -1)
>> + if (snprintf(certfile, sizeof(certfile), "%s:%u",
>> + hbuf, useport) == -1)
>> return (-1);
>> - if ((rlay->rl_ssl_cert = relay_load_file(certfile,
>> - &rlay->rl_conf.ssl_cert_len)) == NULL) {
>> - if (snprintf(certfile, sizeof(certfile),
>> - "/etc/ssl/%s.crt", hbuf) == -1)
>> - return (-1);
>> - if ((rlay->rl_ssl_cert = relay_load_file(certfile,
>> - &rlay->rl_conf.ssl_cert_len)) == NULL)
>> - return (-1);
>> - useport = 0;
>> - }
>> - log_debug("%s: using certificate %s", __func__, certfile);
>> + if (relay_load_certfile(rlay, certfile) == 0)
>> + return (0);
>>
>> - if (useport) {
>> - if (snprintf(certfile, sizeof(certfile),
>> - "/etc/ssl/private/%s:%u.key", hbuf, useport) == -1)
>> - return -1;
>> - } else {
>> - if (snprintf(certfile, sizeof(certfile),
>> - "/etc/ssl/private/%s.key", hbuf) == -1)
>> - return -1;
>> - }
>> - if ((rlay->rl_ssl_key = relay_load_file(certfile,
>> - &rlay->rl_conf.ssl_key_len)) == NULL)
>> - return (-1);
>> - log_debug("%s: using private key %s", __func__, certfile);
>> + if (relay_load_certfile(rlay, hbuf) == 0)
>> + return (0);
>>
>> - return (0);
>> + if (relay_load_certfile(rlay, rlay->rl_conf.name) == 0)
>> + return (0);
>> +
>> + return (-1);
>> }
>>
>> static __inline int
>> Index: relayd.conf.5
>> ===================================================================
>> RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v
>> retrieving revision 1.132
>> diff -u -p -r1.132 relayd.conf.5
>> --- relayd.conf.5 29 Nov 2012 01:01:53 -0000 1.132
>> +++ relayd.conf.5 19 Mar 2013 07:49:28 -0000
>> @@ -665,6 +665,11 @@ If these files are not present, the rela
>> .Pa /etc/ssl/private/address.key
>> and
>> .Pa /etc/ssl/address.crt .
>> +If those files are not present, the relay will finally try to use
>> +.Pa /etc/ssl/private/name.key
>> +and
>> +.Pa /etc/ssl/name.crt
>> +where name is the name of relay service.
>> See
>> .Xr ssl 8
>> for details about SSL server certificates.
