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.

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

Add support for authenticating access to S3 buckets using temporary credentials #3086

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
76 changes: 45 additions & 31 deletions src/H5FDros3.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,16 @@

#ifdef H5_HAVE_ROS3_VFD

/* toggle function call prints: 1 turns on
/* toggle debugging; pick a level
*/
#define ROS3_DEBUG 0
#define ROS3_DEBUG_NONE 0
#define ROS3_DEBUG_TRACE_API 1
#define ROS3_DEBUG_TRACE_INTERNAL 2
#define ROS3_DEBUG ROS3_DEBUG_NONE

/* toggle stats collection and reporting
*/
#define ROS3_STATS 0
#define ROS3_STATS 1

/* The driver identification number, initialized at runtime
*/
Expand Down Expand Up @@ -314,18 +317,18 @@ H5FD_ros3_init(void)
if (H5I_INVALID_HID == H5FD_ROS3_g) {
HGOTO_ERROR(H5E_ID, H5E_CANTREGISTER, H5I_INVALID_HID, "unable to register ros3");
}
}

#if ROS3_STATS
/* pre-compute statsbin boundaries
*/
for (bin_i = 0; bin_i < ROS3_STATS_BIN_COUNT; bin_i++) {
unsigned long long value = 0;
/* pre-compute statsbin boundaries */
/* do it only during initial registration */
for (bin_i = 0; bin_i < ROS3_STATS_BIN_COUNT; bin_i++) {
unsigned long long value = 0;

ROS3_STATS_POW(bin_i, &value)
ros3_stats_boundaries[bin_i] = value;
}
ROS3_STATS_POW(bin_i, &value)
ros3_stats_boundaries[bin_i] = value;
}
#endif
}

ret_value = H5FD_ROS3_g;

Expand Down Expand Up @@ -622,15 +625,15 @@ H5FD__ros3_fapl_free(void *_fa)
*----------------------------------------------------------------------------
*/
static herr_t
ros3_reset_stats(H5FD_ros3_t *file)
H5FD__ros3_reset_stats(H5FD_ros3_t *file)
{
unsigned i = 0;
herr_t ret_value = SUCCEED;

FUNC_ENTER_PACKAGE

#if ROS3_DEBUG
HDprintf("ros3_reset_stats() called\n");
#if ROS3_DEBUG >= ROS3_DEBUG_TRACE_INTERNAL
HDprintf("H5FD__ros3_reset_stats() called\n");
#endif

if (file == NULL)
Expand Down Expand Up @@ -737,10 +740,10 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "problem while computing signing key")

handle = H5FD_s3comms_s3r_open(url, (const char *)fa.aws_region, (const char *)fa.secret_id,
(const unsigned char *)signing_key);
(const char *)fa.session_token, (const unsigned char *)signing_key);
}
else
handle = H5FD_s3comms_s3r_open(url, NULL, NULL, NULL);
handle = H5FD_s3comms_s3r_open(url, NULL, NULL, NULL, NULL);

if (handle == NULL)
/* If we want to check CURL's say on the matter in a controlled
Expand All @@ -758,7 +761,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));

#if ROS3_STATS
if (FAIL == ros3_reset_stats(file))
if (FAIL == H5FD__ros3_reset_stats(file))
HGOTO_ERROR(H5E_INTERNAL, H5E_UNINITIALIZED, NULL, "unable to reset file statistics")
#endif /* ROS3_STATS */

Expand Down Expand Up @@ -833,7 +836,7 @@ H5FD__ros3_open(const char *url, unsigned flags, hid_t fapl_id, haddr_t maxaddr)
*----------------------------------------------------------------------------
*/
static herr_t
ros3_fprint_stats(FILE *stream, const H5FD_ros3_t *file)
H5FD__ros3_fprint_stats(FILE *stream, const H5FD_ros3_t *file)
{
herr_t ret_value = SUCCEED;
parsed_url_t *purl = NULL;
Expand Down Expand Up @@ -914,7 +917,7 @@ ros3_fprint_stats(FILE *stream, const H5FD_ros3_t *file)
* PRINT OVERVIEW *
******************/

HDfprintf(stream, "TOTAL READS: %llu (%llu meta, %llu raw)\n", count_raw + count_meta, count_meta,
HDfprintf(stream, "TOTAL READS: %lu (%lu meta, %lu raw)\n", count_raw + count_meta, count_meta,
count_raw);
HDfprintf(stream, "TOTAL BYTES: %llu (%llu meta, %llu raw)\n", bytes_raw + bytes_meta, bytes_meta,
bytes_raw);
Expand Down Expand Up @@ -1039,7 +1042,7 @@ ros3_fprint_stats(FILE *stream, const H5FD_ros3_t *file)
re_dub /= 1024.0;
HDassert(suffix_i < sizeof(suffixes));

HDfprintf(stream, " %8.3f%c %7d %7d %8.3f%c %8.3f%c %8.3f%c %8.3f%c\n", re_dub,
HDfprintf(stream, " %8.3f%c %7llu %7llu %8.3f%c %8.3f%c %8.3f%c %8.3f%c\n", re_dub,
suffixes[suffix_i], /* bin ceiling */
m->count, /* metadata reads */
r->count, /* raw data reads */
Expand Down Expand Up @@ -1090,17 +1093,17 @@ H5FD__ros3_close(H5FD_t H5_ATTR_UNUSED *_file)
HDassert(file != NULL);
HDassert(file->s3r_handle != NULL);

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

#if ROS3_STATS
/* TODO: mechanism to re-target stats printout */
if (ros3_fprint_stats(stdout, file) == FAIL)
if (H5FD__ros3_fprint_stats(stdout, file) == FAIL)
HGOTO_ERROR(H5E_INTERNAL, H5E_ERROR, FAIL, "problem while writing file statistics")
#endif /* ROS3_STATS */

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

/* Release the file info */
file = H5FL_FREE(H5FD_ros3_t, file);

Expand All @@ -1127,8 +1130,9 @@ H5FD__ros3_close(H5FD_t H5_ATTR_UNUSED *_file)
* + fapl aws_region
* + fapl secret_id
* + fapl secret_key
* + fapl session_token
*
* tl;dr -> check URL, check crentials
* tl;dr -> check URL, check credentials
*
* Return:
*
Expand Down Expand Up @@ -1235,6 +1239,16 @@ H5FD__ros3_cmp(const H5FD_t *_f1, const H5FD_t *_f2)
else if (f2->fa.secret_key[0] != '\0')
HGOTO_DONE(-1)

/* FAPL: SESSION_TOKEN */
if (f1->fa.session_token[0] != '\0' && f2->fa.session_token[0] != '\0') {
if (HDstrcmp(f1->fa.session_token, f2->fa.session_token))
HGOTO_DONE(-1)
}
else if (f1->fa.session_token[0] != '\0')
HGOTO_DONE(-1)
else if (f2->fa.session_token[0] != '\0')
HGOTO_DONE(-1)

done:
FUNC_LEAVE_NOAPI(ret_value)
} /* H5FD__ros3_cmp() */
Expand Down Expand Up @@ -1263,7 +1277,7 @@ H5FD__ros3_query(const H5FD_t H5_ATTR_UNUSED *_file, unsigned long *flags)
{
FUNC_ENTER_PACKAGE_NOERR

#if ROS3_DEBUG
#if ROS3_DEBUG >= ROS3_DEBUG_TRACE_INTERNAL
HDfprintf(stdout, "H5FD__ros3_query() called.\n");
#endif

Expand Down Expand Up @@ -1303,7 +1317,7 @@ H5FD__ros3_get_eoa(const H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type)

FUNC_ENTER_PACKAGE_NOERR

#if ROS3_DEBUG
#if ROS3_DEBUG >= ROS3_DEBUG_TRACE_INTERNAL
HDfprintf(stdout, "H5FD__ros3_get_eoa() called.\n");
#endif

Expand Down Expand Up @@ -1334,7 +1348,7 @@ H5FD__ros3_set_eoa(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, haddr_t addr)

FUNC_ENTER_PACKAGE_NOERR

#if ROS3_DEBUG
#if ROS3_DEBUG >= ROS3_DEBUG_TRACE_INTERNAL
HDfprintf(stdout, "H5FD__ros3_set_eoa() called.\n");
#endif

Expand Down Expand Up @@ -1368,7 +1382,7 @@ H5FD__ros3_get_eof(const H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type)

FUNC_ENTER_PACKAGE_NOERR

#if ROS3_DEBUG
#if ROS3_DEBUG >= ROS3_DEBUG_TRACE_INTERNAL
HDfprintf(stdout, "H5FD__ros3_get_eof() called.\n");
#endif

Expand Down
10 changes: 6 additions & 4 deletions src/H5FDros3.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,20 @@
*
****************************************************************************/

#define H5FD_CURR_ROS3_FAPL_T_VERSION 1
#define H5FD_CURR_ROS3_FAPL_T_VERSION 2

#define H5FD_ROS3_MAX_REGION_LEN 32
#define H5FD_ROS3_MAX_SECRET_ID_LEN 128
#define H5FD_ROS3_MAX_SECRET_KEY_LEN 128
#define H5FD_ROS3_MAX_REGION_LEN 32
#define H5FD_ROS3_MAX_SECRET_ID_LEN 128
#define H5FD_ROS3_MAX_SECRET_KEY_LEN 128
#define H5FD_ROS3_MAX_SESSION_TOKEN_LEN 4096

typedef struct H5FD_ros3_fapl_t {
int32_t version;
hbool_t authenticate;
char aws_region[H5FD_ROS3_MAX_REGION_LEN + 1];
char secret_id[H5FD_ROS3_MAX_SECRET_ID_LEN + 1];
char secret_key[H5FD_ROS3_MAX_SECRET_KEY_LEN + 1];
char session_token[H5FD_ROS3_MAX_SESSION_TOKEN_LEN + 1];
Copy link
Member

Choose a reason for hiding this comment

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

Changing a struct in a public header breaks binary compatibility so this change could not be merged downstream to 1.14, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the change comes with a bump in H5FD_CURR_ROS3_FAPL_T_VERSION, so it is easy to detect clients that have not been recompiled against the updated headers/library. we should be able to restore binary compatibility. shall we walk through the use cases, or are you guys happy to say this is a 1.15 fix?

} H5FD_ros3_fapl_t;

#ifdef __cplusplus
Expand Down
Loading