From 2be78ea4c0715f4315adb8f2021954b66e4f0b38 Mon Sep 17 00:00:00 2001 From: John Connelly Date: Wed, 4 Jan 2023 12:23:25 -0800 Subject: [PATCH 1/3] Retry request on specific authentication errors. In some Instance Principal cases, the security token returned from OCI Identity may have a very short expiration. This change removes the expiration check, and allows a single retry on requests that get specific authN errors. It also adds trace-level logging of authentication token details. --- .../java/oracle/nosql/driver/http/Client.java | 6 ++- .../driver/iam/SecurityTokenSupplier.java | 39 ++++++++++++------- .../oracle/nosql/driver/ops/QueryRequest.java | 2 +- .../oracle/nosql/driver/ops/TableResult.java | 13 +++++++ 4 files changed, 42 insertions(+), 18 deletions(-) diff --git a/driver/src/main/java/oracle/nosql/driver/http/Client.java b/driver/src/main/java/oracle/nosql/driver/http/Client.java index 64308776..26be80a2 100644 --- a/driver/src/main/java/oracle/nosql/driver/http/Client.java +++ b/driver/src/main/java/oracle/nosql/driver/http/Client.java @@ -717,8 +717,10 @@ public Result execute(Request kvRequest) { throw new NoSQLException("Unexpected exception: " + rae.getMessage(), rae); } catch (InvalidAuthorizationException iae) { - /* allow a single retry on clock skew errors */ - if (iae.getMessage().contains("clock skew") == false || + /* allow a single retry on clock skew / auth errors */ + String msg = iae.getMessage(); + if ((msg.contains("clock skew") == false && + msg.contains("request signature is invalid") == false) || kvRequest.getNumRetries() > 0) { /* same as NoSQLException below */ kvRequest.setRateLimitDelayedMs(rateDelayedMs); diff --git a/driver/src/main/java/oracle/nosql/driver/iam/SecurityTokenSupplier.java b/driver/src/main/java/oracle/nosql/driver/iam/SecurityTokenSupplier.java index cc0280c2..a6722ec8 100644 --- a/driver/src/main/java/oracle/nosql/driver/iam/SecurityTokenSupplier.java +++ b/driver/src/main/java/oracle/nosql/driver/iam/SecurityTokenSupplier.java @@ -174,6 +174,18 @@ private synchronized String refreshAndGetTokenInternal() { } SecurityToken token = getSecurityTokenFromIAM(); token.validate(minTokenLifetime); + + /* + * Allow logging of token expiration details + */ + long tokenLifetime = token.getExpiryMS() - token.getCreationTime(); + logTrace(logger, "New security token: lifetime=" + tokenLifetime + + ", expiry=" + token.getExpiryMS() + ", creation=" + + token.getCreationTime()); + if (tokenLifetime < minTokenLifetime) { + logTrace(logger, "token:\n" + token.getSecurityToken()); + } + return token.getSecurityToken(); } @@ -265,10 +277,17 @@ String getStringClaim(String key) { return tokenClaims.get(key); } + long getCreationTime() { + return creationTime; + } + + long getExpiryMS() { + return expiryMS; + } + /* - * Validate the token, also check if the lifetime of token - * is longer than specified minimal lifetime, throws IAE - * if any validation fails. + * Validate the token. + * Throws IAE if any validation fails. */ void validate(long minTokenLifetime) { if (tokenClaims == null) { @@ -288,19 +307,9 @@ void validate(long minTokenLifetime) { } /* - * This is just a safety check, shouldn't happen in OCI environment, - * because security tokens always have more than one hour lifetime. - * It would only be thrown if short-living token being used or - * signature cache is misconfigured. To fix, must adjust the cache - * duration in both cases. + * Note: expiry check removed, as some tokens may have very short + * expirations. */ - long tokenLifetime = expiryMS - creationTime; - if (tokenLifetime < minTokenLifetime) { - throw new IllegalArgumentException( - "Security token has less lifetime than signature cache " + - "duration, reduce signature cache duration less than " + - tokenLifetime + " milliseconds, token:\n" + securityToken); - } /* * Next compare the public key inside the JWT is the same diff --git a/driver/src/main/java/oracle/nosql/driver/ops/QueryRequest.java b/driver/src/main/java/oracle/nosql/driver/ops/QueryRequest.java index 134db28b..c7df2562 100644 --- a/driver/src/main/java/oracle/nosql/driver/ops/QueryRequest.java +++ b/driver/src/main/java/oracle/nosql/driver/ops/QueryRequest.java @@ -651,7 +651,7 @@ public QueryRequest setConsistency(Consistency consistency) { * * @return this * - * @since 5.3.0 + * @since 5.4.0 */ public QueryRequest setDurability(Durability durability) { setDurabilityInternal(durability); diff --git a/driver/src/main/java/oracle/nosql/driver/ops/TableResult.java b/driver/src/main/java/oracle/nosql/driver/ops/TableResult.java index 2d5cd2b8..d60a8242 100644 --- a/driver/src/main/java/oracle/nosql/driver/ops/TableResult.java +++ b/driver/src/main/java/oracle/nosql/driver/ops/TableResult.java @@ -111,6 +111,8 @@ public String getDdl() { * the on-premise service. * * @return the table OCID + * + * @since 5.4 */ public String getTableId() { return tableOcid; @@ -122,6 +124,8 @@ public String getTableId() { * Returns compartment id of the target table * * @return the compartment id if set + * + * @since 5.4 */ public String getCompartmentId() { return compartmentOrNamespace; @@ -135,6 +139,8 @@ public String getCompartmentId() { * is in a namespace. * * @return the namespace id if set + * + * @since 5.4 */ public String getNamespace() { return compartmentOrNamespace; @@ -155,6 +161,8 @@ public String getTableName() { * Returns the JSON-formatted schema of the table if available and null if * not * @return the schema + * + * @since 5.4 */ public String getSchema() { return schema; @@ -177,6 +185,8 @@ public TableLimits getTableLimits() { * if available, or null otherwise. * * @return the FreeFormTags + * + * @since 5.4 */ public FreeFormTags getFreeFormTags() { return freeFormTags; @@ -189,6 +199,8 @@ public FreeFormTags getFreeFormTags() { * if available, or null otherwise. * * @return the DefinedTags + * + * @since 5.4 */ public DefinedTags getDefinedTags() { return definedTags; @@ -204,6 +216,7 @@ public DefinedTags getDefinedTags() { * optimistic concurrency control mechanism. * * @return the matchETag + * * @since 5.4 */ public String getMatchETag() { From 63375b367ef710de47584fa63ad0068f557ab818 Mon Sep 17 00:00:00 2001 From: John Connelly Date: Wed, 4 Jan 2023 13:01:23 -0800 Subject: [PATCH 2/3] Retry on all InvalidAuthorizationExceptions --- driver/src/main/java/oracle/nosql/driver/http/Client.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/driver/src/main/java/oracle/nosql/driver/http/Client.java b/driver/src/main/java/oracle/nosql/driver/http/Client.java index 26be80a2..d4db7568 100644 --- a/driver/src/main/java/oracle/nosql/driver/http/Client.java +++ b/driver/src/main/java/oracle/nosql/driver/http/Client.java @@ -718,10 +718,7 @@ public Result execute(Request kvRequest) { rae.getMessage(), rae); } catch (InvalidAuthorizationException iae) { /* allow a single retry on clock skew / auth errors */ - String msg = iae.getMessage(); - if ((msg.contains("clock skew") == false && - msg.contains("request signature is invalid") == false) || - kvRequest.getNumRetries() > 0) { + if (kvRequest.getNumRetries() > 0) { /* same as NoSQLException below */ kvRequest.setRateLimitDelayedMs(rateDelayedMs); statsControl.observeError(kvRequest); From 4b84a765b461bfaa156a98cdc614648d0e67ae76 Mon Sep 17 00:00:00 2001 From: John Connelly Date: Thu, 5 Jan 2023 07:41:37 -0800 Subject: [PATCH 3/3] Updated comment --- driver/src/main/java/oracle/nosql/driver/http/Client.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/driver/src/main/java/oracle/nosql/driver/http/Client.java b/driver/src/main/java/oracle/nosql/driver/http/Client.java index d4db7568..29874596 100644 --- a/driver/src/main/java/oracle/nosql/driver/http/Client.java +++ b/driver/src/main/java/oracle/nosql/driver/http/Client.java @@ -717,7 +717,11 @@ public Result execute(Request kvRequest) { throw new NoSQLException("Unexpected exception: " + rae.getMessage(), rae); } catch (InvalidAuthorizationException iae) { - /* allow a single retry on clock skew / auth errors */ + /* + * Allow a single retry for invalid/expired auth + * This includes "clock skew" errors + * This does not include permissions-related errors + */ if (kvRequest.getNumRetries() > 0) { /* same as NoSQLException below */ kvRequest.setRateLimitDelayedMs(rateDelayedMs);