Re: tls_load_file.3: tls_config_set_*_file() load files into memory
On Tue, Jun 22, 2021 at 01:29:59PM +, Klemens Nanni wrote: > On Tue, Jun 22, 2021 at 06:35:44AM +0100, Jason McIntyre wrote: > > > -sets the files from which the public certificate, and private key will > > > be read. > > > +loads two files from which the public certificate, and private key will > > > be read. > > > > this is a weird place for a comma. i would remove it. > > jmc > > Agreed, but all the other sentences below have it as well so I left it > to keep my diff simple. I'm sure this manual can be polished further > afterwards. > that is because they are lists of more than two items. putting a comma in a list with two items is wrong (well, that's a simplification, but it's wrong in your diff). jmc > > > .Pp > > > .Fn tls_config_set_keypair_mem > > > directly sets the public certificate, and private key from memory. > > > .Pp > > > .Fn tls_config_set_keypair_ocsp_file > > > -sets the files from which the public certificate, private key, and > > > DER-encoded > > > -OCSP staple will be read. > > > +loads three files containing the public certificate, private key, and > > > DER-encoded > > > +OCSP staple. > > > .Pp > > > .Fn tls_config_set_keypair_ocsp_mem > > > directly sets the public certificate, private key, and DER-encoded OCSP > > > staple > > > > > >
Re: tls_load_file.3: tls_config_set_*_file() load files into memory
On Tue, Jun 22, 2021 at 06:35:44AM +0100, Jason McIntyre wrote: > > -sets the files from which the public certificate, and private key will be > > read. > > +loads two files from which the public certificate, and private key will be > > read. > > this is a weird place for a comma. i would remove it. > jmc Agreed, but all the other sentences below have it as well so I left it to keep my diff simple. I'm sure this manual can be polished further afterwards. > > .Pp > > .Fn tls_config_set_keypair_mem > > directly sets the public certificate, and private key from memory. > > .Pp > > .Fn tls_config_set_keypair_ocsp_file > > -sets the files from which the public certificate, private key, and > > DER-encoded > > -OCSP staple will be read. > > +loads three files containing the public certificate, private key, and > > DER-encoded > > +OCSP staple. > > .Pp > > .Fn tls_config_set_keypair_ocsp_mem > > directly sets the public certificate, private key, and DER-encoded OCSP > > staple > > >
Re: tls_load_file.3: tls_config_set_*_file() load files into memory
On Tue, Jun 22, 2021 at 04:48:39AM +0200, Theo Buehler wrote: > > > > Feedback? OK? > > You have two overlong lines as indicated below. I would have thought > that mandoc -Tlint complains about that, but apparently it doesn't have > such a warning... With those wrapped, > yes, there is no feedback on long lines. although we try to keep the source less than 80 width, there are some places where it is not possible. i'm not sure whether adding a warning would be helpful or disruptive. jmc
Re: tls_load_file.3: tls_config_set_*_file() load files into memory
On Mon, Jun 21, 2021 at 11:26:41PM +, Klemens Nanni wrote: > > Thanks. tls_config_add_*_file also load files into memory, but given > this patch I think their usage of "add" in the manual is enough to infer > that files will also be loaded and added, so no need to change those as > well, I think. > > This should be the complete diff. > > Feedback? OK? > > > Index: man/tls_load_file.3 > === > RCS file: /cvs/src/lib/libtls/man/tls_load_file.3,v > retrieving revision 1.11 > diff -u -p -r1.11 tls_load_file.3 > --- man/tls_load_file.3 29 Nov 2018 14:24:23 - 1.11 > +++ man/tls_load_file.3 21 Jun 2021 23:24:58 - > @@ -217,8 +217,7 @@ call, ensuring that the memory contents > returns the path of the file that contains the default root certificates. > .Pp > .Fn tls_config_set_ca_file > -sets the filename used to load a file > -containing the root certificates. > +loads a file containing the root certificates. > .Pp > .Fn tls_config_set_ca_path > sets the path (directory) which should be searched for root > @@ -228,41 +227,39 @@ certificates. > sets the root certificates directly from memory. > .Pp > .Fn tls_config_set_cert_file > -sets file from which the public certificate will be read. > +loads a file containing the public certificate. > .Pp > .Fn tls_config_set_cert_mem > sets the public certificate directly from memory. > .Pp > .Fn tls_config_set_crl_file > -sets the filename used to load a file containing the > -Certificate Revocation List (CRL). > +loads a file containing the Certificate Revocation List (CRL). > .Pp > .Fn tls_config_set_crl_mem > sets the CRL directly from memory. > .Pp > .Fn tls_config_set_key_file > -sets the file from which the private key will be read. > +loads a file containing the private key. > .Pp > .Fn tls_config_set_key_mem > directly sets the private key from memory. > .Pp > .Fn tls_config_set_ocsp_staple_file > -sets a DER-encoded OCSP response to be stapled during the TLS handshake from > -the specified file. > +loads a file containing a DER-encoded OCSP response to be stapled during the > TLS handshake. > .Pp > .Fn tls_config_set_ocsp_staple_mem > sets a DER-encoded OCSP response to be stapled during the TLS handshake from > memory. > .Pp > .Fn tls_config_set_keypair_file > -sets the files from which the public certificate, and private key will be > read. > +loads two files from which the public certificate, and private key will be > read. this is a weird place for a comma. i would remove it. jmc > .Pp > .Fn tls_config_set_keypair_mem > directly sets the public certificate, and private key from memory. > .Pp > .Fn tls_config_set_keypair_ocsp_file > -sets the files from which the public certificate, private key, and > DER-encoded > -OCSP staple will be read. > +loads three files containing the public certificate, private key, and > DER-encoded > +OCSP staple. > .Pp > .Fn tls_config_set_keypair_ocsp_mem > directly sets the public certificate, private key, and DER-encoded OCSP > staple >
Re: tls_load_file.3: tls_config_set_*_file() load files into memory
On Mon, Jun 21, 2021 at 11:26:41PM +, Klemens Nanni wrote: > On Sun, Jun 20, 2021 at 09:32:36PM +0200, Theo Buehler wrote: > > On Sat, Jun 19, 2021 at 03:34:39PM +, Klemens Nanni wrote: > > > On Thu, Jun 10, 2021 at 10:26:15PM +, Klemens Nanni wrote: > > > > tls_config_set_ca_file(3) and tls_config_set_cert_file(3) do not just > > > > set the file paths (like tls_config_set_ca_path(3) does), they do load > > > > the given file into memory directly using tls_config_load_file(). > > > > > > > > This distinction is important because it means a later tls_connect(3) > > > > will not do any file I/O (at least regarding those files), which is > > > > relevant when for example pleding without "[rwc]path" after loading > > > > files into memory and before doing tls_connect(3). > > > > > > > > The manual's current wording made me use the following due to above way > > > > of pleding a program: > > > > > > > > tls_load_file() > > > > tls_config_set_ca_mem() > > > > tls_unload_file() > > > > > > > > While in fact the following does the same (in my case): > > > > > > > > tls_config_set_ca_file() > > > > > > > > > > > > So clarify this in the manual. > > > > > > > > Feedback? Objections? OK? > > > > > > Ping. > > > > You're right. This was changed in tls_config.c r1.26 (Aug 2016) and the > > documentation wasn't updated. However, the diff is incomplete as this > > concerns all tls_config_set_*_file functions: > > > > tls_config_set_ca_file > > tls_config_set_cert_file > > tls_config_set_crl_file > > tls_config_set_key_file > > tls_config_set_keypair_file > > tls_config_set_keypair_ocsp_file > > tls_config_set_ocsp_staple_file > > Thanks. tls_config_add_*_file also load files into memory, but given > this patch I think their usage of "add" in the manual is enough to infer > that files will also be loaded and added, so no need to change those as > well, I think. Agreed. > This should be the complete diff. > > Feedback? OK? You have two overlong lines as indicated below. I would have thought that mandoc -Tlint complains about that, but apparently it doesn't have such a warning... With those wrapped, ok tb > > Index: man/tls_load_file.3 > === > RCS file: /cvs/src/lib/libtls/man/tls_load_file.3,v > retrieving revision 1.11 > diff -u -p -r1.11 tls_load_file.3 > --- man/tls_load_file.3 29 Nov 2018 14:24:23 - 1.11 > +++ man/tls_load_file.3 21 Jun 2021 23:24:58 - > @@ -217,8 +217,7 @@ call, ensuring that the memory contents > returns the path of the file that contains the default root certificates. > .Pp > .Fn tls_config_set_ca_file > -sets the filename used to load a file > -containing the root certificates. > +loads a file containing the root certificates. > .Pp > .Fn tls_config_set_ca_path > sets the path (directory) which should be searched for root > @@ -228,41 +227,39 @@ certificates. > sets the root certificates directly from memory. > .Pp > .Fn tls_config_set_cert_file > -sets file from which the public certificate will be read. > +loads a file containing the public certificate. > .Pp > .Fn tls_config_set_cert_mem > sets the public certificate directly from memory. > .Pp > .Fn tls_config_set_crl_file > -sets the filename used to load a file containing the > -Certificate Revocation List (CRL). > +loads a file containing the Certificate Revocation List (CRL). > .Pp > .Fn tls_config_set_crl_mem > sets the CRL directly from memory. > .Pp > .Fn tls_config_set_key_file > -sets the file from which the private key will be read. > +loads a file containing the private key. > .Pp > .Fn tls_config_set_key_mem > directly sets the private key from memory. > .Pp > .Fn tls_config_set_ocsp_staple_file > -sets a DER-encoded OCSP response to be stapled during the TLS handshake from > -the specified file. > +loads a file containing a DER-encoded OCSP response to be stapled during the > TLS handshake. line break before "during" > .Pp > .Fn tls_config_set_ocsp_staple_mem > sets a DER-encoded OCSP response to be stapled during the TLS handshake from > memory. > .Pp > .Fn tls_config_set_keypair_file > -sets the files from which the public certificate, and private key will be > read. > +loads two files from which the public certificate, and private key will be > read. > .Pp > .Fn tls_config_set_keypair_mem > directly sets the public certificate, and private key from memory. > .Pp > .Fn tls_config_set_keypair_ocsp_file > -sets the files from which the public certificate, private key, and > DER-encoded > -OCSP staple will be read. > +loads three files containing the public certificate, private key, and > DER-encoded line break: ... key, and DER-encoded OCSP staple. > +OCSP staple. > .Pp > .Fn tls_config_set_keypair_ocsp_mem > directly sets the public certificate, private key, and DER-encoded OCSP > staple
Re: tls_load_file.3: tls_config_set_*_file() load files into memory
On Sun, Jun 20, 2021 at 09:32:36PM +0200, Theo Buehler wrote: > On Sat, Jun 19, 2021 at 03:34:39PM +, Klemens Nanni wrote: > > On Thu, Jun 10, 2021 at 10:26:15PM +, Klemens Nanni wrote: > > > tls_config_set_ca_file(3) and tls_config_set_cert_file(3) do not just > > > set the file paths (like tls_config_set_ca_path(3) does), they do load > > > the given file into memory directly using tls_config_load_file(). > > > > > > This distinction is important because it means a later tls_connect(3) > > > will not do any file I/O (at least regarding those files), which is > > > relevant when for example pleding without "[rwc]path" after loading > > > files into memory and before doing tls_connect(3). > > > > > > The manual's current wording made me use the following due to above way > > > of pleding a program: > > > > > > tls_load_file() > > > tls_config_set_ca_mem() > > > tls_unload_file() > > > > > > While in fact the following does the same (in my case): > > > > > > tls_config_set_ca_file() > > > > > > > > > So clarify this in the manual. > > > > > > Feedback? Objections? OK? > > > > Ping. > > You're right. This was changed in tls_config.c r1.26 (Aug 2016) and the > documentation wasn't updated. However, the diff is incomplete as this > concerns all tls_config_set_*_file functions: > > tls_config_set_ca_file > tls_config_set_cert_file > tls_config_set_crl_file > tls_config_set_key_file > tls_config_set_keypair_file > tls_config_set_keypair_ocsp_file > tls_config_set_ocsp_staple_file Thanks. tls_config_add_*_file also load files into memory, but given this patch I think their usage of "add" in the manual is enough to infer that files will also be loaded and added, so no need to change those as well, I think. This should be the complete diff. Feedback? OK? Index: man/tls_load_file.3 === RCS file: /cvs/src/lib/libtls/man/tls_load_file.3,v retrieving revision 1.11 diff -u -p -r1.11 tls_load_file.3 --- man/tls_load_file.3 29 Nov 2018 14:24:23 - 1.11 +++ man/tls_load_file.3 21 Jun 2021 23:24:58 - @@ -217,8 +217,7 @@ call, ensuring that the memory contents returns the path of the file that contains the default root certificates. .Pp .Fn tls_config_set_ca_file -sets the filename used to load a file -containing the root certificates. +loads a file containing the root certificates. .Pp .Fn tls_config_set_ca_path sets the path (directory) which should be searched for root @@ -228,41 +227,39 @@ certificates. sets the root certificates directly from memory. .Pp .Fn tls_config_set_cert_file -sets file from which the public certificate will be read. +loads a file containing the public certificate. .Pp .Fn tls_config_set_cert_mem sets the public certificate directly from memory. .Pp .Fn tls_config_set_crl_file -sets the filename used to load a file containing the -Certificate Revocation List (CRL). +loads a file containing the Certificate Revocation List (CRL). .Pp .Fn tls_config_set_crl_mem sets the CRL directly from memory. .Pp .Fn tls_config_set_key_file -sets the file from which the private key will be read. +loads a file containing the private key. .Pp .Fn tls_config_set_key_mem directly sets the private key from memory. .Pp .Fn tls_config_set_ocsp_staple_file -sets a DER-encoded OCSP response to be stapled during the TLS handshake from -the specified file. +loads a file containing a DER-encoded OCSP response to be stapled during the TLS handshake. .Pp .Fn tls_config_set_ocsp_staple_mem sets a DER-encoded OCSP response to be stapled during the TLS handshake from memory. .Pp .Fn tls_config_set_keypair_file -sets the files from which the public certificate, and private key will be read. +loads two files from which the public certificate, and private key will be read. .Pp .Fn tls_config_set_keypair_mem directly sets the public certificate, and private key from memory. .Pp .Fn tls_config_set_keypair_ocsp_file -sets the files from which the public certificate, private key, and DER-encoded -OCSP staple will be read. +loads three files containing the public certificate, private key, and DER-encoded +OCSP staple. .Pp .Fn tls_config_set_keypair_ocsp_mem directly sets the public certificate, private key, and DER-encoded OCSP staple
Re: tls_load_file.3: tls_config_set_*_file() load files into memory
On Sat, Jun 19, 2021 at 03:34:39PM +, Klemens Nanni wrote: > On Thu, Jun 10, 2021 at 10:26:15PM +, Klemens Nanni wrote: > > tls_config_set_ca_file(3) and tls_config_set_cert_file(3) do not just > > set the file paths (like tls_config_set_ca_path(3) does), they do load > > the given file into memory directly using tls_config_load_file(). > > > > This distinction is important because it means a later tls_connect(3) > > will not do any file I/O (at least regarding those files), which is > > relevant when for example pleding without "[rwc]path" after loading > > files into memory and before doing tls_connect(3). > > > > The manual's current wording made me use the following due to above way > > of pleding a program: > > > > tls_load_file() > > tls_config_set_ca_mem() > > tls_unload_file() > > > > While in fact the following does the same (in my case): > > > > tls_config_set_ca_file() > > > > > > So clarify this in the manual. > > > > Feedback? Objections? OK? > > Ping. You're right. This was changed in tls_config.c r1.26 (Aug 2016) and the documentation wasn't updated. However, the diff is incomplete as this concerns all tls_config_set_*_file functions: tls_config_set_ca_file tls_config_set_cert_file tls_config_set_crl_file tls_config_set_key_file tls_config_set_keypair_file tls_config_set_keypair_ocsp_file tls_config_set_ocsp_staple_file > > > Index: man/tls_load_file.3 > === > RCS file: /cvs/src/lib/libtls/man/tls_load_file.3,v > retrieving revision 1.11 > diff -u -p -r1.11 tls_load_file.3 > --- man/tls_load_file.3 29 Nov 2018 14:24:23 - 1.11 > +++ man/tls_load_file.3 10 Jun 2021 22:05:00 - > @@ -217,8 +217,7 @@ call, ensuring that the memory contents > returns the path of the file that contains the default root certificates. > .Pp > .Fn tls_config_set_ca_file > -sets the filename used to load a file > -containing the root certificates. > +loads a file containing the root certificates. > .Pp > .Fn tls_config_set_ca_path > sets the path (directory) which should be searched for root > @@ -228,7 +227,7 @@ certificates. > sets the root certificates directly from memory. > .Pp > .Fn tls_config_set_cert_file > -sets file from which the public certificate will be read. > +loads a file containing the public certificate. > .Pp > .Fn tls_config_set_cert_mem > sets the public certificate directly from memory. >
Re: tls_load_file.3: tls_config_set_*_file() load files into memory
On Thu, Jun 10, 2021 at 10:26:15PM +, Klemens Nanni wrote: > tls_config_set_ca_file(3) and tls_config_set_cert_file(3) do not just > set the file paths (like tls_config_set_ca_path(3) does), they do load > the given file into memory directly using tls_config_load_file(). > > This distinction is important because it means a later tls_connect(3) > will not do any file I/O (at least regarding those files), which is > relevant when for example pleding without "[rwc]path" after loading > files into memory and before doing tls_connect(3). > > The manual's current wording made me use the following due to above way > of pleding a program: > > tls_load_file() > tls_config_set_ca_mem() > tls_unload_file() > > While in fact the following does the same (in my case): > > tls_config_set_ca_file() > > > So clarify this in the manual. > > Feedback? Objections? OK? Ping. Index: man/tls_load_file.3 === RCS file: /cvs/src/lib/libtls/man/tls_load_file.3,v retrieving revision 1.11 diff -u -p -r1.11 tls_load_file.3 --- man/tls_load_file.3 29 Nov 2018 14:24:23 - 1.11 +++ man/tls_load_file.3 10 Jun 2021 22:05:00 - @@ -217,8 +217,7 @@ call, ensuring that the memory contents returns the path of the file that contains the default root certificates. .Pp .Fn tls_config_set_ca_file -sets the filename used to load a file -containing the root certificates. +loads a file containing the root certificates. .Pp .Fn tls_config_set_ca_path sets the path (directory) which should be searched for root @@ -228,7 +227,7 @@ certificates. sets the root certificates directly from memory. .Pp .Fn tls_config_set_cert_file -sets file from which the public certificate will be read. +loads a file containing the public certificate. .Pp .Fn tls_config_set_cert_mem sets the public certificate directly from memory.
tls_load_file.3: tls_config_set_*_file() load files into memory
tls_config_set_ca_file(3) and tls_config_set_cert_file(3) do not just set the file paths (like tls_config_set_ca_path(3) does), they do load the given file into memory directly using tls_config_load_file(). This distinction is important because it means a later tls_connect(3) will not do any file I/O (at least regarding those files), which is relevant when for example pleding without "[rwc]path" after loading files into memory and before doing tls_connect(3). The manual's current wording made me use the following due to above way of pleding a program: tls_load_file() tls_config_set_ca_mem() tls_unload_file() While in fact the following does the same (in my case): tls_config_set_ca_file() So clarify this in the manual. Feedback? Objections? OK? Index: man/tls_load_file.3 === RCS file: /cvs/src/lib/libtls/man/tls_load_file.3,v retrieving revision 1.11 diff -u -p -r1.11 tls_load_file.3 --- man/tls_load_file.3 29 Nov 2018 14:24:23 - 1.11 +++ man/tls_load_file.3 10 Jun 2021 22:05:00 - @@ -217,8 +217,7 @@ call, ensuring that the memory contents returns the path of the file that contains the default root certificates. .Pp .Fn tls_config_set_ca_file -sets the filename used to load a file -containing the root certificates. +loads a file containing the root certificates. .Pp .Fn tls_config_set_ca_path sets the path (directory) which should be searched for root @@ -228,7 +227,7 @@ certificates. sets the root certificates directly from memory. .Pp .Fn tls_config_set_cert_file -sets file from which the public certificate will be read. +loads a file containing the public certificate. .Pp .Fn tls_config_set_cert_mem sets the public certificate directly from memory.
