Skip to content

Commit

Permalink
More H5FDs3comms.c/h cleanup (HDFGroup#5231)
Browse files Browse the repository at this point in the history
* Rename functions that create AWS strings to include _make_aws_
  in the name
* Separate out the test-only load AWS credentials function
  • Loading branch information
derobins authored Jan 14, 2025
1 parent 46658a5 commit 01d5a97
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 95 deletions.
16 changes: 8 additions & 8 deletions src/H5FDros3.c
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -452,52 +452,37 @@ typedef struct {
extern "C" {
#endif

/****************************
* 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
3 changes: 2 additions & 1 deletion test/ros3.c
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 Expand Up @@ -484,7 +485,7 @@ test_eof_eoa(void)
H5FD_t *fd = NULL;
hid_t fapl_id = H5I_INVALID_HID;

TESTING("ROS3 eof/eoa gets and sets");
TESTING("ros3 eof/eoa gets and sets");

if (s3_test_credentials_loaded == 0) {
SKIPPED();
Expand Down
Loading

0 comments on commit 01d5a97

Please sign in to comment.