Hi,
On 02/08/18 23:46, Steffan Karger wrote:
> Rewrite buf_write_string_file to buffer_write_file, which is simpler to
> use and can deal with not-null-terminated strings. Mostly implemented so
> this can be easily reused for tls-crypt-v2 (client) key files.
>
> Signed-off-by: Steffan Karger <steffan.kar...@fox-it.com>
> ---
> v3: split change out of "generate client key", reuse in write_key_file()
> v4: improve doxygen and error handling
>
> src/openvpn/buffer.c | 31 ++++++++++++++++++++++++-------
> src/openvpn/buffer.h | 12 ++++++++----
> src/openvpn/crypto.c | 33 ++++++---------------------------
> src/openvpn/crypto.h | 5 +++++
> src/openvpn/init.c | 4 ++++
> 5 files changed, 47 insertions(+), 38 deletions(-)
>
> diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
> index 0972139..7f540d7 100644
> --- a/src/openvpn/buffer.c
> +++ b/src/openvpn/buffer.c
> @@ -343,16 +343,33 @@ convert_to_one_line(struct buffer *buf)
> }
> }
>
> -/* NOTE: requires that string be null terminated */
> -void
> -buf_write_string_file(const struct buffer *buf, const char *filename, int fd)
> +bool
> +buffer_write_file(const char *filename, const struct buffer *buf)
> {
> - const int len = strlen((char *) BPTR(buf));
> - const int size = write(fd, BPTR(buf), len);
> - if (size != len)
> + bool ret = false;
> + int fd = platform_open(filename, O_CREAT | O_TRUNC | O_WRONLY,
> + S_IRUSR | S_IWUSR);
> + if (fd == -1)
> + {
> + msg(M_ERRNO, "Cannot open file '%s' for write", filename);
> + goto cleanup;While running some tests I realized that a failure in open() will cause a double error because close() is performed with fd = -1. The result is: $ ./src/openvpn/openvpn --genkey --secret /root/test.key Fri Aug 3 17:35:46 2018 Cannot open file '/root/test.key' for write: Permission denied (errno=13) Fri Aug 3 17:35:46 2018 Close error on file /root/test.key: Bad file descriptor (errno=9) Fri Aug 3 17:35:46 2018 Failed to write key file Fri Aug 3 17:35:46 2018 Exiting due to fatal error As you can see we have two distinct errors, with the second being caused by the first one. I think this is one of those cases where in case of failure you just return false rather than jumping to cleanup, because nothing has been initialized yet. The double error is harmless, but not very clean, especially because, no matter why open() failed, errno will always be changed to EBADF. (I think this also means that you can get rid of the label at all and slightly restyle the code) Cheers, -- Antonio Quartulli
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
