Skip to content
Permalink
Browse files
version: add 'cainfo' and 'capath' to version info struct
Suggested-by: Timothe Litt
URL: https://curl.haxx.se/mail/lib-2020-03/0090.html
Reviewed-by: Jay Satiro

Closes #5150
  • Loading branch information
bagder committed Mar 27, 2020
1 parent 62112d2 commit 6de756c9b1de34b7a1b1d6cd978ca75f3b1d719b
Showing 4 changed files with 36 additions and 12 deletions.
@@ -5,7 +5,7 @@
.\" * | (__| |_| | _ <| |___
.\" * \___|\___/|_| \_\_____|
.\" *
.\" * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" * Copyright (C) 1998 - 2020, Daniel Stenberg, <daniel@haxx.se>, et al.
.\" *
.\" * This software is licensed as described in the file COPYING, which
.\" * you should have received as part of this distribution. The terms
@@ -49,7 +49,6 @@ The curl_version_info_data struct looks like this
typedef struct {
CURLversion age; /* see description below */

/* when 'age' is 0 or higher, the members below also exist: */
const char *version; /* human readable string */
unsigned int version_num; /* numeric representation */
const char *host; /* human readable string */
@@ -59,34 +58,40 @@ typedef struct {
const char *libz_version; /* human readable string */
const char * const *protocols; /* protocols */

/* when 'age' is 1 or higher, the members below also exist: */
/* when 'age' is CURLVERSION_SECOND or higher, the members below exist */
const char *ares; /* human readable string */
int ares_num; /* number */

/* when 'age' is 2 or higher, the member below also exists: */
/* when 'age' is CURLVERSION_THIRD or higher, the members below exist */
const char *libidn; /* human readable string */

/* when 'age' is 3 or higher (7.16.1 or later), the members below also
exist */
/* when 'age' is CURLVERSION_FOURTH or higher (>= 7.16.1), the members
below exist */
int iconv_ver_num; /* '_libiconv_version' if iconv support enabled */

const char *libssh_version; /* human readable string */

/* when 'age' is 4 or higher (7.57.0 or later), the members below also
exist */
/* when 'age' is CURLVERSION_FIFTH or higher (>= 7.57.0), the members
below exist */
unsigned int brotli_ver_num; /* Numeric Brotli version
(MAJOR << 24) | (MINOR << 12) | PATCH */
const char *brotli_version; /* human readable string. */

/* when 'age is CURLVERSION_SIXTH or alter (7.66.0 or later), these fields
also exist */
/* when 'age' is CURLVERSION_SIXTH or higher (>= 7.66.0), the members
below exist */
unsigned int nghttp2_ver_num; /* Numeric nghttp2 version
(MAJOR << 16) | (MINOR << 8) | PATCH */
const char *nghttp2_version; /* human readable string. */

const char *quic_version; /* human readable quic (+ HTTP/3) library +
version or NULL */

/* when 'age' is CURLVERSION_SEVENTH or higher (>= 7.70.0), the members
below exist */
const char *cainfo; /* the built-in default CURLOPT_CAINFO, might
be NULL */
const char *capath; /* the built-in default CURLOPT_CAPATH, might
be NULL */
} curl_version_info_data;
.fi

@@ -798,6 +798,7 @@ CURLVERSION_FIRST 7.10
CURLVERSION_FOURTH 7.16.1
CURLVERSION_NOW 7.10
CURLVERSION_SECOND 7.11.1
CURLVERSION_SEVENTH 7.70.0
CURLVERSION_SIXTH 7.66.0
CURLVERSION_THIRD 7.12.0
CURL_CHUNK_BGN_FUNC_FAIL 7.21.0
@@ -2729,6 +2729,7 @@ typedef enum {
CURLVERSION_FOURTH,
CURLVERSION_FIFTH,
CURLVERSION_SIXTH,
CURLVERSION_SEVENTH,
CURLVERSION_LAST /* never actually use this */
} CURLversion;

@@ -2737,7 +2738,7 @@ typedef enum {
meant to be a built-in version number for what kind of struct the caller
expects. If the struct ever changes, we redefine the NOW to another enum
from above. */
#define CURLVERSION_NOW CURLVERSION_SIXTH
#define CURLVERSION_NOW CURLVERSION_SEVENTH

typedef struct {
CURLversion age; /* age of the returned struct */
@@ -2776,6 +2777,13 @@ typedef struct {
const char *nghttp2_version; /* human readable string. */
const char *quic_version; /* human readable quic (+ HTTP/3) library +
version or NULL */

/* These fields were added in CURLVERSION_SEVENTH */
const char *cainfo; /* the built-in default CURLOPT_CAINFO, might
be NULL */
const char *capath; /* the built-in default CURLOPT_CAPATH, might
be NULL */

} curl_version_info_data;

#define CURL_VERSION_IPV6 (1<<0) /* IPv6-enabled */
@@ -404,7 +404,17 @@ static curl_version_info_data version_info = {
NULL, /* brotli version */
0, /* nghttp2 version number */
NULL, /* nghttp2 version string */
NULL /* quic library string */
NULL, /* quic library string */
#ifdef CURL_CA_BUNDLE
CURL_CA_BUNDLE, /* cainfo */

This comment has been minimized.

Copy link
@gvanem

gvanem Mar 29, 2020

Contributor

With this in my curl_config.h:

  #define CURL_CA_BUNDLE getenv("CURL_CA_BUNDLE")

the above does no longer compile:

version.c(409,3): error: initializer element is not a compile-time constant
  CURL_CA_BUNDLE, /* cainfo */
  ^~~~~~~~~~~~~~
./curl_config.h(6,24): note: expanded from macro 'CURL_CA_BUNDLE'
#define CURL_CA_BUNDLE getenv("CURL_CA_BUNDLE")
                       ^~~~~~~~~~~~~~~~~~~~~~~~

I'd had this hack for years.

This comment has been minimized.

Copy link
@bagder

bagder Mar 29, 2020

Author Member

That's unfortunate, but what do you suggest we do? Rework this code so that your "hack" still works?

This comment has been minimized.

Copy link
@gvanem

gvanem Mar 29, 2020

Contributor

If it can't be a compile-time constant, then fill it at run-time:

--- a/lib/version.c 2020-03-29 17:38:06
+++ b/lib/version.c 2020-03-29 21:10:00
@@ -405,11 +405,7 @@
   0,    /* nghttp2 version number */
   NULL, /* nghttp2 version string */
   NULL, /* quic library string */
-#ifdef CURL_CA_BUNDLE
-  CURL_CA_BUNDLE, /* cainfo */
-#else
-  NULL,
-#endif
+  NULL, /* cainfo */
 #ifdef CURL_CA_PATH
   CURL_CA_PATH  /* capath */
 #else
@@ -433,6 +429,11 @@
   static char brotli_buffer[80];
 #endif

+#ifdef CURL_CA_BUNDLE
+  if (!version_info.cainfo)
+     version_info.cainfo = CURL_CA_BUNDLE; /* since it could not be a compile-time contant */
+#endif
+
 #ifdef USE_SSL
   Curl_ssl_version(ssl_buffer, sizeof(ssl_buffer));
   version_info.ssl_version = ssl_buffer;

This comment has been minimized.

Copy link
@bagder

bagder Mar 29, 2020

Author Member

Sure, I understand that''s a way to make your hack remain functional. But is it really our job to maintain a single person's "hack" at the expense of adding (a tiny amount of) code to curl? CURL_CA_BUNDLE is made to be a constant string in the code after all. I suppose I'm asking why you have to insist to have this patch in libcurl when it is easy to have outside of it as well?

This comment has been minimized.

Copy link
@jay

jay Mar 29, 2020

Member

We do it elsewhere, so it is an issue.

curl/lib/Makefile.netware

Lines 670 to 674 in b81e0b0

ifdef CABUNDLE
@echo $(DL)#define CURL_CA_BUNDLE "$(CABUNDLE)"$(DL) >> $@
else
@echo $(DL)#define CURL_CA_BUNDLE getenv("CURL_CA_BUNDLE")$(DL) >> $@
endif

#define CURL_CA_BUNDLE getenv("CURL_CA_BUNDLE")

There would have to be a copy because the environment block may change. For example:

diff --git a/lib/version.c b/lib/version.c
index ed5592a..a459285 100644
--- a/lib/version.c
+++ b/lib/version.c
@@ -28,6 +28,7 @@
 #include "http2.h"
 #include "vssh/ssh.h"
 #include "quic.h"
+#include "curl_path.h"
 #include "curl_printf.h"
 
 #ifdef USE_ARES
@@ -405,11 +406,7 @@ static curl_version_info_data version_info = {
   0,    /* nghttp2 version number */
   NULL, /* nghttp2 version string */
   NULL, /* quic library string */
-#ifdef CURL_CA_BUNDLE
-  CURL_CA_BUNDLE, /* cainfo */
-#else
-  NULL,
-#endif
+  NULL, /* cainfo */
 #ifdef CURL_CA_PATH
   CURL_CA_PATH  /* capath */
 #else
@@ -499,6 +496,16 @@ curl_version_info_data *curl_version_info(CURLversion stamp)
   }
 #endif
 
+#ifdef CURL_CA_BUNDLE
+  /* CURL_CA_BUNDLE may be getenv("CURL_CA_BUNDLE") so it must be copied */
+  {
+    static char cainfo_buffer[PATH_MAX];
+    strncpy(cainfo_buffer, CURL_CA_BUNDLE, sizeof(cainfo_buffer) - 1);
+    cainfo_buffer[sizeof(cainfo_buffer) - 1] = '\0';
+    version_info.cainfo = cainfo_buffer;
+  }
+#endif
+
   (void)stamp; /* avoid compiler warnings, we don't use this */
   return &version_info;
 }

This comment has been minimized.

Copy link
@jay

jay Mar 29, 2020

Member

The example isn't thread safe. On second thought maybe we should just not do that anymore.

This comment has been minimized.

Copy link
@bagder

bagder Mar 29, 2020

Author Member

We do it elsewhere, so it is an issue

But I think my argument goes for those places too: why do we do that? Should we really keep doing that? (We should make this a real issue and not just discuss on a commit...)

This comment has been minimized.

Copy link
@jay

jay Mar 29, 2020

Member
#else
NULL,
#endif
#ifdef CURL_CA_PATH
CURL_CA_PATH /* capath */
#else
NULL
#endif
};

curl_version_info_data *curl_version_info(CURLversion stamp)

0 comments on commit 6de756c

Please sign in to comment.