Skip to content

Commit

Permalink
ROS3: (feature+fix) Temporary security credentials - comments 3
Browse files Browse the repository at this point in the history 
- Improved the implementation of adding the session/security
  to the property list. Now the pointer to the string pointer
  is stored in the list. Furthermore, also pointer to the string
  and string itself are managed by the HDF5 library.
- Moved the structure H5FD_ros3_fapl_ext_t to the header file
  h5tools_utils.c.
- Adapted the function h5tools_parse_ros3_fapl_tulpe() and
  h5tools_populate_ros3_fapl() to make use of the structure
  H5FD_ros3_fapl_ext_t. Adjusted the related tests and the
  tool h5dump, h5ls and h5stat accordingly.
- Implemented additional error checks to ensure that the
  variable authorization, buffer1 and signed_headers are properly
  allocated.
- Reduced the size of the tmpstr array by 1 in the function
  H5FD_s3comms_aws_canonical_request().
- Resolved various compiler warnings of h5tools_test_utils tests.
- Improved the inline doxygen documentation.
  • Loading branch information
Jan-Willem Blokland authored and Jan-Willem Blokland committed Jun 9, 2023
1 parent 1068204 commit f8bd2b0
Show file tree
Hide file tree
Showing 9 changed files with 244 additions and 255 deletions.
59 changes: 36 additions & 23 deletions src/H5FDros3.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,8 @@ static herr_t H5FD__ros3_validate_config(const H5FD_ros3_fapl_t *fa);

static herr_t H5FD__ros3_str_token_copy(const char *name, size_t size, void *_value);
static int H5FD__ros3_str_token_cmp(const void *_value1, const void *_value2, size_t size);
static herr_t H5FD__ros3_str_token_close(const char *name, size_t size, void *value);
static herr_t H5FD__ros3_str_token_delete(hid_t prop_id, const char *name, size_t size, void *value);
static herr_t H5FD__ros3_str_token_close(const char *name, size_t size, void *_value);
static herr_t H5FD__ros3_str_token_delete(hid_t prop_id, const char *name, size_t size, void *_value);

static const H5FD_class_t H5FD_ros3_g = {
H5FD_CLASS_VERSION, /* struct version */
Expand Down Expand Up @@ -626,8 +626,8 @@ H5FD__ros3_fapl_free(void *_fa)
herr_t
H5Pget_fapl_ros3_token(hid_t fapl_id, size_t size, char *token_dst /*out*/)
{
H5P_genplist_t *plist = NULL;
char token_src[H5FD_ROS3_MAX_SECRET_TOK_LEN] = {0};
H5P_genplist_t *plist = NULL;
char *token_src;
htri_t token_exists;
size_t tokenlen = 0;
herr_t ret_value = SUCCEED;
Expand All @@ -651,7 +651,7 @@ H5Pget_fapl_ros3_token(hid_t fapl_id, size_t size, char *token_dst /*out*/)
if ((token_exists = H5P_exist_plist(plist, ROS3_TOKEN_PROP_NAME)) < 0)
HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "failed to check if property token exists in plist")
if (token_exists) {
if (H5P_get(plist, ROS3_TOKEN_PROP_NAME, token_src) < 0)
if (H5P_get(plist, ROS3_TOKEN_PROP_NAME, &token_src) < 0)
HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "unable to get token value")
}

Expand Down Expand Up @@ -682,7 +682,7 @@ H5Pget_fapl_ros3_token(hid_t fapl_id, size_t size, char *token_dst /*out*/)
static herr_t
H5FD__ros3_str_token_copy(const char *name, size_t size, void *_value)
{
char **value = (char **)&_value;
char **value = (char **)_value;
herr_t ret_value = SUCCEED;

FUNC_ENTER_PACKAGE
Expand Down Expand Up @@ -716,20 +716,20 @@ H5FD__ros3_str_token_copy(const char *name, size_t size, void *_value)
static int
H5FD__ros3_str_token_cmp(const void *_value1, const void *_value2, size_t size)
{
char const *value1 = (char const *)_value1;
char const *value2 = (char const *)_value2;
int ret_value = SUCCEED;
char *const *value1 = (char *const *)_value1;
char *const *value2 = (char *const *)_value2;
int ret_value = SUCCEED;

FUNC_ENTER_PACKAGE_NOERR

if (value1) {
if (value2)
ret_value = HDstrcmp(value1, value2);
if (*value1) {
if (*value2)
ret_value = HDstrcmp(*value1, *value2);
else
ret_value = 1;
}
else {
if (value2)
if (*value2)
ret_value = -1;
else
ret_value = 0;
Expand All @@ -753,12 +753,16 @@ H5FD__ros3_str_token_cmp(const void *_value1, const void *_value2, size_t size)
*-------------------------------------------------------------------------
*/
static herr_t
H5FD__ros3_str_token_close(const char *name, size_t size, void *value)
H5FD__ros3_str_token_close(const char *name, size_t size, void *_value)
{
char **value = (char **)_value;
herr_t ret_value = SUCCEED;

FUNC_ENTER_PACKAGE_NOERR

if (*value)
HDfree(*value);

FUNC_LEAVE_NOAPI(ret_value)
} /* H5FD__ros3_str_token_close */

Expand All @@ -778,12 +782,16 @@ H5FD__ros3_str_token_close(const char *name, size_t size, void *value)
*-------------------------------------------------------------------------
*/
static herr_t
H5FD__ros3_str_token_delete(hid_t prop_id, const char *name, size_t size, void *value)
H5FD__ros3_str_token_delete(hid_t prop_id, const char *name, size_t size, void *_value)
{
char **value = (char **)_value;
herr_t ret_value = SUCCEED;

FUNC_ENTER_PACKAGE_NOERR

if (*value)
HDfree(*value);

FUNC_LEAVE_NOAPI(ret_value)
} /* H5FD__ros3_str_token_delete */

Expand All @@ -805,6 +813,7 @@ herr_t
H5Pset_fapl_ros3_token(hid_t fapl_id, const char *token)
{
H5P_genplist_t *plist = NULL;
char *token_src;
htri_t token_exists;
herr_t ret_value = SUCCEED;

Expand All @@ -829,13 +838,17 @@ H5Pset_fapl_ros3_token(hid_t fapl_id, const char *token)
HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "failed to check if property token exists in plist")

if (token_exists) {
if (H5P_set(plist, ROS3_TOKEN_PROP_NAME, (void *)token) < 0)
HGOTO_ERROR(H5E_PLIST, H5E_CANTSET, FAIL, "unable to set value")
if (H5P_get(plist, ROS3_TOKEN_PROP_NAME, &token_src) < 0)
HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "unable to get token value")

HDmemcpy(token_src, token, HDstrlen(token) + 1);
}
else {
if (H5P_insert(plist, ROS3_TOKEN_PROP_NAME, sizeof(char) * (H5FD_ROS3_MAX_SECRET_TOK_LEN + 1),
(void *)token, NULL, NULL, NULL, NULL, H5FD__ros3_str_token_delete,
H5FD__ros3_str_token_copy, H5FD__ros3_str_token_cmp, H5FD__ros3_str_token_close) < 0)
token_src = HDmalloc(sizeof(char) * (H5FD_ROS3_MAX_SECRET_TOK_LEN + 1));
HDmemcpy(token_src, token, HDstrlen(token) + 1);
if (H5P_insert(plist, ROS3_TOKEN_PROP_NAME, sizeof(char *), &token_src, NULL, NULL, NULL, NULL,
H5FD__ros3_str_token_delete, H5FD__ros3_str_token_copy, H5FD__ros3_str_token_cmp,
H5FD__ros3_str_token_close) < 0)
HGOTO_ERROR(H5E_PLIST, H5E_CANTREGISTER, FAIL, "unable to register property in plist")
}

Expand Down Expand Up @@ -939,8 +952,8 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr)
H5FD_ros3_fapl_t fa;
H5P_genplist_t *plist = NULL;
htri_t token_exists;
char token[H5FD_ROS3_MAX_SECRET_TOK_LEN] = {0};
H5FD_t *ret_value = NULL;
char *token;
H5FD_t *ret_value = NULL;

FUNC_ENTER_PACKAGE

Expand Down Expand Up @@ -973,7 +986,7 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr)
if ((token_exists = H5P_exist_plist(plist, ROS3_TOKEN_PROP_NAME)) < 0)
HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, NULL, "failed to check if property token exists in plist")
if (token_exists) {
if (H5P_get(plist, ROS3_TOKEN_PROP_NAME, token) < 0)
if (H5P_get(plist, ROS3_TOKEN_PROP_NAME, &token) < 0)
HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, NULL, "unable to get token value")
}

Expand Down
30 changes: 6 additions & 24 deletions src/H5FDros3.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,27 +135,6 @@ typedef struct H5FD_ros3_fapl_t {
char secret_key[H5FD_ROS3_MAX_SECRET_KEY_LEN + 1];
} H5FD_ros3_fapl_t;

/**
* \struct H5FD_ros3_fapl_ext_t
* \brief Extended configuration structure for holding the configuration data
* to the #H5FD_ROS3 driver via a File Access Property List.
*
* \details H5FD_ros_fapl_ext_t is a public structure that is used to pass
* configuration data to the #H5FD_ROS3 driver via a File Access
* Property List.
*
* \var H5FD_ros3_fapl_t H5FD_ros3_fapl_ext_t::fa
* Configuration structure for H5Pset_fapl_ros3() / H5Pget_fapl_ros3().
*
* \var char H5FD_ros3_fapl_ext_t::token[H5FD_ROS3_MAX_SECRET_TOK_LEN + 1]
* A string which specifies the session/security token.
*
*/
typedef struct H5FD_ros3_fapl_ext_t {
H5FD_ros3_fapl_t fa;
char token[H5FD_ROS3_MAX_SECRET_TOK_LEN + 1];
} H5FD_ros3_fapl_ext_t;

#ifdef __cplusplus
extern "C" {
#endif
Expand Down Expand Up @@ -213,10 +192,13 @@ H5_DLL herr_t H5Pget_fapl_ros3_token(hid_t fapl_id, size_t size, char *token);
* \param[in] token Session/security token.
* \returns \herr_t
*
* \details H5Pset_fapy_ros3_token() modifies the File Access Property List to use the
* #H5FD_ROS3 driver by adding or updating the session/security token.
* \details H5Pset_fapl_ros3_token() modifies an existing File Access Property List which
* is used by #H5FD_ROS3 driver by adding or updating the session/security token
* of the property list. Be aware, to set the token first you need to create
* a proper File Access Property List using H5Pset_fapl_ros() and use this list
* as input argument of the function H5Pset_fapl_ros3_token().
*
* The session token is only needed when you want to access a S3 bucket
* Note, the session token is only needed when you want to access a S3 bucket
* using temporary security credentials.
*
* \since 1.14.2
Expand Down
13 changes: 9 additions & 4 deletions src/H5FDs3comms.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1322,6 +1322,8 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest)
/* authenticate request
*/
authorization = (char *)H5MM_malloc(512 + H5FD_ROS3_MAX_SECRET_TOK_LEN + 1);
if (authorization == NULL)
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "cannot make space for authorization variable.");
/* 2048 := approximate max length...
* 67 <len("AWS4-HMAC-SHA256 Credential=///s3/aws4_request,"
* "SignedHeaders=,Signature=")>
Expand All @@ -1334,9 +1336,13 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest)
*/
char buffer2[256 + 1]; /* -> String To Sign -> Credential */
char iso8601now[ISO8601_SIZE];
buffer1 = (char *)H5MM_malloc(512 + H5FD_ROS3_MAX_SECRET_TOK_LEN +
1); /* -> Canonical Request -> Signature */
buffer1 = (char *)H5MM_malloc(512 + H5FD_ROS3_MAX_SECRET_TOK_LEN +
1); /* -> Canonical Request -> Signature */
if (buffer1 == NULL)
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "cannot make space for buffer1 variable.");
signed_headers = (char *)H5MM_malloc(48 + H5FD_ROS3_MAX_SECRET_KEY_LEN + 1);
if (signed_headers == NULL)
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "cannot make space for signed_headers variable.");
/* should be large enough for nominal listing:
* "host;range;x-amz-content-sha256;x-amz-date;x-amz-security-token"
* + '\0', with "range;" and/or "x-amz-security-token" possibly absent
Expand Down Expand Up @@ -1664,8 +1670,7 @@ H5FD_s3comms_aws_canonical_request(char *canonical_request_dest, int _cr_size, c
size_t sh_size = (size_t)_sh_size;
size_t cr_len = 0; /* working length of canonical request str */
size_t sh_len = 0; /* working length of signed headers str */
char tmpstr[1024 + 1];
tmpstr[1024] = 0; /* terminating NULL */
char tmpstr[1024];

/* "query params" refers to the optional element in the URL, e.g.
* http://bucket.aws.com/myfile.txt?max-keys=2&prefix=J
Expand Down
31 changes: 15 additions & 16 deletions tools/lib/h5tools_utils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1028,8 +1028,7 @@ h5tools_getenv_update_hyperslab_bufsize(void)
*----------------------------------------------------------------------------
*/
herr_t
h5tools_parse_ros3_fapl_tuple(const char *tuple_str, int delim, H5FD_ros3_fapl_t *fapl_config_out,
char *token_out)
h5tools_parse_ros3_fapl_tuple(const char *tuple_str, int delim, H5FD_ros3_fapl_ext_t *fapl_config_out)
{
const char *ccred[4];
unsigned nelems = 0;
Expand All @@ -1055,7 +1054,7 @@ h5tools_parse_ros3_fapl_tuple(const char *tuple_str, int delim, H5FD_ros3_fapl_t
ccred[3] = (const char *)s3cred[3];
}

if (0 == h5tools_populate_ros3_fapl(fapl_config_out, token_out, ccred))
if (0 == h5tools_populate_ros3_fapl(fapl_config_out, ccred))
H5TOOLS_GOTO_ERROR(FAIL, "failed to populate S3 VFD FAPL config");

done:
Expand Down Expand Up @@ -1131,7 +1130,7 @@ h5tools_parse_ros3_fapl_tuple(const char *tuple_str, int delim, H5FD_ros3_fapl_t
*----------------------------------------------------------------------------
*/
int
h5tools_populate_ros3_fapl(H5FD_ros3_fapl_t *fa, char *token, const char **values)
h5tools_populate_ros3_fapl(H5FD_ros3_fapl_ext_t *fa, const char **values)
{
int show_progress = 0; /* set to 1 for debugging */
int ret_value = 1; /* 1 for success, 0 for failure */
Expand All @@ -1149,7 +1148,7 @@ h5tools_populate_ros3_fapl(H5FD_ros3_fapl_t *fa, char *token, const char **value
goto done;
}

if (token == NULL) {
if (fa->token == NULL) {
if (show_progress) {
HDprintf(" ERROR: null pointer to token\n");
}
Expand All @@ -1160,12 +1159,12 @@ h5tools_populate_ros3_fapl(H5FD_ros3_fapl_t *fa, char *token, const char **value
if (show_progress) {
HDprintf(" preset fapl with default values\n");
}
fa->version = H5FD_CURR_ROS3_FAPL_T_VERSION;
fa->authenticate = FALSE;
*(fa->aws_region) = '\0';
*(fa->secret_id) = '\0';
*(fa->secret_key) = '\0';
*token = '\0';
fa->fa.version = H5FD_CURR_ROS3_FAPL_T_VERSION;
fa->fa.authenticate = FALSE;
*(fa->fa.aws_region) = '\0';
*(fa->fa.secret_id) = '\0';
*(fa->fa.secret_key) = '\0';
*(fa->token) = '\0';

/* sanity-check supplied values
*/
Expand Down Expand Up @@ -1210,7 +1209,7 @@ h5tools_populate_ros3_fapl(H5FD_ros3_fapl_t *fa, char *token, const char **value
ret_value = 0;
goto done;
}
HDmemcpy(fa->aws_region, values[0], (HDstrlen(values[0]) + 1));
HDmemcpy(fa->fa.aws_region, values[0], (HDstrlen(values[0]) + 1));
if (show_progress) {
HDprintf(" aws_region set\n");
}
Expand All @@ -1222,7 +1221,7 @@ h5tools_populate_ros3_fapl(H5FD_ros3_fapl_t *fa, char *token, const char **value
ret_value = 0;
goto done;
}
HDmemcpy(fa->secret_id, values[1], (HDstrlen(values[1]) + 1));
HDmemcpy(fa->fa.secret_id, values[1], (HDstrlen(values[1]) + 1));
if (show_progress) {
HDprintf(" secret_id set\n");
}
Expand All @@ -1234,7 +1233,7 @@ h5tools_populate_ros3_fapl(H5FD_ros3_fapl_t *fa, char *token, const char **value
ret_value = 0;
goto done;
}
HDmemcpy(fa->secret_key, values[2], (HDstrlen(values[2]) + 1));
HDmemcpy(fa->fa.secret_key, values[2], (HDstrlen(values[2]) + 1));
if (show_progress) {
HDprintf(" secret_key set\n");
}
Expand All @@ -1246,12 +1245,12 @@ h5tools_populate_ros3_fapl(H5FD_ros3_fapl_t *fa, char *token, const char **value
ret_value = 0;
goto done;
}
HDmemcpy(token, values[3], (HDstrlen(values[3]) + 1));
HDmemcpy(fa->token, values[3], (HDstrlen(values[3]) + 1));
if (show_progress) {
HDprintf(" token set\n");
}

fa->authenticate = TRUE;
fa->fa.authenticate = TRUE;
if (show_progress) {
HDprintf(" set to authenticate\n");
}
Expand Down
12 changes: 10 additions & 2 deletions tools/lib/h5tools_utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ typedef struct find_objs_t {
table_t *dset_table;
} find_objs_t;

#ifdef H5_HAVE_ROS3_VFD
/*extended configuration struct for holding the configuration data to the #H5FD_ROS3 driver */
typedef struct H5FD_ros3_fapl_ext_t {
H5FD_ros3_fapl_t fa; /* ROS3 configuration struct*/
char token[H5FD_ROS3_MAX_SECRET_TOK_LEN + 1]; /* Session/security token*/
} H5FD_ros3_fapl_ext_t;
#endif /* H5_HAVE_ROS3_VFD */

H5TOOLS_DLLVAR unsigned h5tools_nCols; /*max number of columns for outputting */

/* Definitions of useful routines */
Expand Down Expand Up @@ -129,8 +137,8 @@ H5TOOLS_DLL void h5tools_setstatus(int d_status);
H5TOOLS_DLL int h5tools_getenv_update_hyperslab_bufsize(void);
#ifdef H5_HAVE_ROS3_VFD
H5TOOLS_DLL herr_t h5tools_parse_ros3_fapl_tuple(const char *tuple_str, int delim,
H5FD_ros3_fapl_t *fapl_config_out, char *token);
H5TOOLS_DLL int h5tools_populate_ros3_fapl(H5FD_ros3_fapl_t *fa, char *token, const char **values);
H5FD_ros3_fapl_ext_t *fapl_config_out);
H5TOOLS_DLL int h5tools_populate_ros3_fapl(H5FD_ros3_fapl_ext_t *fa, const char **values);
#endif /* H5_HAVE_ROS3_VFD */
#ifdef H5_HAVE_LIBHDFS
H5TOOLS_DLL herr_t h5tools_parse_hdfs_fapl_tuple(const char *tuple_str, int delim,
Expand Down
Loading

0 comments on commit f8bd2b0

Please sign in to comment.