On Sun, Sep 06 2020, Theo Buehler <t...@theobuehler.org> wrote:
> On Sun, Sep 06, 2020 at 12:55:17AM +0200, Jeremie Courreges-Anglas wrote:
>> On Sat, Sep 05 2020, Theo Buehler <t...@theobuehler.org> wrote:
>> > I found this small diff useful more than once (admittedly for debugging).
>> > It allows specifying the protocols that may be used by ftp the same way
>> > as nc -Tprotocols works. For example:
>> >
>> > $ ftp -Sprotocols=tlsv1.2:tlsv1.1 https://example.com/file
>> >
>> > Perhaps someone else has use for it, too?
>> 
>> I think it's useful indeed.  ok jca@
>> 
>> One nit below,
>> 
>> > Index: ftp.1
>> > ===================================================================
>> > RCS file: /var/cvs/src/usr.bin/ftp/ftp.1,v
>> > retrieving revision 1.119
>> > diff -u -p -r1.119 ftp.1
>> > --- ftp.1  11 Feb 2020 18:41:39 -0000      1.119
>> > +++ ftp.1  5 Sep 2020 18:11:24 -0000
>> > @@ -261,6 +261,12 @@ Don't perform server certificate validat
>> >  Require the server to present a valid OCSP stapling in the TLS handshake.
>> >  .It Cm noverifytime
>> >  Disable validation of certificate times and OCSP validation.
>> > +.It Cm protocols Ns = Ns Ar protocol_list
>> > +Specify the supported TLS protocols that will be used by
>> > +.Nm
>> > +(see
>> > +.Xr tls_config_parse_protocols 3
>> > +for details).
>> >  .It Cm session Ns = Ns Ar /path/to/session
>> >  Specify a file to use for TLS session data.
>> >  If this file has a non-zero length, the session data will be read from 
>> > this file
>> > Index: main.c
>> > ===================================================================
>> > RCS file: /var/cvs/src/usr.bin/ftp/main.c,v
>> > retrieving revision 1.132
>> > diff -u -p -r1.132 main.c
>> > --- main.c 1 Sep 2020 12:33:48 -0000       1.132
>> > +++ main.c 1 Sep 2020 18:13:09 -0000
>> > @@ -209,6 +209,8 @@ char * const ssl_verify_opts[] = {
>> >    "noverifytime",
>> >  #define SSL_SESSION               8
>> >    "session",
>> > +#define SSL_PROTOCOLS             9
>> > +  "protocols",
>> >    NULL
>> >  };
>> >  
>> > @@ -220,6 +222,7 @@ process_ssl_options(char *cp)
>> >  {
>> >    const char *errstr;
>> >    long long depth;
>> 
>> (Should probably be an int.)
>>
>> > +  uint32_t protocols;
>> 
>> Could be moved below str (smaller size on lp64, see style(9)).
>> 
>> >    char *str;
>
> If we care about this, shouldn't I rather move *str up below *errstr?

Yes.  The diff below shows the overall changes I had in mind.  Please
pick it up if you like it, or just use your version.

Sorry for the bikeshed color concern, I should have brought this up
later in private (if at all).


Index: main.c
===================================================================
RCS file: /d/cvs/src/usr.bin/ftp/main.c,v
retrieving revision 1.132
diff -u -p -p -u -r1.132 main.c
--- main.c      1 Sep 2020 12:33:48 -0000       1.132
+++ main.c      6 Sep 2020 08:26:25 -0000
@@ -209,6 +209,8 @@ char * const ssl_verify_opts[] = {
        "noverifytime",
 #define SSL_SESSION            8
        "session",
+#define SSL_PROTOCOLS          9
+       "protocols",
        NULL
 };
 
@@ -219,8 +221,9 @@ static void
 process_ssl_options(char *cp)
 {
        const char *errstr;
-       long long depth;
        char *str;
+       int depth;
+       uint32_t protocols;
 
        while (*cp) {
                switch (getsubopt(&cp, ssl_verify_opts, &str)) {
@@ -259,7 +262,7 @@ process_ssl_options(char *cp)
                        if (errstr)
                                errx(1, "certificate validation depth is %s",
                                    errstr);
-                       tls_config_set_verify_depth(tls_config, (int)depth);
+                       tls_config_set_verify_depth(tls_config, depth);
                        break;
                case SSL_MUSTSTAPLE:
                        tls_config_ocsp_require_stapling(tls_config);
@@ -278,6 +281,14 @@ process_ssl_options(char *cp)
                            tls_session_fd) == -1)
                                errx(1, "failed to set session: %s",
                                    tls_config_error(tls_config));
+                       break;
+               case SSL_PROTOCOLS:
+                       if (str == NULL)
+                               errx(1, "missing protocol name");
+                       if (tls_config_parse_protocols(&protocols, str) != 0)
+                               errx(1, "failed to parse TLS protocols");
+                       if (tls_config_set_protocols(tls_config, protocols) != 
0)
+                               errx(1, "failed to set TLS protocols");
                        break;
                default:
                        errx(1, "unknown -S suboption `%s'",

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to