Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More H5FDs3comms.c/h cleanup #5231

Merged
merged 3 commits into from
Jan 14, 2025
Merged

More H5FDs3comms.c/h cleanup #5231

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fac8e96
More H5FDs3comms.c/h cleanup
Jan 14, 2025
6d0bd40
Minor tweak to test output
Jan 14, 2025
6bdfdcb
Another small test output change
Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/H5FDros3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ H5Pset_fapl_ros3(hid_t fapl_id, const H5FD_ros3_fapl_t *fa)
if (plist == NULL)
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a file access property list");

if (FAIL == H5FD__ros3_validate_config(fa))
if (H5FD__ros3_validate_config(fa) < 0)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid ros3 config");

ret_value = H5P_set_driver(plist, H5FD_ROS3, (const void *)fa, NULL);
Expand Down Expand Up @@ -754,8 +754,8 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr)
assert(now != NULL);
if (ISO8601NOW(iso8601now, now) != (ISO8601_SIZE - 1))
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "problem while writing iso8601 timestamp");
if (FAIL == H5FD_s3comms_signing_key(signing_key, (const char *)fa->secret_key,
(const char *)fa->aws_region, (const char *)iso8601now))
if (H5FD_s3comms_make_aws_signing_key(signing_key, (const char *)fa->secret_key,
(const char *)fa->aws_region, (const char *)iso8601now) < 0)
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, NULL, "problem while computing signing key");

if (token_exists)
Expand Down Expand Up @@ -783,7 +783,7 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr)
H5MM_memcpy(&(file->fa), fa, sizeof(H5FD_ros3_fapl_t));

#ifdef ROS3_STATS
if (FAIL == H5FD__ros3_reset_stats(file))
if (H5FD__ros3_reset_stats(file) < 0)
HGOTO_ERROR(H5E_VFL, H5E_UNINITIALIZED, NULL, "unable to reset file statistics");
#endif

Expand All @@ -804,7 +804,7 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr)
done:
if (ret_value == NULL) {
if (handle != NULL)
if (FAIL == H5FD_s3comms_s3r_close(handle))
if (H5FD_s3comms_s3r_close(handle) < 0)
HDONE_ERROR(H5E_VFL, H5E_CANTCLOSEFILE, NULL, "unable to close s3 file handle");
if (file != NULL) {
H5MM_xfree(file->cache);
Expand Down Expand Up @@ -836,12 +836,12 @@ H5FD__ros3_close(H5FD_t H5_ATTR_UNUSED *_file)
assert(file->s3r_handle != NULL);

#ifdef ROS3_STATS
if (H5FD__ros3_print_stats(stdout, file) == FAIL)
if (H5FD__ros3_print_stats(stdout, file) < 0)
HGOTO_ERROR(H5E_INTERNAL, H5E_ERROR, FAIL, "problem while writing file statistics");
#endif

/* Close the underlying request handle */
if (FAIL == H5FD_s3comms_s3r_close(file->s3r_handle))
if (H5FD_s3comms_s3r_close(file->s3r_handle) < 0)
HGOTO_ERROR(H5E_VFL, H5E_CANTCLOSEFILE, FAIL, "unable to close S3 request handle");

/* Release the file info */
Expand Down Expand Up @@ -1119,7 +1119,7 @@ H5FD__ros3_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU
memcpy(buf, file->cache + addr, size);
}
else {
if (H5FD_s3comms_s3r_read(file->s3r_handle, addr, size, buf) == FAIL)
if (H5FD_s3comms_s3r_read(file->s3r_handle, addr, size, buf) < 0)
HGOTO_ERROR(H5E_VFL, H5E_READERROR, FAIL, "unable to execute read");

#ifdef ROS3_STATS
Expand Down
43 changes: 24 additions & 19 deletions src/H5FDs3comms.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
/* Headers */
/***********/

/* There's no H5FDs3comms_test.c file, so the test functions are located here */
#define H5FD_S3COMMS_TESTING

#include "H5private.h" /* generic functions */
#include "H5Eprivate.h" /* error handling */
#include "H5MMprivate.h" /* memory management */
Expand Down Expand Up @@ -432,7 +435,7 @@ H5FD_s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value)
} /* end while is_looking */

done:
if (ret_value == FAIL) {
if (ret_value < 0) {
H5MM_xfree(nvcat);
H5MM_xfree(namecpy);
H5MM_xfree(lowername);
Expand Down Expand Up @@ -1168,19 +1171,20 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest)
/**** COMPUTE AUTHORIZATION ****/

/* buffer1 -> canonical request */
if (H5FD_s3comms_aws_canonical_request(buffer1, 512 + H5FD_ROS3_MAX_SECRET_TOK_LEN, signed_headers,
48 + H5FD_ROS3_MAX_SECRET_TOK_LEN, request) < 0) {
if (H5FD_s3comms_make_aws_canonical_request(buffer1, 512 + H5FD_ROS3_MAX_SECRET_TOK_LEN,
signed_headers, 48 + H5FD_ROS3_MAX_SECRET_TOK_LEN,
request) < 0) {
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "bad canonical request");
}
/* buffer2->string-to-sign */
if (H5FD_s3comms_tostringtosign(buffer2, buffer1, iso8601now, handle->region) < 0)
if (H5FD_s3comms_make_aws_stringtosign(buffer2, buffer1, iso8601now, handle->region) < 0)
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "bad string-to-sign");

/* buffer1 -> signature */
HMAC(EVP_sha256(), handle->signing_key, SHA256_DIGEST_LENGTH, (const unsigned char *)buffer2,
strlen(buffer2), md, &md_len);
if (H5FD__s3comms_bytes_to_hex(buffer1, 512 + H5FD_ROS3_MAX_SECRET_TOK_LEN + 1,
(const unsigned char *)md, (size_t)md_len) == FAIL)
(const unsigned char *)md, (size_t)md_len) < 0)
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "could not convert to hex string.");

/* Trim to yyyyMMDD */
Expand All @@ -1197,7 +1201,7 @@ H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest)
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to format aws4 authorization string");

/* Append authorization header to http request buffer */
if (H5FD_s3comms_hrb_node_set(&headers, "Authorization", (const char *)authorization) == FAIL)
if (H5FD_s3comms_hrb_node_set(&headers, "Authorization", (const char *)authorization) < 0)
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to set Authorization header");
if (headers == NULL)
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "problem building headers list");
Expand Down Expand Up @@ -1322,7 +1326,7 @@ gmnow(void)

/*----------------------------------------------------------------------------
*
* Function: H5FD_s3comms_aws_canonical_request()
* Function: H5FD_s3comms_make_aws_canonical_request()
*
* Purpose:
*
Expand Down Expand Up @@ -1357,8 +1361,8 @@ gmnow(void)
*----------------------------------------------------------------------------
*/
herr_t
H5FD_s3comms_aws_canonical_request(char *canonical_request_dest, int _cr_size, char *signed_headers_dest,
int _sh_size, hrb_t *http_request)
H5FD_s3comms_make_aws_canonical_request(char *canonical_request_dest, int _cr_size, char *signed_headers_dest,
int _sh_size, hrb_t *http_request)
{
hrb_node_t *node = NULL;
const char *query_params = ""; /* unused at present */
Expand Down Expand Up @@ -1445,7 +1449,7 @@ H5FD_s3comms_aws_canonical_request(char *canonical_request_dest, int _cr_size, c
free(tmpstr);

FUNC_LEAVE_NOAPI(ret_value)
} /* end H5FD_s3comms_aws_canonical_request() */
} /* end H5FD_s3comms_make_aws_canonical_request() */

/*----------------------------------------------------------------------------
* Function: H5FD__s3comms_bytes_to_hex()
Expand Down Expand Up @@ -1703,7 +1707,7 @@ H5FD_s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char *
credfile = fopen(filepath, "r");
if (credfile != NULL) {
if (H5FD__s3comms_load_aws_creds_from_file(credfile, profile_name, key_id_out, secret_access_key_out,
aws_region_out) == FAIL)
aws_region_out) < 0)
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to load from aws credentials");
if (fclose(credfile) == EOF)
HGOTO_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, FAIL, "unable to close credentials file");
Expand All @@ -1719,7 +1723,7 @@ H5FD_s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char *
if (H5FD__s3comms_load_aws_creds_from_file(
credfile, profile_name, (*key_id_out == 0) ? key_id_out : NULL,
(*secret_access_key_out == 0) ? secret_access_key_out : NULL,
(*aws_region_out == 0) ? aws_region_out : NULL) == FAIL)
(*aws_region_out == 0) ? aws_region_out : NULL) < 0)
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "unable to load from aws config");
if (fclose(credfile) == EOF)
HGOTO_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, FAIL, "unable to close config file");
Expand All @@ -1740,7 +1744,7 @@ H5FD_s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char *

/*----------------------------------------------------------------------------
*
* Function: H5FD_s3comms_signing_key()
* Function: H5FD_s3comms_make_aws_signing_key()
*
* Purpose:
*
Expand Down Expand Up @@ -1774,7 +1778,8 @@ H5FD_s3comms_load_aws_profile(const char *profile_name, char *key_id_out, char *
*----------------------------------------------------------------------------
*/
herr_t
H5FD_s3comms_signing_key(unsigned char *md, const char *secret, const char *region, const char *iso8601now)
H5FD_s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const char *region,
const char *iso8601now)
{
char *AWS4_secret = NULL;
size_t AWS4_secret_len = 0;
Expand Down Expand Up @@ -1825,11 +1830,11 @@ H5FD_s3comms_signing_key(unsigned char *md, const char *secret, const char *regi
H5MM_xfree(AWS4_secret);

FUNC_LEAVE_NOAPI(ret_value)
} /* end H5FD_s3comms_signing_key() */
} /* end H5FD_s3comms_make_aws_signing_key() */

/*----------------------------------------------------------------------------
*
* Function: H5FD_s3comms_tostringtosign()
* Function: H5FD_s3comms_make_aws_stringtosign()
*
* Purpose:
*
Expand Down Expand Up @@ -1861,7 +1866,7 @@ H5FD_s3comms_signing_key(unsigned char *md, const char *secret, const char *regi
*----------------------------------------------------------------------------
*/
herr_t
H5FD_s3comms_tostringtosign(char *dest, const char *req, const char *now, const char *region)
H5FD_s3comms_make_aws_stringtosign(char *dest, const char *req, const char *now, const char *region)
{
unsigned char checksum[S3COMMS_SHA256_HEXSTR_LENGTH];
size_t d = 0;
Expand Down Expand Up @@ -1909,7 +1914,7 @@ H5FD_s3comms_tostringtosign(char *dest, const char *req, const char *now, const
SHA256((const unsigned char *)req, strlen(req), checksum);

if (H5FD__s3comms_bytes_to_hex(hexsum, S3COMMS_SHA256_HEXSTR_LENGTH, (const unsigned char *)checksum,
SHA256_DIGEST_LENGTH) == FAIL)
SHA256_DIGEST_LENGTH) < 0)
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "could not create hex string");

for (i = 0; i < S3COMMS_SHA256_HEXSTR_LENGTH - 1; i++)
Expand All @@ -1919,6 +1924,6 @@ H5FD_s3comms_tostringtosign(char *dest, const char *req, const char *now, const

done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5ros3_tostringtosign() */
} /* end H5ros3_make_aws_stringtosign() */

#endif /* H5_HAVE_ROS3_VFD */
53 changes: 19 additions & 34 deletions src/H5FDs3comms.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,52 +452,37 @@ typedef struct {
extern "C" {
#endif

/****************************
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks messy, but if you look at the raw files you'll see it's just some reorg.

* HTTP FIELD LIST ROUTINES *
****************************/

H5_DLL herr_t H5FD_s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value);

/********************************
* HTTP REQUEST BUFFER ROUTINES *
********************************/

H5_DLL herr_t H5FD_s3comms_hrb_destroy(hrb_t *buf);

/* HTTP request buffer routines */
H5_DLL hrb_t *H5FD_s3comms_hrb_init_request(const char *verb, const char *resource, const char *host);
H5_DLL herr_t H5FD_s3comms_hrb_destroy(hrb_t *buf);
H5_DLL herr_t H5FD_s3comms_hrb_node_set(hrb_node_t **L, const char *name, const char *value);

/**********************
* S3REQUEST ROUTINES *
**********************/

H5_DLL herr_t H5FD_s3comms_s3r_close(s3r_t *handle);

H5_DLL size_t H5FD_s3comms_s3r_get_filesize(s3r_t *handle);

/* S3 request buffer routines */
H5_DLL s3r_t *H5FD_s3comms_s3r_open(const char url[], const char region[], const char id[],
const unsigned char signing_key[], const char token[]);

H5_DLL herr_t H5FD_s3comms_s3r_close(s3r_t *handle);
H5_DLL size_t H5FD_s3comms_s3r_get_filesize(s3r_t *handle);
H5_DLL herr_t H5FD_s3comms_s3r_read(s3r_t *handle, haddr_t offset, size_t len, void *dest);

/******************
* OTHER ROUTINES *
******************/
/* Functions that construct AWS things */
H5_DLL herr_t H5FD_s3comms_make_aws_canonical_request(char *canonical_request_dest, int cr_size,
char *signed_headers_dest, int sh_size,
hrb_t *http_request);
H5_DLL herr_t H5FD_s3comms_make_aws_signing_key(unsigned char *md, const char *secret, const char *region,
const char *iso8601now);
H5_DLL herr_t H5FD_s3comms_make_aws_stringtosign(char *dest, const char *req_str, const char *now,
const char *region);

/* Misc routines */
H5_DLL struct tm *gmnow(void);
H5_DLL herr_t H5FD_s3comms_free_purl(parsed_url_t *purl);

H5_DLL herr_t H5FD_s3comms_aws_canonical_request(char *canonical_request_dest, int cr_size,
char *signed_headers_dest, int sh_size, hrb_t *http_request);

H5_DLL herr_t H5FD_s3comms_free_purl(parsed_url_t *purl);

/* Testing routines */
#ifdef H5FD_S3COMMS_TESTING
H5_DLL herr_t H5FD_s3comms_load_aws_profile(const char *name, char *key_id_out, char *secret_access_key_out,
char *aws_region_out);
#endif /* H5FD_S3COMMS_TESTING */

H5_DLL herr_t H5FD_s3comms_signing_key(unsigned char *md, const char *secret, const char *region,
const char *iso8601now);

H5_DLL herr_t H5FD_s3comms_tostringtosign(char *dest, const char *req_str, const char *now,
const char *region);
#ifdef __cplusplus
}
#endif
Expand Down
1 change: 1 addition & 0 deletions test/ros3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "H5FDprivate.h" /* Virtual File Driver utilities */
#include "H5FDros3.h" /* this file driver's utilities */
#define H5FD_S3COMMS_TESTING
#include "H5FDs3comms.h" /* for loading of credentials */

#ifdef H5_HAVE_ROS3_VFD
Expand Down
Loading
Loading