diff --git a/.github/workflows/test.java.yml b/.github/workflows/test.java.yml index 99061b550..05d46b06f 100644 --- a/.github/workflows/test.java.yml +++ b/.github/workflows/test.java.yml @@ -1,19 +1,43 @@ name: Test-Java on: + # The gate. Runs on the merge result (head merged into base), so it can block + # a bad change before it lands, on release branches as well as master. pull_request: + branches: + - master + - 'release/sdk/java/core/**' + paths: + - 'sdk/java/core/**' + - '.github/workflows/test.java.yml' + # master only. Two purposes: catch a direct push that bypassed a PR, and seed + # the writable Gradle cache that every release-branch PR run then restores. + # Deliberately NOT on release/**, where it would only re-test a commit the + # pull_request run already tested. + push: branches: [ master ] paths: - 'sdk/java/core/**' - '.github/workflows/test.java.yml' + # Manual re-run after a transient infrastructure failure, no empty commit needed. + workflow_dispatch: permissions: contents: read +# One in-flight run per ref. A new push supersedes the previous run rather than +# racing it, which also keeps simultaneous Maven Central requests down. Never +# cancel a master run, since that is what populates the cache. +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: test-java: runs-on: ubuntu-latest strategy: + # One flaky JDK must not hide the results of the other three. + fail-fast: false matrix: java-version: [ '8', '11', '17', '21' ] name: KSM test with Java ${{ matrix.java-version }} @@ -34,6 +58,26 @@ jobs: uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0 with: gradle-version: '8.14' + # Writable on master only, so one seeded cache serves every branch. + # Without this the cache is never written and every job resolves the + # full dependency graph over the network. + cache-read-only: ${{ github.event_name != 'push' }} - name: Build and Test - run: gradle build test + shell: bash + run: | + log="${RUNNER_TEMP}/gradle-output.log" + for attempt in 1 2 3; do + if gradle build test 2>&1 | tee "${log}"; then + exit 0 + fi + if ! grep -qE 'Too Many Requests|Could not (resolve|GET|download)' "${log}"; then + echo "::error::Build or tests failed; not a dependency-resolution error, so not retrying" + exit 1 + fi + delay=$((attempt * 30)) + echo "::warning::Dependency resolution failed on attempt ${attempt}; retrying in ${delay}s" + sleep "${delay}" + done + echo "::error::Dependency resolution still failing after 3 attempts" + exit 1 diff --git a/examples/java/android-example/README.md b/examples/java/android-example/README.md index 1d1b715e2..9f89eb553 100644 --- a/examples/java/android-example/README.md +++ b/examples/java/android-example/README.md @@ -1,75 +1,63 @@ # Android Example using KSM Java SDK -**The absolute simplest Android app to prove the Keeper Secrets Manager Java SDK works on Android.** +This is a minimal Android app that shows the Keeper Secrets Manager Java SDK works on Android. -> **WARNING: This is a DEMO application for educational purposes only. -> Do NOT use this code in production without implementing proper security measures.** +> **WARNING**: This is a demo application for educational purposes only. +> Do not use this code in production without implementing proper security measures. ## Prerequisites -Before running this example, ensure you have: +Before running this example, make sure you have: -1. **Android Studio** (Arctic Fox or newer recommended) -2. **Android SDK** with API level 26+ (minSdk requirement) -3. **Keeper Secrets Manager Account** - [Sign up here](https://www.keepersecurity.com/) -4. **One-Time Access Token** - Generate from Keeper Secrets Manager: - - Log into Keeper Secrets Manager - - Navigate to your application - - Generate a one-time access token - - The token format is: `US:XXXXXX` or `EU:XXXXXX` (region prefix + token) +1. **Android Studio** (Arctic Fox or newer) +2. **Android SDK** with API level 26 or higher (minSdk requirement) +3. **Keeper Secrets Manager Account**: [Sign up here](https://www.keepersecurity.com/) +4. **One-Time Access Token**: Generate from Keeper Secrets Manager: + - Log in to Keeper Secrets Manager. + - Go to your application. + - Generate a one-time access token. + - The token format is `US:XXXXXX` or `EU:XXXXXX` (region prefix followed by the token). -## 🎯 Purpose +## Purpose -This is the **minimal working example** mentioned in the Android Compatibility Analysis. It demonstrates that with just proper threading, the SDK works as-is on Android. +This example shows that with proper threading, the SDK works as-is on Android. It uses `InMemoryStorage` and the default `HttpsURLConnection` to keep the configuration as simple as possible. -## ⚡ What This Proves +## What This Example Shows -✅ **SDK works on Android** with minimal changes -✅ **InMemoryStorage works** (no file I/O issues) -✅ **Crypto operations work** (AES/GCM, ECDH, ECDSA) -✅ **Network communication works** (HttpsURLConnection) -✅ **No ANR with proper threading** (Coroutines) +- The SDK initializes on Android. +- `InMemoryStorage` works (no file I/O required). +- Crypto operations work (AES/GCM, ECDH, ECDSA). +- Network communication works (`HttpsURLConnection`). +- Running SDK calls on a background thread prevents ANR errors. -## 📦 What's Included +## Limitations -**This is intentionally minimal:** -- ❌ No encrypted storage (uses `InMemoryStorage`) -- ❌ No OkHttp (uses default `HttpsURLConnection`) -- ❌ No fancy UI (simple XML layout) -- ❌ Config not persisted (lost on app restart) -- ❌ Minimal error handling +This example is intentionally minimal. It does not include: -## 🚀 Quick Start +- Encrypted storage (uses `InMemoryStorage`) +- OkHttp (uses the default `HttpsURLConnection`) +- Persisted configuration (config is lost when the app restarts) +- Full error handling -### 1. Open Project in Android Studio (30 seconds) +## Quick Start -### 2. Wait for Gradle Sync (2 minutes) +1. Open the project in Android Studio. +2. Wait for Gradle sync to complete. +3. Click **Run**. +4. Enter your Keeper one-time token. +5. Tap **Initialize**. +6. Wait 2-5 seconds. +7. Tap **Load Secrets**. -Let Android Studio download dependencies. +## Expected Output -### 3. Run (30 seconds) - -Click the green ▶️ Run button. - -### 4. Test (1 minute) - -1. Enter your Keeper one-time token -2. Tap "1️⃣ Initialize" -3. Wait 2-5 seconds -4. Tap "2️⃣ Load Secrets" -5. See your secrets! - -**Total time: ~4 minutes** ⚡ - -## 📋 What You'll See - -### After Initialize: +After initialization: ``` ✅ Initialized successfully! Now tap 'Load Secrets' ``` -### After Load Secrets: +After loading secrets: ``` ✅ Secrets loaded successfully! @@ -91,140 +79,100 @@ Now tap 'Load Secrets' (no password) ``` -## 🔍 Code Overview +## Code Overview -### MainActivity.kt (~150 lines) - -The entire app in one file: +`MainActivity.kt` (~150 lines) contains the entire app. The SDK calls require only a background thread: ```kotlin -// Initialize KSM private fun initializeKsm(token: String) { lifecycleScope.launch { withContext(Dispatchers.IO) { - // SDK call - works as-is! initializeStorage(storage, token) } statusText.text = "✅ Initialized!" } } -// Load secrets private fun loadSecrets() { lifecycleScope.launch { val secrets = withContext(Dispatchers.IO) { val options = SecretsManagerOptions(storage) - getSecrets(options) // SDK call - works! + getSecrets(options) } displaySecrets(secrets) } } ``` -**That's it!** The SDK works with just proper threading. - -## 📊 Project Structure +## Project Structure ``` android-example/ -├── build.gradle.kts # Root config -├── settings.gradle.kts # Project settings +├── build.gradle.kts +├── settings.gradle.kts ├── gradle.properties ├── .gitignore -│ └── app/ - ├── build.gradle.kts # Dependencies (minimal!) - ├── src/main/ - │ ├── AndroidManifest.xml # Permissions - │ ├── java/com/keeper/minimal/ - │ │ └── MainActivity.kt # THE ENTIRE APP (150 lines) - │ └── res/ - │ ├── layout/ - │ │ └── activity_main.xml # Simple UI - │ └── values/ - │ └── strings.xml + ├── build.gradle.kts + └── src/main/ + ├── AndroidManifest.xml + ├── java/com/keeper/minimal/ + │ └── MainActivity.kt + └── res/ + ├── layout/ + │ └── activity_main.xml + └── values/ + └── strings.xml ``` -**Total files: 10** -**Total code: ~300 lines** - -## 🔧 Dependencies +Total: 10 files, ~300 lines of code. -**Minimal - only what's needed:** +## Dependencies ```kotlin dependencies { - // The SDK - REQUIRED implementation("com.keepersecurity.secrets-manager:keeper-secrets-manager-core:17.1.2") - - // Basic Android UI implementation("androidx.appcompat:appcompat:1.6.1") implementation("androidx.constraintlayout:constraintlayout:2.1.4") - - // Coroutines for background threading implementation("org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.3") } ``` -**That's all!** No OkHttp, no encryption libraries, no compose. - -## ✅ What Works - -- ✅ SDK initialization -- ✅ Fetching secrets -- ✅ Displaying secrets -- ✅ Password retrieval -- ✅ All crypto operations -- ✅ Network communication -- ✅ Runs on Android 8.0-16 (API 26-36) - -## ❌ What Doesn't Work / Limitations - -Since this is **intentionally minimal**: - -1. **No persistence** - Config lost on app restart (uses `InMemoryStorage`) -2. **Not optimized** - Uses `HttpsURLConnection` (battery drain) -3. **No encryption** - Storage not encrypted (just in-memory) -4. **Minimal error handling** - Basic try/catch only -5. **Simple UI** - No Material3, no fancy design -6. **No offline support** - Requires network for everything - +The example does not use OkHttp, encryption libraries, or Compose. ## Security Considerations for Production -This example intentionally uses simplified implementations for clarity. For production apps: +This example uses simplified implementations for educational purposes. For production apps: + +- **Token Storage**: Use Android Keystore or `EncryptedSharedPreferences` instead of in-memory storage. +- **Sensitive Data**: Use biometric authentication before the app displays sensitive data. +- **Network Security**: Implement certificate pinning. +- **Error Handling**: Do not expose internal error details to users. +- **Logging**: Remove all sensitive data from logs before release. +- **Code Obfuscation**: Enable ProGuard/R8 with the appropriate keep rules for the SDK. -- **Token Storage**: Use Android Keystore or EncryptedSharedPreferences instead of in-memory storage -- **Token Input**: Consider using biometric authentication before displaying sensitive data -- **Network Security**: Implement certificate pinning -- **Error Handling**: Never expose internal error details to users -- **Logging**: Remove all sensitive data from logs -- **Code Obfuscation**: Enable ProGuard/R8 with appropriate keep rules for the SDK +## Troubleshooting -## 🐛 Troubleshooting +### Gradle sync fails -### "Gradle sync failed" ```bash -# File → Invalidate Caches → Restart +# File > Invalidate Caches > Restart ``` -### "SDK location not found" +### SDK location not found + ```bash echo "sdk.dir=$HOME/Library/Android/sdk" > local.properties ``` -### "App crashes on initialization" -**Check Logcat:** -- Look for network errors -- Verify token format (starts with US:, EU:, etc.) -- Check internet connection - -### "Loading takes forever" -**This is expected on first run:** -- `SecureRandom.getInstanceStrong()` can take 3-5 seconds -- Subsequent runs are faster -- This is a known issue (see compatibility analysis) - -### "Config lost after restart" -**This is by design:** -- Using `InMemoryStorage` (not persisted) \ No newline at end of file +### App crashes on initialization + +Check Logcat for network errors. Make sure the token format starts with a region prefix (for example, `US:` or `EU:`). Make sure the device has an internet connection. + +### Loading takes a long time on first run + +`SecureRandom.getInstanceStrong()` can take 3-5 seconds on the first call. Subsequent calls are faster. This is expected behavior. + +### Config is lost after restart + +This is by design. The example uses `InMemoryStorage`, which does not persist the configuration to disk. diff --git a/examples/java/hello-secret/README.md b/examples/java/hello-secret/README.md index c8c3590f7..e16964227 100644 --- a/examples/java/hello-secret/README.md +++ b/examples/java/hello-secret/README.md @@ -1,11 +1,11 @@ # Keeper Secrets Manager Java SDK Example -Sample project demonstrating how to extract shared secrets from Keeper. +Sample project that shows how to extract shared secrets from Keeper. Prerequisites: - Java 8 or higher -- One or more one-time access tokens obtained from the owner of the secret. +- One or more one-time access tokens from the owner of the shared secret. Usage: @@ -18,6 +18,6 @@ For example: ./gradlew run --args="config.json US:EvdTdbH1xbHuRcja7QG3wMOyLUbvoQgF9WkkrHTdkh8" ``` -The One-Time Access Token is used once to initialize the SDK configuration. After the SDK configuration is initialized, the One-Time Access Token can be removed. +The SDK uses the One-Time Access Token once to initialize its configuration. After initialization, you can remove the token. For more information see our official documentation page https://docs.keeper.io/secrets-manager/secrets-manager/developer-sdk-library/java-sdk diff --git a/sdk/java/core/README.md b/sdk/java/core/README.md index 4174b4e2a..df5e022fd 100644 --- a/sdk/java/core/README.md +++ b/sdk/java/core/README.md @@ -4,6 +4,42 @@ For more information see our official documentation page https://docs.keeper.io/ # Change Log +## 17.4.0 +**Breaking Changes** +- `deleteFolder()` returns `SecretsManagerDeleteFolderResponse` instead of `SecretsManagerDeleteResponse`. The new type exposes a `folders` list, where each entry has `folderUid`, `responseCode`, and an optional `errorMessage`; callers that read `.records` on the old return type must switch to `.folders`. In practice no working code can be affected: the old return type required a `records` field while the backend sends `folders`, so every previous call to `deleteFolder()` threw `MissingFieldException` instead of returning a value. +- `KeeperRecord` gained a constructor parameter (`isEditable`) and `SecretsManagerOptions` gained three (`connectTimeoutMillis`, `readTimeoutMillis`, `proxyUrl`). Recompiling is enough: Kotlin source needs no edit, and Java call sites keep every constructor form published in 17.3.0 because both types carry `@JvmOverloads`. What does change is bytecode-level. The generated `copy()` methods and the synthetic constructor Kotlin emits for omitted default arguments both changed arity, so Kotlin code compiled against 17.3.0 throws `NoSuchMethodError` if the 17.4.0 jar is swapped in without recompiling. Rebuild dependents against 17.4.0 rather than replacing the jar in place. + +- KSM-531 - Add HTTP/HTTPS proxy support + - New `proxyUrl` option on `SecretsManagerOptions`, e.g. `SecretsManagerOptions(storage, proxyUrl = "http://proxy.local:8080")`. Java callers can also use `SecretsManagerOptions.withProxy(storage, "http://proxy.local:8080")`. + - Authenticated proxies use the `http://user:password@host:port` URL form. Reserved characters in the password can be percent-encoded (e.g. `p%40ss` for `p@ss`). An incomplete credential in an explicit `proxyUrl` is rejected as a configuration error, including an empty half (`http://user:@host`), which is what a templated proxy URL produces when its password variable goes unset. Ambient proxy settings never throw, but an empty password there is not treated as a credential either. + - Applies to secret queries (`getSecrets`), file uploads (`uploadFile`), and file downloads. File downloads require the options-taking overloads: `downloadFile(options, file)` and `downloadThumbnail(options, file)`. The single-argument forms `downloadFile(file)` and `downloadThumbnail(file)` use ambient proxy settings (`HTTPS_PROXY`, `https.proxyHost`, etc.) but do not pick up `allowUnverifiedCertificate` from options. Notation lookups that resolve file attachments (`getValue`) use ambient proxy settings but cannot carry `options.proxyUrl`; use `getNotationResults(options, notation)` for explicit-proxy-aware notation resolution. + - **Upgrade note:** 17.4.0 makes all SDK connections honour `HTTPS_PROXY` and `https.proxyHost` environment variables and JVM system properties that the SDK previously ignored. Deployments running in containers or environments where these variables are set for other tools should verify the values before upgrading. + - When `proxyUrl` is not set, the SDK checks JVM system properties (`https.proxyHost`/`https.proxyPort`) then the `HTTPS_PROXY`/`https_proxy` environment variables; `NO_PROXY`/`no_proxy` and `http.nonProxyHosts` exclusions are honored. `HTTP_PROXY` is intentionally excluded: all KSM traffic is HTTPS and an http-scoped setting would route traffic the operator may not have intended. + - Credentials found in ambient environment variables (`HTTPS_PROXY`) are detected but not registered with the JVM `Authenticator`. Registering the global `Authenticator` from ambient env values would silently interfere with other libraries in the same process. To use authenticated proxy credentials, pass them explicitly via the `proxyUrl` field in `SecretsManagerOptions`. + - **Authenticated proxies require a JVM startup flag in most applications.** Java disables Basic auth over HTTPS CONNECT tunnels by default (`jdk.http.auth.tunneling.disabledSchemes`, a CVE-2016-5597 mitigation), and that default locks in the first time `java.net.HttpURLConnection` is loaded (which any HTTP or HTTPS connection can trigger) before this SDK gets a chance to run. The SDK does clear it automatically when proxy credentials are supplied, but that only works if the SDK's proxied call happens to be the very first connection in the JVM, which is not the common case in a real application. If you use an authenticated proxy, set this **before your application makes any other HTTP or HTTPS call**: + - Command line: `-Djdk.http.auth.tunneling.disabledSchemes=` + - Environment variable (Java 9+): `JDK_JAVA_OPTIONS=-Djdk.http.auth.tunneling.disabledSchemes=` + - Environment variable (Java 8): `_JAVA_OPTIONS=-Djdk.http.auth.tunneling.disabledSchemes=` or `JAVA_TOOL_OPTIONS=-Djdk.http.auth.tunneling.disabledSchemes=` + + If this is not set in time, the SDK throws a `SecretsManagerException` with this exact remediation in the message rather than surfacing a bare 407, so the failure is loud and actionable, not a silent/confusing auth error. + - **Clearing that flag is process-wide, not SDK-scoped.** `jdk.http.auth.tunneling.disabledSchemes` is a JVM system property, so whether you set it at startup or the SDK clears it for you, Basic auth over CONNECT is re-enabled for every `java.net.HttpURLConnection` in the process, not only the SDK's. Two things keep the SDK's own write narrow: it happens only when you supply credentials in an explicit `proxyUrl`, and a value your application already set is left untouched. Applications sharing a JVM with other HTTP clients should treat this as a deliberate decision rather than something inherited from the SDK. + - Proxy credentials are supplied via a default `java.net.Authenticator` scoped to the configured proxy host, re-asserted on every proxied connection. Applications that install their own default `Authenticator` should pass proxy credentials through that mechanism instead. + - `cachingPostFunction` cannot carry `options.proxyUrl`; ambient proxies (`HTTPS_PROXY`, `https.proxyHost`) still apply. To use an explicit `proxyUrl`, use the default query path instead. + - `cachingPostFunction` now prints a warning to stderr when it falls back to stale cached data. +This output is unconditional and not gated by `loggingEnabled`, because the function has no access to `SecretsManagerOptions`. +KSM-1298 tracks the proper fix for the next release. + - `allowUnverifiedCertificate` now applies to file downloads and uploads when using the options-taking overloads, in addition to secret queries. Deployments that set this flag should be aware that certificate verification is now bypassed on all SDK outbound connections when it is enabled. +- KSM-1081 - Fixed `getFolders()` crashing when any folder in the response has a corrupted or missing key. The SDK now skips undecryptable folders and returns the remaining folders normally. The skipped-folder diagnostic names the exception type and is suppressed when `loggingEnabled` is false. +- KSM-1086 - Fixed `deleteFolder()` to return `SecretsManagerDeleteFolderResponse` (typed per-folder status), matching `deleteSecret()`. Both `deleteFolder()` and `deleteSecret()` now report per-item server failures to stderr, gated on `loggingEnabled`, and include them in the return value so callers can detect partial failures. +- KSM-1176 - `KeeperRecord` now exposes `isEditable: Boolean`, forwarded from the server response envelope. Callers can inspect this field before calling `updateSecret` to determine whether the app has write permission for the record. The field is the last constructor parameter and `KeeperRecord` carries `@JvmOverloads`, so Java code that constructs a record positionally, and Kotlin code that destructures one, are unaffected. +- KSM-1203 - Fixed `generatePassword` using a non-cryptographic PRNG (Kotlin `Random.Default`) for the final character shuffle. The shuffle now uses `SecureRandom`, so the entire password generation path is cryptographically secure. Passwords already generated do not need to be rotated: character selection always drew from `SecureRandom`, so only the arrangement of already-secret characters was affected, and the strength that remained is far beyond brute-force reach (for `generatePassword(32, 8, 8, 8, 8)`, roughly 137 of the 193 bits were never at risk, and the default `generatePassword()` was unaffected because all of its characters come from a single character set). +- KSM-1207 - Fixed all `HttpsURLConnection` calls defaulting to an infinite timeout. Added `connectTimeoutMillis` (default 5 000 ms) and `readTimeoutMillis` (default 30 000 ms) to `SecretsManagerOptions`. Both defaults come from a single constant shared with the overloads that take a `KeeperFile` without options, and `SecretsManagerOptions.toString()` reports both values. A stalled or unresponsive server now causes a `SocketTimeoutException` rather than an indefinite hang. The configured values apply to API requests and to the file upload transport. `downloadFile()` and `downloadThumbnail()` take only a `KeeperFile`, so they apply the built-in defaults rather than the values configured on `SecretsManagerOptions`. A custom `queryFunction` supplies its own transport and is responsible for its own timeouts. +- KSM-1248 - Server-supplied key IDs are validated against the embedded public key table before being stored. An unrecognized key ID throws `SecretsManagerException` and leaves storage unchanged. Key rotation retries are now capped at `MAX_KEY_ROTATION_RETRIES` (3); exhausted retries throw a typed error naming the last suggested key ID. +- KSM-1262 - On POSIX systems, config and cache files are now written via a temp-file swap with 0600 permissions set before data is written, closing the window where other local users could read the file during a write. Three behavior changes come with the new approach: (1) a symlinked config path is replaced by a regular file on the first write; (2) a config file in a directory without write permission (for example, a read-only container volume mount) will fail at temp-file creation, so move the config to a writable directory or use `InMemoryStorage` with an injected config string instead; (3) the config file is now read as UTF-8 explicitly, where it was previously read using the JVM default charset while always being written as UTF-8. A write that cannot be staged now reports what the file system returned and retains the original `IOException` as the exception cause, and `SecretsManagerException` gained a `(message, cause)` constructor to carry it. +- KSM-1269 - Fixed the Java CI workflow not running on pull requests targeting release branches. +The test matrix now triggers on both `master` and `release/sdk/java/core/**` targets. +- KSM-1270 - Fixed `getSharedFolderKey` looping indefinitely when server folder data contains a parent cycle. The function now tracks visited folder UIDs and exits on re-visit; `getFolders` skips the affected folders and continues normally. + ## 17.3.0 **Breaking Changes** - `KeeperFile.url` changed from `String` to `String?` — callers that access `url` directly must now handle null; `downloadFile()` already does this with a typed exception diff --git a/sdk/java/core/build.gradle.kts b/sdk/java/core/build.gradle.kts index ffcc58314..c51c41ec7 100644 --- a/sdk/java/core/build.gradle.kts +++ b/sdk/java/core/build.gradle.kts @@ -5,7 +5,7 @@ import org.jetbrains.kotlin.gradle.dsl.JvmTarget group = "com.keepersecurity.secrets-manager" // During publishing, If version ends with '-SNAPSHOT' then it will be published to Maven snapshot repository -version = "17.3.0" +version = "17.4.0" plugins { `java-library` diff --git a/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/CryptoUtils.kt b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/CryptoUtils.kt index 91bef3284..7219fd502 100644 --- a/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/CryptoUtils.kt +++ b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/CryptoUtils.kt @@ -56,12 +56,12 @@ internal fun bytesToBase64(data: ByteArray): String { } internal fun base64ToBytes(data: String): ByteArray { - if (data.isEmpty()) throw SecretsManagerException("Base64-encoded value is empty") // KSM-985 + if (data.isEmpty()) throw SecretsManagerException("Base64-encoded value is empty") return Base64.getDecoder().decode(data) } internal fun webSafe64ToBytes(data: String): ByteArray { - if (data.isEmpty()) throw SecretsManagerException("Base64url-encoded value is empty") // KSM-985 + if (data.isEmpty()) throw SecretsManagerException("Base64url-encoded value is empty") return Base64.getUrlDecoder().decode(data) } @@ -416,7 +416,7 @@ fun generatePassword( if (it.first > 0) passwordCharacters += randomSample(it.first, it.second) } - val pCharArray = passwordCharacters.toCharArray() - pCharArray.shuffle() - return String(pCharArray) + val pCharList = passwordCharacters.toMutableList() + Collections.shuffle(pCharList, SecureRandom.getInstanceStrong()) + return String(pCharList.toCharArray()) } diff --git a/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/LocalConfigStorage.kt b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/LocalConfigStorage.kt index c9aa1fd99..c5e3a129b 100644 --- a/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/LocalConfigStorage.kt +++ b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/LocalConfigStorage.kt @@ -6,33 +6,52 @@ import kotlinx.serialization.decodeFromString import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json import java.io.* +import java.nio.file.AtomicMoveNotSupportedException import java.nio.file.Files -import java.nio.file.attribute.PosixFilePermission +import java.nio.file.StandardCopyOption import java.nio.file.attribute.PosixFilePermissions import java.util.* import kotlin.collections.HashMap fun saveCachedValue(data: ByteArray) { - val file = File("cache.dat") - FileOutputStream(file).use { fos -> fos.write(data) } // KSM-855: .use{} closes on exception - - // Set file permissions to 0600 (owner read/write only) + val targetPath = File("cache.dat").absoluteFile.toPath() + val tmpPath = try { + Files.createTempFile(targetPath.parent, "ksm_", ".tmp") + } catch (e: IOException) { + // Report what actually failed: createTempFile also fails on a full or read-only volume, + // a missing parent, or an fd limit, none of which are a permission problem. + throw SecretsManagerException( + "Cannot write cache $targetPath: could not create a temporary file in ${targetPath.parent} " + + "(${e.javaClass.simpleName}: ${e.message}). The directory must exist and be writable.", + e + ) + } try { - val perms = PosixFilePermissions.fromString("rw-------") - Files.setPosixFilePermissions(file.toPath(), perms) - } catch (e: UnsupportedOperationException) { - // Windows or file system doesn't support POSIX permissions - // File.setReadable/setWritable provides basic protection - file.setReadable(false, false) // Remove all read permissions - file.setWritable(false, false) // Remove all write permissions - file.setReadable(true, true) // Owner read only - file.setWritable(true, true) // Owner write only + try { + Files.setPosixFilePermissions(tmpPath, PosixFilePermissions.fromString("rw-------")) + } catch (_: UnsupportedOperationException) { + tmpPath.toFile().let { f -> + f.setReadable(false, false) + f.setWritable(false, false) + f.setReadable(true, true) + f.setWritable(true, true) + } + } + Files.write(tmpPath, data) + try { + Files.move(tmpPath, targetPath, StandardCopyOption.ATOMIC_MOVE) + } catch (_: AtomicMoveNotSupportedException) { + Files.move(tmpPath, targetPath, StandardCopyOption.REPLACE_EXISTING) + } + } catch (e: Exception) { + try { Files.deleteIfExists(tmpPath) } catch (_: Exception) { } + throw e } } fun getCachedValue(): ByteArray { try { - return FileInputStream("cache.dat").use { it.readBytes() } // KSM-855: .use{} closes on exception + return FileInputStream("cache.dat").use { it.readBytes() } // .use{} closes on exception } catch (e: Exception) { throw SecretsManagerException("Cached value does not exist") } @@ -115,7 +134,7 @@ class LocalConfigStorage(configName: String? = null) : KeyValueStorage { private val file = configName?.let { File(it) } private var storage: InMemoryStorage = if (file != null && file.exists()) { - val content = BufferedReader(FileReader(file)).use { it.readText() } // KSM-855: was never closed + val content = file.readText(Charsets.UTF_8) InMemoryStorage(content) } else { InMemoryStorage() @@ -125,29 +144,49 @@ class LocalConfigStorage(configName: String? = null) : KeyValueStorage { private fun saveToFile() { if (file == null) return - val config = LocalConfig() - config.hostname = storage.getString(KEY_HOSTNAME) - config.clientId = storage.getString(KEY_CLIENT_ID) - config.privateKey = storage.getString(KEY_PRIVATE_KEY) - config.clientKey = storage.getString(KEY_CLIENT_KEY) - config.appKey = storage.getString(KEY_APP_KEY) - config.appOwnerPublicKey = storage.getString(KEY_OWNER_PUBLIC_KEY) - config.serverPublicKeyId = storage.getString(KEY_SERVER_PUBLIC_KEY_ID) - config.serverPublicKey = storage.getString(KEY_SERVER_PUBLIC_KEY) - val json = prettyJson.encodeToString(config) - BufferedWriter(FileWriter(file)).use { it.write(json) } // KSM-855: .use{} closes on exception - - // Set file permissions to 0600 (owner read/write only) + val targetPath = file.absoluteFile.toPath() + val tmpPath = try { + Files.createTempFile(targetPath.parent, "ksm_", ".tmp") + } catch (e: IOException) { + // Report what actually failed: createTempFile also fails on a full or read-only volume, + // a missing parent, or an fd limit, none of which are a permission problem. + throw SecretsManagerException( + "Cannot write config $targetPath: could not create a temporary file in ${targetPath.parent} " + + "(${e.javaClass.simpleName}: ${e.message}). The directory must exist and be writable; " + + "move the config to a writable directory or use InMemoryStorage with an injected config string.", + e + ) + } try { - val perms = PosixFilePermissions.fromString("rw-------") - Files.setPosixFilePermissions(file.toPath(), perms) - } catch (e: UnsupportedOperationException) { - // Windows or file system doesn't support POSIX permissions - // File.setReadable/setWritable provides basic protection - file.setReadable(false, false) // Remove all read permissions - file.setWritable(false, false) // Remove all write permissions - file.setReadable(true, true) // Owner read only - file.setWritable(true, true) // Owner write only + try { + Files.setPosixFilePermissions(tmpPath, PosixFilePermissions.fromString("rw-------")) + } catch (_: UnsupportedOperationException) { + tmpPath.toFile().let { f -> + f.setReadable(false, false) + f.setWritable(false, false) + f.setReadable(true, true) + f.setWritable(true, true) + } + } + val config = LocalConfig( + hostname = storage.getString(KEY_HOSTNAME), + clientId = storage.getString(KEY_CLIENT_ID), + privateKey = storage.getString(KEY_PRIVATE_KEY), + clientKey = storage.getString(KEY_CLIENT_KEY), + appKey = storage.getString(KEY_APP_KEY), + appOwnerPublicKey = storage.getString(KEY_OWNER_PUBLIC_KEY), + serverPublicKeyId = storage.getString(KEY_SERVER_PUBLIC_KEY_ID), + serverPublicKey = storage.getString(KEY_SERVER_PUBLIC_KEY) + ) + Files.write(tmpPath, prettyJson.encodeToString(config).toByteArray()) + try { + Files.move(tmpPath, targetPath, StandardCopyOption.ATOMIC_MOVE) + } catch (_: AtomicMoveNotSupportedException) { + Files.move(tmpPath, targetPath, StandardCopyOption.REPLACE_EXISTING) + } + } catch (e: Exception) { + try { Files.deleteIfExists(tmpPath) } catch (_: Exception) { } + throw e } } diff --git a/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/ProxySupport.kt b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/ProxySupport.kt new file mode 100644 index 000000000..aa32e04ae --- /dev/null +++ b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/ProxySupport.kt @@ -0,0 +1,336 @@ +package com.keepersecurity.secretsManager.core + +import java.io.IOException +import java.net.Authenticator +import java.net.HttpURLConnection.HTTP_PROXY_AUTH +import java.net.InetSocketAddress +import java.net.PasswordAuthentication +import java.net.Proxy +import java.net.URI +import java.util.Locale +import java.util.concurrent.ConcurrentHashMap +import javax.net.ssl.HttpsURLConnection + +internal data class ResolvedProxy(val proxy: Proxy, val username: String?, val password: String?, val isExplicit: Boolean = false) { + val hasCredentials: Boolean get() = username != null && password != null + override fun toString(): String = + "ResolvedProxy(proxy=$proxy, username=$username, " + + "password=${if (password != null) "" else null}, isExplicit=$isExplicit)" +} + +/** + * Seam over the ambient environment so proxy resolution stays deterministic under test. + * Production uses the real process environment and JVM system properties. + */ +internal interface ProxyEnvironment { + fun env(name: String): String? + fun property(name: String): String? +} + +internal object SystemProxyEnvironment : ProxyEnvironment { + override fun env(name: String): String? = System.getenv(name) + override fun property(name: String): String? = System.getProperty(name) +} + +/** + * Resolves the proxy for a target URL, or null when no proxy applies (caller then opens a direct + * connection). Precedence: explicit proxyUrl, then JVM system properties (https/http.proxyHost), + * then HTTP(S)_PROXY environment variables. NO_PROXY / http.nonProxyHosts exclude the target. + */ +internal fun resolveProxy( + explicitProxyUrl: String?, + targetUrl: String, + environment: ProxyEnvironment = SystemProxyEnvironment +): ResolvedProxy? { + // Explicit proxyUrl is authoritative: NO_PROXY / http.nonProxyHosts do not override it, + // and an unparseable value fails closed rather than falling through to a direct connection. + // Checked before parsing targetUrl so that non-URI-parseable hosts (e.g. underscore names + // that java.net.URI rejects but java.net.URL connects to) still honour an explicit proxy. + if (!explicitProxyUrl.isNullOrBlank()) { + return parseProxy(explicitProxyUrl, isExplicit = true) + ?: throw SecretsManagerException("proxyUrl '${redactProxyUrl(explicitProxyUrl)}' could not be parsed as a valid proxy URL") + } + + val targetHost = runCatching { URI(targetUrl).host }.getOrNull() ?: return null + + // Ambient proxy: apply exclusions before selecting a candidate. + if (isExcluded(targetHost, environment)) return null + // HTTP_PROXY / http_proxy are intentionally excluded: all KSM traffic is HTTPS, and routing + // it through an http-scoped proxy setting would silently proxy traffic the operator may not + // have intended (same reasoning as the http.proxyHost exclusion above). + val candidate = systemPropertyProxy(environment) + ?: environment.env("HTTPS_PROXY")?.takeIf { it.isNotBlank() } + ?: environment.env("https_proxy")?.takeIf { it.isNotBlank() } + ?: return null + + return parseProxy(candidate, isExplicit = false) +} + +private fun systemPropertyProxy(environment: ProxyEnvironment): String? { + // Only https.proxyHost is honored: all KSM traffic is HTTPS, and the JDK's own ProxySelector + // never applies http.proxyHost to HTTPS URLs. Applying it here would silently proxy traffic + // the operator may not have intended to route through that host. + val host = environment.property("https.proxyHost")?.takeIf { it.isNotBlank() } ?: return null + val port = environment.property("https.proxyPort")?.takeIf { it.isNotBlank() } ?: "443" + return "$host:$port" +} + +private fun parseProxy(raw: String, isExplicit: Boolean = false): ResolvedProxy? { + val normalized = if (raw.contains("://")) raw else "http://$raw" + val uri = runCatching { URI(normalized) }.getOrNull() ?: return null + val host = uri.host ?: return null + + // The JDK's HttpURLConnection cannot speak TLS to a proxy. An https:// proxy URL misleads + // callers into thinking TLS is used between the client and the proxy when it is not. + if (uri.scheme?.equals("https", ignoreCase = true) == true) { + if (isExplicit) throw SecretsManagerException( + "Invalid proxy URL: HTTPS proxies are not supported by the JDK's HttpURLConnection. " + + "Use an http:// proxy URL. The SDK still connects to KSM over HTTPS regardless of the proxy scheme." + ) + return null + } + + // https:// proxy URLs are rejected above; no need to handle the https scheme here. + val port = if (uri.port != -1) uri.port else 80 + val userInfo = uri.userInfo + val username = userInfo?.substringBefore(':')?.takeIf { it.isNotEmpty() } + // An empty half is not a credential: it yields null here exactly as a missing half does, so + // hasCredentials stays false and the check below sees both cases the same way. + val password = userInfo?.substringAfter(':', "")?.takeIf { it.isNotEmpty() } + + // Credentials in explicit config must be complete. Rejecting an empty half alongside a missing + // one turns http://user@host, http://:secret@host and http://user:@host into the same config + // error, instead of letting the last one reach the proxy and come back as an opaque 407. A + // templated URL whose password variable went unset (http://$USER:$PASS@host) is the usual way + // that shape appears. + if (isExplicit && userInfo != null && (username == null || password == null)) { + throw SecretsManagerException("Invalid proxy URL: both username and password are required (format: http://user:pass@host:port)") + } + + // Use createUnresolved to defer DNS to connection time, avoiding two round-trips per request + // and preventing .local / mDNS lookups from blocking proxy-resolution in tests. + val proxy = runCatching { + Proxy(Proxy.Type.HTTP, InetSocketAddress.createUnresolved(host, port)) + }.getOrElse { e -> + if (isExplicit) throw SecretsManagerException("Invalid proxy URL: port out of range (0-65535)", e) + return null + } + return ResolvedProxy(proxy, username, password, isExplicit) +} + +internal fun isExcluded(host: String, environment: ProxyEnvironment): Boolean { + val noProxy = environment.env("NO_PROXY")?.takeIf { it.isNotBlank() } + ?: environment.env("no_proxy")?.takeIf { it.isNotBlank() } + val nonProxyHosts = environment.property("http.nonProxyHosts") + val patterns = buildList { + noProxy?.split(',')?.forEach { add(it.trim()) } + nonProxyHosts?.split('|')?.forEach { add(it.trim()) } + }.filter { it.isNotEmpty() } + val lowerHost = host.lowercase(Locale.ROOT) + return patterns.any { pattern -> + val p = pattern.lowercase(Locale.ROOT).removePrefix("*").removePrefix(".") + pattern == "*" || lowerHost == p || lowerHost.endsWith(".$p") + } +} + +/** + * Opens an HTTPS connection through the resolved proxy (or directly when none applies), applying + * the cert-verification bypass and registering proxy credentials when present. + */ +internal fun openProxiedConnection( + targetUrl: String, + explicitProxyUrl: String?, + allowUnverifiedCertificate: Boolean, + environment: ProxyEnvironment = SystemProxyEnvironment +): HttpsURLConnection { + val targetHost = runCatching { URI(targetUrl).host }.getOrNull() + val resolved = resolveProxy(explicitProxyUrl, targetUrl, environment) + + // When the host is explicitly excluded by NO_PROXY/http.nonProxyHosts, force a direct + // connection with Proxy.NO_PROXY so the JDK's default ProxySelector cannot re-introduce a + // proxy that the operator has opted out of. An unparseable ambient proxy that falls through + // resolveProxy as null is NOT treated as an exclusion — only isExcluded() determines that. + val isExcluded = resolved == null && targetHost != null && isExcluded(targetHost, environment) + + // Only register the JVM-wide Authenticator when the proxy (and its credentials) came from the + // caller's explicit proxyUrl option, not from ambient env variables. Ambient env credentials + // are owned by the host application; overriding the default Authenticator from them would + // silently interfere with other libraries that installed their own Authenticator. + // Must run before openConnection(): the JDK reads jdk.http.auth.tunneling.disabledSchemes + // into a static field the first time HttpURLConnection's class is loaded, so clearing it + // afterward has no effect on this connection. + if (resolved?.isExplicit == true && resolved.hasCredentials) { + val address = resolved.proxy.address() as InetSocketAddress + ProxyAuthenticator.register(address.hostString, address.port, resolved.username!!, resolved.password!!) + } + val url = URI.create(targetUrl).toURL() + val proxy = when { + resolved != null -> resolved.proxy + isExcluded -> Proxy.NO_PROXY + else -> null + } + val connection = (if (proxy != null) url.openConnection(proxy) else url.openConnection()) + as HttpsURLConnection + if (allowUnverifiedCertificate) { + connection.sslSocketFactory = trustAllSslSocketFactory() + } + return connection +} + +/** + * Reads the response code, turning an authenticated-proxy rejection (407) into a clear, + * actionable [SecretsManagerException] instead of a bare status code or opaque IOException. + * + * A 407 here almost always means the JDK's own jdk.http.auth.tunneling.disabledSchemes guard + * (see openProxiedConnection) was already locked to its default "Basic disabled" value by an + * earlier HTTPS connection somewhere in this process, before this call had a chance to clear it. + * That guard can only be cleared before the *first* HTTPS connection in the JVM, and no runtime + * code can undo it retroactively. + */ +internal fun HttpsURLConnection.checkedResponseCode( + explicitProxyUrl: String?, + targetUrl: String, + environment: ProxyEnvironment = SystemProxyEnvironment +): Int { + val resolved = resolveProxy(explicitProxyUrl, targetUrl, environment) + val requiresProxyAuth = resolved?.hasCredentials ?: false + if (!requiresProxyAuth) { + return responseCode + } + val isAmbientCredentials = !resolved!!.isExplicit + return try { + val code = responseCode + if (code == HTTP_PROXY_AUTH) throw SecretsManagerException(proxyAuthFailureMessage(null, isAmbientCredentials)) else code + } catch (e: IOException) { + // The JDK surfaces "Unable to tunnel through proxy. Proxy returns 'HTTP/1.1 407 ...'" as + // an IOException on the response-code read. Check for "407" specifically to avoid + // misclassifying legitimate proxy 502/503/504 tunnel errors as auth failures. + if (e.message?.contains("407") == true) { + throw SecretsManagerException(proxyAuthFailureMessage(e.message, isAmbientCredentials), e) + } + throw e + } +} + +internal fun proxyAuthFailureMessage(cause: String?, isAmbientCredentials: Boolean = false): String { + val baseMessage = "Authenticated proxy rejected the connection (407 Proxy Authentication Required)" + + (cause?.let { ": $it" } ?: "") + val guidance = if (isAmbientCredentials) { + ". Credentials were detected in environment variables (HTTP_PROXY, HTTPS_PROXY, etc.) " + + "but were not registered with the JVM Authenticator because no explicit proxyUrl was passed " + + "to the SDK. Registering the global Authenticator from ambient environment variables risks " + + "interfering with other libraries in the same process. To use the proxy credentials, set " + + "the proxyUrl field in SecretsManagerOptions with the full authenticated proxy URL." + } else { + ". First, double-check the proxy username/password in proxyUrl. If those are correct, " + + "the likely cause is that this JVM already made an HTTPS connection before this one: Java " + + "disables Basic auth over HTTPS CONNECT tunnels by default (CVE-2016-5597 mitigation), and " + + "that default locks in the first time any HTTPS connection is made in the process, so the " + + "SDK's own attempt to clear it then comes too late, and Java won't have even attempted to " + + "send credentials (a 407 looks the same either way). Set the JVM property " + + "jdk.http.auth.tunneling.disabledSchemes to an empty value at process startup, before any " + + "other HTTPS traffic: pass -Djdk.http.auth.tunneling.disabledSchemes= on the java command " + + "line, or set it via the JDK_JAVA_OPTIONS environment variable (Java 9+) or " + + "_JAVA_OPTIONS/JAVA_TOOL_OPTIONS (Java 8)." + } + return baseMessage + guidance +} + +/** + * KSM endpoints are HTTPS, so authenticated proxies are reached via a CONNECT tunnel. On Java 8 the + * only way to supply tunnel credentials is the process-global default Authenticator, so this is + * installed lazily (only when an authenticated proxy is actually used) and answers solely for the + * registered proxy host/port. Migrating to per-connection HttpURLConnection.setAuthenticator is a + * Java 9+ change tracked for the next major. + */ +internal object ProxyAuthenticator : Authenticator() { + private data class Credential(val username: String, val password: CharArray) + + private val credentials = ConcurrentHashMap() + + fun register(host: String, port: Int, username: String, password: String) { + credentials[key(host, port)] = Credential(username, password.toCharArray()) + enableBasicProxyAuthOverTunnel() + // Re-asserted on every call rather than once: Authenticator.getDefault() (needed to check + // whether we're still installed) isn't available until Java 9, and the default can be + // silently replaced or cleared by anything else in the process (other libraries, test + // cleanup) between connections. + Authenticator.setDefault(this) + } + + override fun getPasswordAuthentication(): PasswordAuthentication? { + if (requestorType != RequestorType.PROXY) return null + val credential = credentials[key(requestingHost, requestingPort)] ?: return null + return PasswordAuthentication(credential.username, credential.password.clone()) + } + + // Drops every registered credential. Exists for test isolation: this object is a process-wide + // singleton, so without it a test that registers a proxy credential leaves it answerable for + // the rest of the JVM's life. Not part of the SDK's supported surface. + fun reset() = credentials.clear() + + private fun key(host: String, port: Int) = "${host.lowercase(Locale.ROOT)}:$port" +} + +/** + * Strips credentials from a proxy URL for safe use in error messages without risking credential + * exposure. Works textually rather than via URI parsing so it handles reserved-character passwords + * (containing '@', spaces, or other characters that make URI parsing fail). + */ +internal fun redactProxyUrl(proxyUrl: String): String { + val schemeEnd = proxyUrl.indexOf("://") + val authorityStart = if (schemeEnd < 0) 0 else schemeEnd + 3 + val afterScheme = proxyUrl.substring(authorityStart) + val atIndex = afterScheme.lastIndexOf('@') + if (atIndex < 0) return proxyUrl + val userInfo = afterScheme.substring(0, atIndex) + val colonIndex = userInfo.indexOf(':') + val redactedUserInfo = if (colonIndex >= 0) { + userInfo.substring(0, colonIndex) + ":***" + } else { + "$userInfo:***" + } + return proxyUrl.substring(0, authorityStart) + redactedUserInfo + "@" + afterScheme.substring(atIndex + 1) +} + +/** + * Basic auth on HTTPS CONNECT tunnels is disabled by default since 8u111. Clear it (best effort, + * unless the host app set it explicitly) so authenticated proxies work. May still require the + * -Djdk.http.auth.tunneling.disabledSchemes= JVM flag if a tunneled connection was opened earlier. + * + * The property is JVM-wide, so clearing it re-enables Basic over CONNECT for every + * HttpURLConnection in the process, not only this SDK's. Two things keep that scoped: it runs only + * when the caller supplied credentials in an explicit proxyUrl, and a value the host application + * already set is left untouched. + */ +private fun enableBasicProxyAuthOverTunnel() { + val property = "jdk.http.auth.tunneling.disabledSchemes" + if (System.getProperty(property) == null) { + System.setProperty(property, "") + } +} + +/** + * Builds a socket factory that trusts all certificates. Used only when + * SecretsManagerOptions.allowUnverifiedCertificate is set, and kept private to this file so it + * is not accessible as a public API entry point from consumer code. + */ +private fun trustAllSslSocketFactory(): javax.net.ssl.SSLSocketFactory { + val trustAllCerts: Array = arrayOf( + object : javax.net.ssl.X509TrustManager { + private val acceptedIssuers = arrayOf() + override fun checkClientTrusted(certs: Array?, authType: String?) {} + override fun checkServerTrusted(certs: Array?, authType: String?) {} + override fun getAcceptedIssuers(): Array = acceptedIssuers + } + ) + val sslContext = javax.net.ssl.SSLContext.getInstance("TLS") + try { + sslContext.init(null, trustAllCerts, java.security.SecureRandom()) + } catch (e: java.security.NoSuchAlgorithmException) { + e.printStackTrace() + } catch (e: java.security.KeyManagementException) { + e.printStackTrace() + } + return sslContext.socketFactory +} diff --git a/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/RecordData.kt b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/RecordData.kt index cf364db68..e53b5704b 100644 --- a/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/RecordData.kt +++ b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/RecordData.kt @@ -20,7 +20,7 @@ data class KeeperRecordData @JvmOverloads constructor( var title: String, val type: String, val fields: MutableList, - @EncodeDefault var custom: MutableList = mutableListOf(), // KSM-823: always serialize "custom":[] even when empty + @EncodeDefault var custom: MutableList = mutableListOf(), // always serialize "custom":[] even when empty var notes: String? = null ) { inline fun getField(): T? { @@ -59,8 +59,8 @@ data class KeeperFileData @JvmOverloads constructor( val name: String, val type: String? = null, val size: Long, - @Serializable(with = FlexibleLongSerializer::class) // KSM-673: iOS/Android clients send fractional epoch seconds - val lastModified: Long = 0 // KSM-854: non-SDK clients omit this field; default 0 matches .NET behavior + @Serializable(with = FlexibleLongSerializer::class) // iOS/Android clients send fractional epoch seconds + val lastModified: Long = 0 // non-SDK clients omit this field; default 0 matches .NET behavior ) @Serializable diff --git a/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/SecretsManager.kt b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/SecretsManager.kt index 6321bd19e..e927118d1 100644 --- a/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/SecretsManager.kt +++ b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/SecretsManager.kt @@ -8,6 +8,7 @@ import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.jsonObject import kotlinx.serialization.json.jsonPrimitive +import java.io.IOException import java.net.HttpURLConnection.HTTP_FORBIDDEN import kotlinx.serialization.json.JsonArray import kotlinx.serialization.json.JsonElement @@ -18,24 +19,26 @@ import kotlinx.serialization.json.longOrNull import kotlinx.serialization.json.doubleOrNull import java.net.HttpURLConnection.HTTP_OK import java.net.URI -import java.security.KeyManagementException -import java.security.NoSuchAlgorithmException import java.security.SecureRandom -import java.security.cert.X509Certificate import java.time.Instant import java.util.* import java.util.concurrent.* import kotlin.random.Random -import javax.net.ssl.* -const val KEEPER_CLIENT_VERSION = "mj17.3.0" +const val KEEPER_CLIENT_VERSION = "mj17.4.0" -// Throttle retry (KSM-876 / KSM-878). The backend throttles HTTP 403 {"error":"throttled"} +// Throttle retry. The backend throttles HTTP 403 {"error":"throttled"} // per clientId+endpoint (100 requests / 10s window; memcached TTL 10s that resets on every // request, so the counter only clears after 10s of silence). const val MAX_THROTTLE_RETRIES = 5 const val BASE_THROTTLE_DELAY_SEC = 11 // 1s safety margin over the backend's 10s memcached TTL const val MAX_THROTTLE_DELAY_SEC = 176 // caps a backend-supplied retry_after from forcing an excessive wait +const val MAX_KEY_ROTATION_RETRIES = 3 // parity with JS SDK open PR #1078 + +// Connection timeouts. Single source for both the SecretsManagerOptions defaults and the +// overloads that take a KeeperFile without options, so the two cannot drift apart. +private const val DEFAULT_CONNECT_TIMEOUT_MS = 5_000 +private const val DEFAULT_READ_TIMEOUT_MS = 30_000 const val KEY_HOSTNAME = "hostname" // base url for the Secrets Manager service const val KEY_SERVER_PUBLIC_KEY_ID = "serverPublicKeyId" @@ -67,13 +70,32 @@ data class SecretsManagerOptions @JvmOverloads constructor( val serverPublicKey: String? = null, val serverPublicKeyId: String? = null, // Override the sleep between throttle retries (primarily for tests). Defaults to Thread.sleep. - val throttleSleepMillis: ((Long) -> Unit)? = null + val throttleSleepMillis: ((Long) -> Unit)? = null, + val connectTimeoutMillis: Int = DEFAULT_CONNECT_TIMEOUT_MS, + val readTimeoutMillis: Int = DEFAULT_READ_TIMEOUT_MS, + val proxyUrl: String? = null ) { + companion object { + // Static factory for Java callers — preferred over the 8-arg @JvmOverloads form. + // Adding proxyUrl to the primary constructor changed the copy() binary signature; + // callers compiled against 17.3.0 must recompile against 17.4.0. + @JvmStatic + fun withProxy(storage: KeyValueStorage, proxyUrl: String) = + SecretsManagerOptions(storage = storage, proxyUrl = proxyUrl) + } + init { testSecureRandom() serverPublicKey?.let { storage.saveString(KEY_SERVER_PUBLIC_KEY, it) } serverPublicKeyId?.let { storage.saveString(KEY_SERVER_PUBLIC_KEY_ID, it) } } + + override fun toString(): String = + "SecretsManagerOptions(storage=$storage, queryFunction=$queryFunction, " + + "allowUnverifiedCertificate=$allowUnverifiedCertificate, loggingEnabled=$loggingEnabled, " + + "serverPublicKey=$serverPublicKey, serverPublicKeyId=$serverPublicKeyId, " + + "throttleSleepMillis=$throttleSleepMillis, connectTimeoutMillis=$connectTimeoutMillis, " + + "readTimeoutMillis=$readTimeoutMillis, proxyUrl=${if (proxyUrl != null) "" else null})" } data class QueryOptions @JvmOverloads constructor( @@ -238,7 +260,7 @@ private data class SecretsManagerResponseRecord( * `is_launch_credential`, `is_iam_user`, `belongs_to` and `rotation_settings`; or no data at all * (a pure record reference). * - path "ai_settings" / "jit_settings" (self-links): data is AES-256-GCM encrypted under the - * owning record's key — see [getDecryptedData]. + * owning record's key; see [getDecryptedData]. * * Accessors never throw: parse, decode or decryption failures yield null/false. [getLinkData] * returns the complete parsed payload with nested objects and arrays preserved, so fields unknown @@ -296,7 +318,7 @@ data class KeeperRecordLink( * Get a strict boolean value from the parsed JSON data; missing or non-boolean values are false. * * When [checkAllowedSettings] is true the nested `allowedSettings` object is consulted if the - * key is absent at the top level — a top-level boolean wins. The backend nests permission flags + * key is absent at the top level; a top-level boolean wins. The backend nests permission flags * under `allowedSettings` in `path:"meta"` links. */ private fun getBooleanValue(key: String, checkAllowedSettings: Boolean = false): Boolean { @@ -476,9 +498,8 @@ data class KeeperRecordLink( } /** - * Check if this link contains encrypted data by examining the actual content - * This method inspects the data to determine if it's encrypted, rather than - * relying on path naming conventions + * Check if this link contains encrypted data by examining the actual content, rather than + * relying on path naming conventions (see [mightBeEncrypted]). * @return true if the data appears to be encrypted */ fun hasEncryptedData(): Boolean { @@ -494,9 +515,8 @@ data class KeeperRecordLink( } /** - * Decrypt the link data using the provided record key - * This method attempts decryption only if the data appears to be encrypted - * + * Decrypt the link data using the provided record key. + * * @param recordKey The record's encryption key * @return Decrypted string data, or null if decryption fails */ @@ -520,12 +540,11 @@ data class KeeperRecordLink( } /** - * Get link data - automatically handles both encrypted and plain JSON - * - * This method is designed to be forward-compatible as Keeper evolves - * the data structures. Returns a Map to preserve all fields, even ones - * this SDK version doesn't know about yet. - * + * Get link data - automatically handles both encrypted and plain JSON. + * + * Forward-compatible as Keeper evolves the data structures: returns a Map to preserve all + * fields, even ones this SDK version doesn't know about yet. + * * @param recordKey Optional key for decrypting encrypted link data * @return Parsed data as a Map, or null if parsing fails */ @@ -584,11 +603,7 @@ data class KeeperRecordLink( val sampleSize = minOf(str.length, 100) return (printableCount.toFloat() / sampleSize) > 0.9f } - - // ============================================================================ - // Convenience Methods for Settings Access - // ============================================================================ - + /** * Get AI settings data from this link * @@ -630,12 +645,8 @@ data class KeeperRecordLink( } /** - * Get settings data for any path - * - * This method works for current and future settings paths. - * It automatically detects whether the data is encrypted and - * handles it appropriately. - * + * Get settings data for any current or future settings path. + * * @param settingsPath The path to check (e.g., "ai_settings", "security_settings") * @param recordKey The record's encryption key (required for encrypted data) * @return Settings data as a Map, or null if path doesn't match or parsing fails @@ -647,7 +658,7 @@ data class KeeperRecordLink( } /** - * Get PAM settings data from this link — only when [path] == "meta". + * Get PAM settings data from this link; only valid when [path] == "meta". * * Meta links are self-links (recordUid == owning record) carrying the record's own PAM settings: * `allowedSettings`, `rotateOnTermination`, `version`, `no_update_services`. Plain JSON today; @@ -662,7 +673,7 @@ private data class SecretsManagerResponseFile( val fileUid: String, val fileKey: String, val data: String, - val url: String?, // KSM-765: server may omit url; nullable prevents NPE on deserialization + val url: String?, // server may omit url; nullable prevents NPE on deserialization val thumbnailUrl: String? ) @@ -689,6 +700,18 @@ data class SecretsManagerDeleteResponseRecord( val responseCode: String ) +@Serializable +data class SecretsManagerDeleteFolderResponseRecord( + val errorMessage: String? = null, + val folderUid: String, + val responseCode: String +) + +@Serializable +data class SecretsManagerDeleteFolderResponse( + val folders: List +) + @Serializable private data class SecretsManagerAddFileResponse( val url: String, @@ -713,7 +736,7 @@ data class KeeperSecrets(val appData: AppData, val records: List, @Serializable data class AppData(val title: String, val type: String) -data class KeeperRecord( +data class KeeperRecord @JvmOverloads constructor( val recordKey: ByteArray, val recordUid: String, var folderUid: String? = null, @@ -722,7 +745,18 @@ data class KeeperRecord( val data: KeeperRecordData, val revision: Long, val files: List? = null, - val links: List? = null + val links: List? = null, + // Appended rather than inserted: Java sees no default arguments, so an interior + // parameter would shift the constructor Java callers already compile against. + /** + * Write permission for this record, set by the backend from how the application's share was + * configured: `true` when the application may call [updateSecret] on it, `false` when the + * share is read-only. Informational only; the SDK does not enforce it before an update. + * + * The default applies only to direct construction. Records returned by [getSecrets] always + * carry the server's value, because the field is required in the response envelope. + */ + val isEditable: Boolean = false ) { fun getPassword(): String? { val passwordField = data.getField() ?: return null @@ -771,7 +805,7 @@ data class KeeperFile( val fileKey: ByteArray, val fileUid: String, val data: KeeperFileData, - val url: String?, // KSM-765: nullable; server may omit url for files without a download URL + val url: String?, // nullable; server may omit url for files without a download URL val thumbnailUrl: String? ) @@ -1019,7 +1053,7 @@ fun getNotationResults(options: SecretsManagerOptions, notation: String): List): SecretsManagerDeleteResponse { val payload = prepareDeletePayload(options.storage, recordUids) val responseData = postQuery(options, "delete_secret", payload) - return nonStrictJson.decodeFromString(bytesToString(responseData)) + val response: SecretsManagerDeleteResponse = nonStrictJson.decodeFromString(bytesToString(responseData)) + for (r in response.records) { + if (r.responseCode != "ok" && options.loggingEnabled) { + System.err.println("Failed to delete record ${r.recordUid}: ${r.responseCode} ${r.errorMessage ?: ""}") + } + } + return response } @ExperimentalSerializationApi -fun deleteFolder(options: SecretsManagerOptions, folderUids: List, forceDeletion: Boolean = false): SecretsManagerDeleteResponse { +fun deleteFolder(options: SecretsManagerOptions, folderUids: List, forceDeletion: Boolean = false): SecretsManagerDeleteFolderResponse { val payload = prepareDeleteFolderPayload(options.storage, folderUids, forceDeletion) val responseData = postQuery(options, "delete_folder", payload) - return nonStrictJson.decodeFromString(bytesToString(responseData)) + val response: SecretsManagerDeleteFolderResponse = nonStrictJson.decodeFromString(bytesToString(responseData)) + for (f in response.folders) { + if (f.responseCode != "ok" && options.loggingEnabled) { + System.err.println("Failed to delete folder ${f.folderUid}: ${f.responseCode} ${f.errorMessage ?: ""}") + } + } + return response } @ExperimentalSerializationApi @@ -1150,30 +1196,46 @@ fun uploadFile(options: SecretsManagerOptions, ownerRecord: KeeperRecord, file: val payloadAndFile = prepareFileUploadPayload(options.storage, ownerRecord, file) val responseData = postQuery(options, "add_file", payloadAndFile.payload) val response = nonStrictJson.decodeFromString(bytesToString(responseData)) - val uploadResult = uploadFile(response.url, response.parameters, payloadAndFile.encryptedFile) + val uploadResult = uploadFile(response.url, response.parameters, payloadAndFile.encryptedFile, options.proxyUrl, options.allowUnverifiedCertificate, options.connectTimeoutMillis, options.readTimeoutMillis) if (uploadResult.statusCode != response.successStatusCode) { throw SecretsManagerException("Upload failed (${bytesToString(uploadResult.data)}), code ${uploadResult.statusCode}") } return payloadAndFile.payload.fileRecordUid } -fun downloadFile(file: KeeperFile): ByteArray { +fun downloadFile(options: SecretsManagerOptions, file: KeeperFile): ByteArray { val url = file.url ?: throw SecretsManagerException("File ${file.fileUid} has no download URL") - return downloadFile(file, url) + return downloadFileFromUrl(file, url, options.proxyUrl, options.allowUnverifiedCertificate, options.connectTimeoutMillis, options.readTimeoutMillis) } -fun downloadThumbnail(file: KeeperFile): ByteArray { +@JvmOverloads +fun downloadFile(file: KeeperFile, proxyUrl: String? = null): ByteArray { + val url = file.url ?: throw SecretsManagerException("File ${file.fileUid} has no download URL") + return downloadFileFromUrl(file, url, proxyUrl, false) +} + +fun downloadThumbnail(options: SecretsManagerOptions, file: KeeperFile): ByteArray { if (file.thumbnailUrl == null) { throw SecretsManagerException("Thumbnail does not exist for the file ${file.fileUid}") } - return downloadFile(file, file.thumbnailUrl) + return downloadFileFromUrl(file, file.thumbnailUrl, options.proxyUrl, options.allowUnverifiedCertificate, options.connectTimeoutMillis, options.readTimeoutMillis) } -private fun downloadFile(file: KeeperFile, url: String): ByteArray { - val connection = URI.create(url).toURL().openConnection() as HttpsURLConnection // KSM-855 +@JvmOverloads +fun downloadThumbnail(file: KeeperFile, proxyUrl: String? = null): ByteArray { + if (file.thumbnailUrl == null) { + throw SecretsManagerException("Thumbnail does not exist for the file ${file.fileUid}") + } + return downloadFileFromUrl(file, file.thumbnailUrl, proxyUrl, false) +} + +private fun downloadFileFromUrl(file: KeeperFile, url: String, proxyUrl: String? = null, allowUnverifiedCertificate: Boolean = false, connectTimeoutMillis: Int = DEFAULT_CONNECT_TIMEOUT_MS, readTimeoutMillis: Int = DEFAULT_READ_TIMEOUT_MS): ByteArray { + val connection = openProxiedConnection(url, proxyUrl, allowUnverifiedCertificate) try { + connection.connectTimeout = connectTimeoutMillis + connection.readTimeout = readTimeoutMillis connection.requestMethod = "GET" - val statusCode = connection.responseCode + val statusCode = connection.checkedResponseCode(proxyUrl, url) val data = when { connection.errorStream != null -> connection.errorStream.readBytes() else -> connection.inputStream.readBytes() @@ -1187,31 +1249,45 @@ private fun downloadFile(file: KeeperFile, url: String): ByteArray { } } -private fun uploadFile(url: String, parameters: String, fileData: ByteArray): KeeperHttpResponse { +private fun uploadFile(url: String, parameters: String, fileData: ByteArray, proxyUrl: String?, allowUnverifiedCertificate: Boolean = false, connectTimeoutMillis: Int = DEFAULT_CONNECT_TIMEOUT_MS, readTimeoutMillis: Int = DEFAULT_READ_TIMEOUT_MS): KeeperHttpResponse { var statusCode: Int var data: ByteArray val boundary = String.format("----------%x", Instant.now().epochSecond) val boundaryBytes: ByteArray = stringToBytes("\r\n--$boundary") val paramJson = Json.parseToJsonElement(parameters) as JsonObject - val connection = URI.create(url).toURL().openConnection() as HttpsURLConnection // KSM-855 + val connection = openProxiedConnection(url, proxyUrl, allowUnverifiedCertificate) try { + connection.connectTimeout = connectTimeoutMillis + connection.readTimeout = readTimeoutMillis connection.requestMethod = "POST" connection.useCaches = false connection.doInput = true connection.doOutput = true connection.setRequestProperty("Content-Type", "multipart/form-data; boundary=$boundary") - connection.outputStream.use { os -> - for (param in paramJson.entries) { + try { + connection.outputStream.use { os -> + for (param in paramJson.entries) { + os.write(boundaryBytes) + os.write(stringToBytes("\r\nContent-Disposition: form-data; name=\"${param.key}\"\r\n\r\n${param.value.jsonPrimitive.content}")) + } + os.write(boundaryBytes) + os.write(stringToBytes("\r\nContent-Disposition: form-data; name=\"file\"\r\nContent-Type: application/octet-stream\r\n\r\n")) + os.write(fileData) os.write(boundaryBytes) - os.write(stringToBytes("\r\nContent-Disposition: form-data; name=\"${param.key}\"\r\n\r\n${param.value.jsonPrimitive.content}")) + os.write(stringToBytes("--\r\n")) } - os.write(boundaryBytes) - os.write(stringToBytes("\r\nContent-Disposition: form-data; name=\"file\"\r\nContent-Type: application/octet-stream\r\n\r\n")) - os.write(fileData) - os.write(boundaryBytes) - os.write(stringToBytes("--\r\n")) + } catch (e: IOException) { + // For tunneled HTTPS connections the JDK can throw here when the CONNECT tunnel fails. + if (e.message?.contains("407") == true) { + val resolved = resolveProxy(proxyUrl, url) + if (resolved != null && resolved.hasCredentials) { + val isAmbientCredentials = !resolved.isExplicit + throw SecretsManagerException(proxyAuthFailureMessage(e.message, isAmbientCredentials), e) + } + } + throw e } - statusCode = connection.responseCode + statusCode = connection.checkedResponseCode(proxyUrl, url) data = when { connection.errorStream != null -> connection.errorStream.readBytes() else -> connection.inputStream.readBytes() @@ -1247,7 +1323,7 @@ private fun fetchAndDecryptSecrets( } else { appKey = storage.getBytes(KEY_APP_KEY) ?: throw SecretsManagerException("App key is missing from the storage") } - // KSM-753: records created via non-SDK clients in shared folders appear in response.records[] + // Records created via non-SDK clients in shared folders appear in response.records[] // with innerFolderUid set; their recordKey is encrypted with the folder key, not the app key. val folderKeyMap: Map = response.folders ?.mapNotNull { f -> @@ -1273,7 +1349,9 @@ private fun fetchAndDecryptSecrets( records.add(decryptedRecord) } } catch (e: Exception) { - System.err.println("Record ${it.recordUid} skipped due to error: ${e.javaClass.simpleName}, ${e.message}") + if (options.loggingEnabled) { + System.err.println("Record ${it.recordUid} skipped due to error: ${e.javaClass.simpleName}, ${e.message}") + } } } } @@ -1291,11 +1369,15 @@ private fun fetchAndDecryptSecrets( records.add(decryptedRecord) } } catch (e: Exception) { - System.err.println("Record ${record.recordUid} in folder ${folder.folderUid} skipped due to error: ${e.javaClass.simpleName}, ${e.message}") + if (options.loggingEnabled) { + System.err.println("Record ${record.recordUid} in folder ${folder.folderUid} skipped due to error: ${e.javaClass.simpleName}, ${e.message}") + } } } } catch (e: Exception) { - System.err.println("Folder ${folder.folderUid} skipped due to error: ${e.javaClass.simpleName}, ${e.message}") + if (options.loggingEnabled) { + System.err.println("Folder ${folder.folderUid} skipped due to error: ${e.javaClass.simpleName}, ${e.message}") + } } } } @@ -1332,7 +1414,9 @@ private fun decryptRecord(record: SecretsManagerResponseRecord, recordKey: ByteA ) ) } catch (e: Exception) { - System.err.println("File ${it.fileUid} skipped due to error: ${e.javaClass.simpleName}, ${e.message}") + if (options.loggingEnabled) { + System.err.println("File ${it.fileUid} skipped due to error: ${e.javaClass.simpleName}, ${e.message}") + } } } } @@ -1402,15 +1486,16 @@ private fun decryptRecord(record: SecretsManagerResponseRecord, recordKey: ByteA } return if (recordData != null) KeeperRecord( - recordKey, - record.recordUid, - null, - null, - record.innerFolderUid, - recordData, - record.revision, - files, - record.links + recordKey = recordKey, + recordUid = record.recordUid, + folderUid = null, + folderKey = null, + innerFolderUid = record.innerFolderUid, + data = recordData, + revision = record.revision, + files = files, + links = record.links, + isEditable = record.isEditable ) else null } @@ -1429,26 +1514,37 @@ private fun fetchAndDecryptFolders( val folders: MutableList = mutableListOf() val appKey = storage.getBytes(KEY_APP_KEY) ?: throw SecretsManagerException("App key is missing from the storage") response.folders.forEach { folder -> - val folderKey: ByteArray = if (folder.parent == null) { - decrypt(folder.folderKey, appKey) - } else { - val sharedFolderKey = getSharedFolderKey(folders, response.folders, folder.parent) ?: throw SecretsManagerException("Folder data inconsistent - unable to locate shared folder") - decrypt(folder.folderKey, sharedFolderKey, true) + try { + val folderKey: ByteArray = if (folder.parent == null) { + decrypt(folder.folderKey, appKey) + } else { + val sharedFolderKey = getSharedFolderKey(folders, response.folders, folder.parent) ?: throw SecretsManagerException("Folder data inconsistent - unable to locate shared folder") + decrypt(folder.folderKey, sharedFolderKey, true) + } + val decryptedData = decrypt(folder.data!!, folderKey, true) + val folderNameJson = bytesToString(decryptedData) + val folderName = nonStrictJson.decodeFromString(folderNameJson) + folders.add(KeeperFolder(folderKey, folder.folderUid, folder.parent, folderName.name)) + } catch (e: Exception) { + if (options.loggingEnabled) { + // Same shape as the skip diagnostics in fetchAndDecryptSecrets. The class name + // matters because the common causes (a null data field, a tag mismatch) carry no + // message, which would otherwise log "skipped due to error: null". + System.err.println("Folder ${folder.folderUid} skipped due to error: ${e.javaClass.simpleName}, ${e.message}") + } } - val decryptedData = decrypt(folder.data!!, folderKey, true) - val folderNameJson = bytesToString(decryptedData) - val folderName = nonStrictJson.decodeFromString(folderNameJson) - folders.add(KeeperFolder(folderKey, folder.folderUid, folder.parent, folderName.name)) } return folders } private fun getSharedFolderKey(folders: List, responseFolders: List, parent: String): ByteArray? { + val visited = HashSet() var currentParent = parent - while (true) { + while (visited.add(currentParent)) { val parentFolder = responseFolders.find { x -> x.folderUid == currentParent } ?: return null currentParent = parentFolder.parent ?: return folders.find { it.folderUid == parentFolder.folderUid }?.folderKey } + throw SecretsManagerException("Folder data inconsistent - parent cycle detected at folder UID $currentParent") } private fun prepareGetPayload( @@ -1655,7 +1751,15 @@ fun cachingPostFunction(url: String, transmissionKey: TransmissionKey, payload: } response } catch (e: Exception) { - val cachedData = getCachedValue() + val cachedData = try { + getCachedValue() + } catch (cacheE: Exception) { + throw SecretsManagerException("KSM request failed and no cached data is available: ${e.message}", e) + } + System.err.println("WARNING: KSM request failed (${e.message}); serving stale cached secrets. " + + "Note: cachingPostFunction cannot use SecretsManagerOptions.proxyUrl; ambient env proxies " + + "(HTTPS_PROXY, https.proxyHost) still apply. To use an explicit proxyUrl, use the default " + + "postFunction instead of cachingPostFunction.") val cachedTransmissionKey = cachedData.copyOfRange(0, 32) transmissionKey.key = cachedTransmissionKey val data = cachedData.copyOfRange(32, cachedData.size) @@ -1663,27 +1767,51 @@ fun cachingPostFunction(url: String, transmissionKey: TransmissionKey, payload: } } +// Kept as an explicit overload rather than a default argument on the function below: this is the +// arity Java callers and cachingPostFunction already compile against. No @JvmOverloads, because +// there are no default arguments here for it to generate overloads from. fun postFunction( url: String, transmissionKey: TransmissionKey, payload: EncryptedPayload, allowUnverifiedCertificate: Boolean +): KeeperHttpResponse = postFunction(url, transmissionKey, payload, allowUnverifiedCertificate, null) + +fun postFunction( + url: String, + transmissionKey: TransmissionKey, + payload: EncryptedPayload, + allowUnverifiedCertificate: Boolean, + proxyUrl: String? = null, + connectTimeoutMillis: Int = DEFAULT_CONNECT_TIMEOUT_MS, + readTimeoutMillis: Int = DEFAULT_READ_TIMEOUT_MS, ): KeeperHttpResponse { var statusCode: Int var data: ByteArray - val connection = URI.create(url).toURL().openConnection() as HttpsURLConnection // KSM-855 + val connection = openProxiedConnection(url, proxyUrl, allowUnverifiedCertificate) try { - if (allowUnverifiedCertificate) { - connection.sslSocketFactory = trustAllSocketFactory() - } + connection.connectTimeout = connectTimeoutMillis + connection.readTimeout = readTimeoutMillis connection.requestMethod = "POST" connection.doOutput = true connection.setRequestProperty("PublicKeyId", transmissionKey.publicKeyId.toString()) connection.setRequestProperty("TransmissionKey", bytesToBase64(transmissionKey.encryptedKey)) connection.setRequestProperty("Authorization", "Signature ${bytesToBase64(payload.signature)}") - connection.outputStream.write(payload.payload) - connection.outputStream.flush() - statusCode = connection.responseCode + try { + connection.outputStream.write(payload.payload) + connection.outputStream.flush() + } catch (e: IOException) { + // For tunneled HTTPS connections the JDK throws here when the CONNECT tunnel fails. + if (e.message?.contains("407") == true) { + val resolved = resolveProxy(proxyUrl, url) + if (resolved != null && resolved.hasCredentials) { + val isAmbientCredentials = !resolved.isExplicit + throw SecretsManagerException(proxyAuthFailureMessage(e.message, isAmbientCredentials), e) + } + } + throw e + } + statusCode = connection.checkedResponseCode(proxyUrl, url) data = when { connection.errorStream != null -> connection.errorStream.readBytes() else -> connection.inputStream.readBytes() @@ -1752,7 +1880,7 @@ private inline fun encryptAndSignPayload( @ExperimentalSerializationApi // Returns the throttle retry_after (>= 0) when [body] is a backend throttle error // (result_code/error == "throttled"), otherwise null so the caller falls through to normal -// error handling. Non-JSON / non-object bodies return null. (KSM-876 / KSM-878) +// error handling. Non-JSON / non-object bodies return null. internal fun parseThrottle(body: String): Double? { val obj = try { nonStrictJson.parseToJsonElement(body).jsonObject @@ -1774,7 +1902,7 @@ internal fun throttleDelayMillis(attempt: Int, retryAfter: Double, jitter: Doubl return (sec * 1000).toLong() } -// Random jitter multiplier in [0, 0.25) — one-sided so a delay is only ever padded, never +// Random jitter multiplier in [0, 0.25); one-sided so a delay is only ever padded, never // undercuts the server-requested (or exponential-backoff) floor. Kept separate so unit tests // exercise throttleDelayMillis with a pinned jitter instead. internal fun throttleJitter(): Double = Random.nextDouble(0.0, 0.25) @@ -1789,17 +1917,18 @@ private inline fun postQuery( val url = "https://${hostName}/api/rest/sm/v1/${path}" val throttleSleep = options.throttleSleepMillis ?: { ms -> Thread.sleep(ms) } var throttleAttempt = 0 + var keyRotationAttempt = 0 while (true) { val transmissionKey = generateTransmissionKey(options.storage) val encryptedPayload = encryptAndSignPayload(options.storage, transmissionKey, payload) val response = if (options.queryFunction == null) { - postFunction(url, transmissionKey, encryptedPayload, options.allowUnverifiedCertificate) + postFunction(url, transmissionKey, encryptedPayload, options.allowUnverifiedCertificate, options.proxyUrl, options.connectTimeoutMillis, options.readTimeoutMillis) } else { options.queryFunction.invoke(url, transmissionKey, encryptedPayload) } if (response.statusCode != HTTP_OK) { val errorMessage = String(response.data) - // Throttle retry with exponential backoff + jitter (KSM-876 / KSM-878). Checked before + // Throttle retry with exponential backoff + jitter. Checked before // key-rotation so that path (incl. the IL5 custom-key suppression) is untouched, and // gated on the 403 status so a non-403 response carrying a {"error":"throttled"} body // is not mistaken for a throttle and retried. @@ -1829,6 +1958,18 @@ private inline fun postQuery( val currentKeyId = options.storage.getString(KEY_SERVER_PUBLIC_KEY_ID) throw SecretsManagerException("Server rejected the custom server public key (id $currentKeyId). The server suggested key id ${error.key_id}. Please update your IL5 KSM configuration.") } + if (!keeperPublicKeys.containsKey(error.key_id)) { + val ids = keeperPublicKeys.keys.sorted() + throw SecretsManagerException( + "Server suggested unsupported key id ${error.key_id}; this SDK version supports key ids ${ids.first()}-${ids.last()}" + ) + } + if (keyRotationAttempt >= MAX_KEY_ROTATION_RETRIES) { + throw SecretsManagerException( + "Server key rotation exhausted $MAX_KEY_ROTATION_RETRIES retries; key id ${transmissionKey.publicKeyId} was not accepted" + ) + } + keyRotationAttempt++ options.storage.saveString(KEY_SERVER_PUBLIC_KEY_ID, error.key_id.toString()) continue } @@ -1841,36 +1982,6 @@ private inline fun postQuery( } } -private fun trustAllSocketFactory(): SSLSocketFactory { - val trustAllCerts: Array = arrayOf( - object : X509TrustManager { - private val AcceptedIssuers = arrayOf() - override fun checkClientTrusted( - certs: Array?, authType: String? - ) { - } - - override fun checkServerTrusted( - certs: Array?, authType: String? - ) { - } - - override fun getAcceptedIssuers(): Array { - return AcceptedIssuers - } - } - ) - val sslContext = SSLContext.getInstance("TLS") - try { - sslContext.init(null, trustAllCerts, SecureRandom()) - } catch (e: NoSuchAlgorithmException) { - e.printStackTrace() - } catch (e: KeyManagementException) { - e.printStackTrace() - } - return sslContext.socketFactory -} - internal object TestStubs { lateinit var transmissionKeyStub: () -> ByteArray diff --git a/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/SecretsManagerExceptions.kt b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/SecretsManagerExceptions.kt index 5ebbd94df..875c5b475 100644 --- a/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/SecretsManagerExceptions.kt +++ b/sdk/java/core/src/main/kotlin/com/keepersecurity/secretsManager/core/SecretsManagerExceptions.kt @@ -2,7 +2,24 @@ package com.keepersecurity.secretsManager.core -open class SecretsManagerException(message: String): Exception(message) +/** + * Base type for errors raised by this SDK. + * + * [cause] is optional so a wrapped failure can keep the underlying exception and its stack trace. + * `@JvmOverloads` keeps the single-argument form Java callers and subclasses already compile against. + */ +open class SecretsManagerException @JvmOverloads constructor( + message: String, + cause: Throwable? = null +): Exception(message, cause) { + companion object { + // Pins the SUID to the value computed for the original single-constructor shape so jars + // built before this change can deserialize exceptions from jars built after it (e.g. Jenkins + // controller/agent remoting). private const val generates private static final in bytecode, + // which is the conventional form recognized by ObjectStreamClass. + private const val serialVersionUID: Long = 5401507264959279624L + } +} internal class SecureRandomException(message: String): SecretsManagerException(message) internal class SecureRandomSlowGenerationException(message: String): SecretsManagerException(message) diff --git a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/CryptoUtilsTest.kt b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/CryptoUtilsTest.kt index 778a4e6d3..edf9913de 100644 --- a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/CryptoUtilsTest.kt +++ b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/CryptoUtilsTest.kt @@ -165,6 +165,38 @@ internal class CryptoUtilsTest { assertTrue { password.filter { "\"!@#$%()+;<>=?[\\]{}^.,".contains(it) }.length == password.length } } + @Test + fun testGeneratePasswordMixedCategoriesAreShuffled() { + // Every other testGeneratePassword case asks for a single category, so all 32 characters + // come from one charset and the final shuffle cannot be observed. randomSample() emits + // characters grouped by category, so a mixed request is the only way to tell a real + // shuffle from no shuffle at all: without one, position 0 is always lowercase. + // + // Scope: this pins down that the shuffle happens and that it preserves the requested + // count per category. It cannot distinguish a CSPRNG from a weak PRNG, since both + // produce a uniform permutation; that the shuffle draws from SecureRandom is enforced + // by review of generatePassword, not by this test. + val charsets = listOf(AsciiLowercase, AsciiUppercase, AsciiDigits, AsciiSpecialCharacters) + val categoryOfFirstChar = mutableSetOf() + repeat(200) { + val password = generatePassword(32, 8, 8, 8, 8) + assertEquals(32, password.length) + // The shuffle must preserve the requested count for each category. + charsets.forEachIndexed { category, charset -> + assertEquals( + 8, password.count { it in charset }, + "category $category count changed. Password: $password" + ) + } + categoryOfFirstChar.add(charsets.indexOfFirst { password[0] in it }) + } + assertTrue( + categoryOfFirstChar.size > 1, + "position 0 only ever held category $categoryOfFirstChar across 200 passwords, " + + "so the characters are not being shuffled across categories" + ) + } + @Test fun testWebSafe64FromBytes() { val urlSafeRegex = "^[a-zA-Z0-9_-]*\$".toRegex() diff --git a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/IL5EdgeCaseTest.kt b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/IL5EdgeCaseTest.kt index 9fdf95930..b9a54f8c6 100644 --- a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/IL5EdgeCaseTest.kt +++ b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/IL5EdgeCaseTest.kt @@ -17,7 +17,7 @@ internal class IL5EdgeCaseTest { @Test fun localConfigStoragePersistsServerPublicKey() { - // Delete immediately — we only want the unique path, not an empty file that would fail JSON parse. + // Delete immediately; we only want the unique path, not an empty file that would fail JSON parse. val configFile = File.createTempFile("ksm-test-", ".json").also { it.delete() } try { initializeStorage(LocalConfigStorage(configFile.absolutePath), "IL5:FAKE_CLIENT_KEY:20:$fakeServerPublicKey") diff --git a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/KeeperRecordLinkTest.kt b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/KeeperRecordLinkTest.kt index a7f5fcf80..9b9113049 100644 --- a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/KeeperRecordLinkTest.kt +++ b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/KeeperRecordLinkTest.kt @@ -10,8 +10,8 @@ import kotlin.test.assertTrue /** * Unit tests for the [KeeperRecordLink] typed accessor layer. * - * Mirrors the Python reference suite `sdk/python/core/tests/record_link_test.py` (KSM-992, PR #1036) - * so the Java accessors match the live-verified backend payload shapes: permission booleans with an + * Mirrors the Python reference suite `sdk/python/core/tests/record_link_test.py` so the Java + * accessors match the live-verified backend payload shapes: permission booleans with an * `allowedSettings` fallback (top-level wins), the new credential/meta accessors, lossless * `getLinkData()`, and the encrypted ai/jit settings. */ @@ -168,7 +168,7 @@ class KeeperRecordLinkTest { path = "meta" ) - // Permission booleans are absent at the top level — read via the allowedSettings fallback. + // Permission booleans are absent at the top level; read via the allowedSettings fallback. assertTrue(meta.allowsRotation()) assertTrue(meta.allowsConnections()) assertTrue(meta.allowsPortForwards()) diff --git a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/ProxyTest.kt b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/ProxyTest.kt new file mode 100644 index 000000000..460c92738 --- /dev/null +++ b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/ProxyTest.kt @@ -0,0 +1,323 @@ +package com.keepersecurity.secretsManager.core + +import java.net.Authenticator +import java.net.Proxy +import kotlin.test.Test +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +internal class ProxyTest { + + private class FakeProxyEnvironment( + private val envVars: Map = emptyMap(), + private val properties: Map = emptyMap() + ) : ProxyEnvironment { + override fun env(name: String): String? = envVars[name] + override fun property(name: String): String? = properties[name] + } + + private val target = "https://vault.keepersecurity.com/api/rest/sm/v1/get_secret" + + private val tunnelingSchemesProperty = "jdk.http.auth.tunneling.disabledSchemes" + private var tunnelingSchemesBefore: String? = null + + @BeforeTest + fun captureTunnelingSchemes() { + tunnelingSchemesBefore = System.getProperty(tunnelingSchemesProperty) + } + + // Registering a proxy credential mutates three pieces of process-wide state: the default + // Authenticator, the ProxyAuthenticator credential store, and the JDK's tunneling-schemes + // property. Gradle runs the whole suite in one JVM, so all three have to be put back or they + // stay visible to every test that follows, including the weakened tunneling default. + @AfterTest + fun restoreGlobalProxyState() { + Authenticator.setDefault(null) + ProxyAuthenticator.reset() + val before = tunnelingSchemesBefore + if (before == null) System.clearProperty(tunnelingSchemesProperty) + else System.setProperty(tunnelingSchemesProperty, before) + } + + @Test + fun explicitProxyUrlWithCredentialsIsParsed() { + val resolved = resolveProxy("http://user:pass@proxy.local:8080", target, FakeProxyEnvironment()) + assertEquals(Proxy.Type.HTTP, resolved!!.proxy.type()) + val address = resolved.proxy.address() as java.net.InetSocketAddress + assertEquals("proxy.local", address.hostString) + assertEquals(8080, address.port) + assertEquals("user", resolved.username) + assertEquals("pass", resolved.password) + } + + @Test + fun precedenceIsExplicitThenSystemPropsThenEnv() { + val env = FakeProxyEnvironment( + envVars = mapOf("HTTPS_PROXY" to "http://env.local:9999"), + properties = mapOf("https.proxyHost" to "sys.local", "https.proxyPort" to "3128") + ) + assertEquals("explicit.local", host(resolveProxy("http://explicit.local:1111", target, env))) + assertEquals("sys.local", host(resolveProxy(null, target, env))) + + val envOnly = FakeProxyEnvironment(envVars = mapOf("HTTPS_PROXY" to "http://env.local:9999")) + assertEquals("env.local", host(resolveProxy(null, target, envOnly))) + } + + @Test + fun noProxyExclusionReturnsNull() { + val exactMatch = FakeProxyEnvironment( + envVars = mapOf("HTTPS_PROXY" to "http://proxy.local:8080", "NO_PROXY" to "vault.keepersecurity.com") + ) + assertNull(resolveProxy(null, target, exactMatch)) + + val suffixMatch = FakeProxyEnvironment( + envVars = mapOf("HTTPS_PROXY" to "http://proxy.local:8080", "NO_PROXY" to ".keepersecurity.com") + ) + assertNull(resolveProxy(null, "https://ksm.keepersecurity.com/path", suffixMatch)) + } + + @Test + fun noProxyConfiguredReturnsNull() { + assertNull(resolveProxy(null, target, FakeProxyEnvironment())) + } + + @Test + fun explicitProxyUrlIgnoresNoProxy() { + // NO_PROXY must not override an explicit proxyUrl — the caller opted in deliberately. + val env = FakeProxyEnvironment( + envVars = mapOf( + "NO_PROXY" to "vault.keepersecurity.com", + "HTTPS_PROXY" to "http://ambient.example.com:9999" + ) + ) + val resolved = resolveProxy("http://explicit.example.com:1234", target, env) + assertEquals("explicit.example.com", host(resolved)) + } + + @Test + fun unparsableExplicitProxyUrlFailsClosed() { + // An explicit proxyUrl that cannot be parsed must throw rather than fall through to + // a direct connection — the caller intended to use a proxy. + assertFailsWith { + resolveProxy(":::not-a-url:::", target, FakeProxyEnvironment()) + } + } + + @Test + fun unparsableExplicitProxyUrlRedactsCredentials() { + // Reserved-char passwords (containing '@', spaces) prevent URI parsing, so redactProxyUrl + // must strip credentials textually rather than relying on URI.getUserInfo(). + val e = assertFailsWith { + resolveProxy("http://user:p@ssword@proxy.corp:8080", target, FakeProxyEnvironment()) + } + assertFalse(e.message!!.contains("p@ssword"), "Password must be redacted in exception message") + assertTrue(e.message!!.contains("user:***"), "Redacted form must appear in exception message") + } + + @Test + fun redactProxyUrlStripsCredentialsTextually() { + assertEquals("http://user:***@proxy.corp:8080", redactProxyUrl("http://user:p@ssword@proxy.corp:8080")) + assertEquals("http://user:***@proxy.corp:8080", redactProxyUrl("http://user:pass word@proxy.corp:8080")) + assertEquals("http://user:***@proxy.corp:8080", redactProxyUrl("http://user:simplepass@proxy.corp:8080")) + assertEquals("http://user:***@proxy.corp:8080", redactProxyUrl("http://user:a/b@proxy.corp:8080")) + assertEquals("user:***@proxy.corp:8080", redactProxyUrl("user:pa ss/word@proxy.corp:8080")) + assertEquals("http://proxy.corp:8080", redactProxyUrl("http://proxy.corp:8080")) + assertEquals("notaurl", redactProxyUrl("notaurl")) + } + + @Test + fun explicitProxyIsMarkedAsExplicit() { + val resolved = resolveProxy("http://proxy.example.com:8080", target, FakeProxyEnvironment()) + assertTrue(resolved!!.isExplicit) + } + + @Test + fun ambientProxyIsNotMarkedAsExplicit() { + val env = FakeProxyEnvironment(envVars = mapOf("HTTPS_PROXY" to "http://proxy.example.com:8080")) + val resolved = resolveProxy(null, target, env) + assertFalse(resolved!!.isExplicit) + } + + @Test + fun httpsSchemeProxyIsRejectedWhenExplicit() { + assertFailsWith { + resolveProxy("https://proxy.corp:443", target, FakeProxyEnvironment()) + } + } + + @Test + fun httpsSchemeAmbientProxyDegradesToNull() { + // An https:// value in an ambient env var degrades gracefully instead of throwing. + val env = FakeProxyEnvironment(envVars = mapOf("HTTPS_PROXY" to "https://proxy.corp:443")) + assertNull(resolveProxy(null, target, env)) + } + + @Test + fun schemeDefaultPortIsHttpElse80() { + // http:// without a port should default to 80. + val http = resolveProxy("http://proxy.corp", target, FakeProxyEnvironment()) + assertEquals(80, (http!!.proxy.address() as java.net.InetSocketAddress).port) + } + + @Test + fun outOfRangePortForExplicitProxyFailsClosed() { + assertFailsWith { + resolveProxy("http://proxy.corp:99999", target, FakeProxyEnvironment()) + } + } + + @Test + fun outOfRangePortForAmbientProxyDegradesToNull() { + val env = FakeProxyEnvironment(envVars = mapOf("HTTPS_PROXY" to "http://proxy.corp:99999")) + assertNull(resolveProxy(null, target, env)) + } + + @Test + fun partialUserinfoExplicitProxyThrows() { + // Every incomplete credential shape fails as a config error. The empty-password form is + // the one a templated proxy URL produces when its password variable goes unset, and it + // has to fail here rather than reaching the proxy and returning an opaque 407. + for (url in listOf( + "http://user@proxy.corp:8080", + "http://:secret@proxy.corp:8080", + "http://user:@proxy.corp:8080" + )) { + assertFailsWith("expected $url to be rejected") { + resolveProxy(url, target, FakeProxyEnvironment()) + } + } + } + + @Test + fun emptyPasswordAmbientProxyIsNotTreatedAsCredentials() { + // Ambient config never throws, but an empty password must not count as a credential: + // hasCredentials drives both the Authenticator registration and the 407 message branch. + val env = FakeProxyEnvironment(envVars = mapOf("HTTPS_PROXY" to "http://user:@proxy.corp:8080")) + val resolved = resolveProxy(null, target, env) + assertNotNull(resolved) + assertEquals("proxy.corp", host(resolved)) + assertNull(resolved.password) + assertFalse(resolved.hasCredentials) + } + + @Test + fun blankEnvVarDoesNotMaskLowerPriorityVar() { + // A set-but-empty HTTPS_PROXY must not hide a valid https_proxy (case variant). + val env = FakeProxyEnvironment( + envVars = mapOf("HTTPS_PROXY" to "", "https_proxy" to "http://fallback.corp:3128") + ) + assertEquals("fallback.corp", host(resolveProxy(null, target, env))) + } + + @Test + fun blankSystemPropertyHostDegradesToNull() { + // An empty https.proxyHost must not produce an unparseable ":443" candidate. + val env = FakeProxyEnvironment( + properties = mapOf("https.proxyHost" to "") + ) + assertNull(resolveProxy(null, target, env)) + } + + @Test + fun httpProxyHostPropertyIsIgnoredForHttpsTraffic() { + // The JDK's ProxySelector never applies http.proxyHost to HTTPS URLs. + // KSM connects only to HTTPS endpoints, so http.proxyHost must have no effect. + val env = FakeProxyEnvironment( + properties = mapOf("http.proxyHost" to "legacyproxy.corp", "http.proxyPort" to "3128") + ) + assertNull(resolveProxy(null, target, env)) + } + + @Test + fun unparsableAmbientProxyDoesNotForceDirectConnection() { + // A proxy candidate that fails parseProxy (e.g. underscore hostname rejected by URI) must + // fall through without forcing Proxy.NO_PROXY — isExcluded() is the only exclusion signal. + // We verify this by checking that isExcluded returns false for a non-excluded host even + // when systemPropertyProxy produces a candidate that is then rejected. + val env = FakeProxyEnvironment( + properties = mapOf("https.proxyHost" to "my_proxy", "https.proxyPort" to "3128") + ) + // resolveProxy returns null when the ambient candidate can't be parsed AND the host is not + // excluded — the caller (openProxiedConnection) must fall back to the system ProxySelector, + // not force Proxy.NO_PROXY. + val resolved = resolveProxy(null, target, env) + assertNull(resolved) + // The host is NOT excluded, so isExcluded must return false. + assertFalse(isExcluded("vault.keepersecurity.com", env)) + } + + @Test + fun isExcludedReturnsTrueForNoProxyMatch() { + val env = FakeProxyEnvironment( + envVars = mapOf("HTTPS_PROXY" to "http://proxy.local:8080", "NO_PROXY" to "vault.keepersecurity.com") + ) + assertTrue(isExcluded("vault.keepersecurity.com", env)) + } + + @Test + fun proxyAuthenticatorAnswersOnlyForRegisteredProxy() { + ProxyAuthenticator.register("proxy.local", 8080, "u", "p") + + val proxyMatch = Authenticator.requestPasswordAuthentication( + "proxy.local", null, 8080, "http", "", "basic", null, Authenticator.RequestorType.PROXY + ) + assertEquals("u", proxyMatch?.userName) + + val serverChallenge = Authenticator.requestPasswordAuthentication( + "proxy.local", null, 8080, "https", "", "basic", null, Authenticator.RequestorType.SERVER + ) + assertNull(serverChallenge) + + val otherProxy = Authenticator.requestPasswordAuthentication( + "other.local", null, 8080, "http", "", "basic", null, Authenticator.RequestorType.PROXY + ) + assertNull(otherProxy) + } + + @Test + fun proxyAuthenticatorResetDropsRegisteredCredentials() { + // Guards the teardown above: if reset() ever stops emptying the store, a registered + // credential outlives its test and the leak this test exists to prevent comes back. + ProxyAuthenticator.register("proxy.local", 8080, "u", "p") + ProxyAuthenticator.reset() + + val afterReset = Authenticator.requestPasswordAuthentication( + "proxy.local", null, 8080, "http", "", "basic", null, Authenticator.RequestorType.PROXY + ) + assertNull(afterReset) + } + + @Test + fun proxyAuthFailureMessageBranchContentIsCorrect() { + // The ambient-credentials branch must only fire when username is non-null. + // An unauthenticated ambient proxy (no credentials in env var) must use the standard path. + val standardMsg = proxyAuthFailureMessage("407", isAmbientCredentials = false) + val ambientMsg = proxyAuthFailureMessage("407", isAmbientCredentials = true) + assertTrue(standardMsg.contains("proxyUrl"), "Standard message must mention proxyUrl") + assertTrue(ambientMsg.contains("SecretsManagerOptions"), "Ambient message must point to SecretsManagerOptions") + assertFalse(ambientMsg.contains("downloadFile"), "Ambient message must not mention downloadFile as a proxyUrl carrier") + assertTrue(ambientMsg.contains("proxyUrl field"), "Ambient message must mention proxyUrl field in options") + } + + @Test + fun redactProxyUrlHandlesSchemeLessUrl() { + assertEquals("user:***@proxy.corp:8080", redactProxyUrl("user:p@ssword@proxy.corp:8080")) + } + + @Test + fun secretsManagerExceptionSerialVersionUidMatchesReleasedJars() { + assertEquals( + 5401507264959279624L, + java.io.ObjectStreamClass.lookup(SecretsManagerException::class.java).serialVersionUID + ) + } + + private fun host(resolved: ResolvedProxy?): String = + (resolved!!.proxy.address() as java.net.InetSocketAddress).hostString +} diff --git a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/RecordDataPamFieldsTest.kt b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/RecordDataPamFieldsTest.kt index e6a363fb9..84dba357e 100644 --- a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/RecordDataPamFieldsTest.kt +++ b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/RecordDataPamFieldsTest.kt @@ -7,7 +7,7 @@ import kotlinx.serialization.json.Json import kotlin.test.* /** - * Test suite for PAM connection settings fields added in KSM-738 (VAUL-7662). + * Test suite for PAM connection settings fields. * Tests serialization/deserialization of new fields in: * - PamRbiConnection (6 new fields: audio/clipboard controls) * - PamSettingsPortForward (2 new fields: local port configuration) diff --git a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/SecretsManagerTest.kt b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/SecretsManagerTest.kt index 200a1902b..5bebb26a4 100644 --- a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/SecretsManagerTest.kt +++ b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/SecretsManagerTest.kt @@ -127,7 +127,7 @@ internal class SecretsManagerTest { @Test fun testIL5OttFourSegmentParsing() { - // KSM-902: 4-segment OTT IL5:clientKey:keyId:serverPublicKey stores key and ID in storage + // 4-segment OTT IL5:clientKey:keyId:serverPublicKey stores key and ID in storage val fakeServerPublicKey = "BK9w6TZFxE6nFNbMfIpULCup2a8xc6w2tUTABjxny7yFmxW0dAEojwC6j6zb5nTlmb1dAx8nwo3qF7RPYGmloRM" val storage = InMemoryStorage() initializeStorage(storage, "IL5:FAKE_CLIENT_KEY:20:$fakeServerPublicKey") @@ -149,7 +149,7 @@ internal class SecretsManagerTest { @Test fun testIL5ConstructorParamPersistsToStorage() { - // KSM-902: serverPublicKey constructor param is saved to storage so generateTransmissionKey can use it + // serverPublicKey constructor param is saved to storage so generateTransmissionKey can use it val fakeServerPublicKey = "BK9w6TZFxE6nFNbMfIpULCup2a8xc6w2tUTABjxny7yFmxW0dAEojwC6j6zb5nTlmb1dAx8nwo3qF7RPYGmloRM" val storage = InMemoryStorage() SecretsManagerOptions(storage, serverPublicKey = fakeServerPublicKey) @@ -158,7 +158,7 @@ internal class SecretsManagerTest { @Test fun testIL5ConfigFieldOverridesEmbeddedTable() { - // KSM-902: KEY_SERVER_PUBLIC_KEY in storage must be used instead of the embedded key table. + // KEY_SERVER_PUBLIC_KEY in storage must be used instead of the embedded key table. // Key ID 999 is not in the embedded table. With a custom key in storage, generateTransmissionKey // must use the storage key. If it falls through to the table, it throws // "Key number 999 is not supported" and the post function is never called. @@ -180,9 +180,162 @@ internal class SecretsManagerTest { assertEquals(999, capturedKeyId, "generateTransmissionKey must use storage key (ID 999) not the embedded table") } + @Test + fun testServerKeyIdOutOfRangeIsRejected() { + // Server-supplied key_id outside the known range must be rejected before storage is written. + val storage = InMemoryStorage() + initializeStorage(storage, "fake.keepersecurity.com:FAKE_CLIENT_KEY") + TestStubs.transmissionKeyStub = { ByteArray(32) } + val testPostFunction: (String, TransmissionKey, EncryptedPayload) -> KeeperHttpResponse = { _, _, _ -> + KeeperHttpResponse(400, """{"error":"key","key_id":999}""".toByteArray()) + } + val options = SecretsManagerOptions(storage, testPostFunction) + val ex = assertFailsWith { + getSecrets(options) + } + assertTrue( + ex.message?.contains("unsupported key id 999") == true, + "Exception must come from the key-table guard and name the rejected id. Got: ${ex.message}" + ) + assertNull( + storage.getString(KEY_SERVER_PUBLIC_KEY_ID), + "Storage must not be poisoned with key_id=999. Got: ${storage.getString(KEY_SERVER_PUBLIC_KEY_ID)}" + ) + } + + @Test + fun testCustomServerKeyGuardTakesPriorityOverKeyTable() { + // When a custom server key is configured, a server key-rotation hint must reach + // the custom-key branch and throw the actionable error before the key-table guard. + val storage = InMemoryStorage() + initializeStorage(storage, "fake.keepersecurity.com:FAKE_CLIENT_KEY") + // Use a real EC public key so webSafe64ToBytes succeeds in generateTransmissionKey. + // The value here is keeper public key #7 (from the embedded table). + storage.saveString(KEY_SERVER_PUBLIC_KEY, "BK9w6TZFxE6nFNbMfIpULCup2a8xc6w2tUTABjxny7yFmxW0dAEojwC6j6zb5nTlmb1dAx8nwo3qF7RPYGmloRM") + storage.saveString(KEY_SERVER_PUBLIC_KEY_ID, "20") + TestStubs.transmissionKeyStub = { ByteArray(32) } + val testPostFunction: (String, TransmissionKey, EncryptedPayload) -> KeeperHttpResponse = { _, _, _ -> + KeeperHttpResponse(400, """{"error":"key","key_id":999}""".toByteArray()) + } + val options = SecretsManagerOptions(storage, testPostFunction) + val ex = assertFailsWith { + getSecrets(options) + } + assertTrue( + ex.message?.contains("custom server public key") == true, + "Custom-key path must fire before the key-table guard. Got: ${ex.message}" + ) + } + + @Test + fun testServerKeyRotationRetryCap() { + // When the server keeps returning an in-table key_id, the loop must stop after MAX_KEY_ROTATION_RETRIES. + val storage = InMemoryStorage() + initializeStorage(storage, "fake.keepersecurity.com:FAKE_CLIENT_KEY") + TestStubs.transmissionKeyStub = { ByteArray(32) } + val callCount = intArrayOf(0) + val testPostFunction: (String, TransmissionKey, EncryptedPayload) -> KeeperHttpResponse = { _, _, _ -> + callCount[0]++ + KeeperHttpResponse(400, """{"error":"key","key_id":10}""".toByteArray()) + } + val options = SecretsManagerOptions(storage, testPostFunction) + val ex = assertFailsWith { + getSecrets(options) + } + assertTrue( + ex.message?.contains("key rotation exhausted $MAX_KEY_ROTATION_RETRIES retries") == true, + "Exception must name the retry limit. Got: ${ex.message}" + ) + assertEquals( + MAX_KEY_ROTATION_RETRIES + 1, callCount[0], + "Must make exactly MAX_KEY_ROTATION_RETRIES+1 calls (initial + cap retries)" + ) + } + + @Test + fun testConfigFileAtomicWriteIsOwnerOnly() { + // KSM-1262: the config file must be written via atomic rename and be readable only by the owner. + // The 0600 guarantee is POSIX-only; skip rather than fail on Windows or non-POSIX file systems. + org.junit.Assume.assumeTrue( + "POSIX file permissions not supported on this file system", + java.nio.file.FileSystems.getDefault().supportedFileAttributeViews().contains("posix") + ) + val configFile = File.createTempFile("ksm-test-", ".json").also { it.delete() } + try { + val storage = LocalConfigStorage(configFile.absolutePath) + initializeStorage(storage, "fake.keepersecurity.com:FAKE_CLIENT_KEY") + assertTrue(configFile.exists(), "Config file must exist after initializeStorage") + val perms = java.nio.file.Files.getPosixFilePermissions(configFile.toPath()) + val expected = java.nio.file.attribute.PosixFilePermissions.fromString("rw-------") + assertEquals(expected, perms, "Config file must be owner-read/write only. Got: $perms") + + // Permissions alone do not prove the fix: the previous implementation also ended at + // 0600 by calling chmod after writing the private key through an 0644 handle. What is + // new is that the secrets are never written to the visible path at all. A rename + // installs a different inode, so fileKey() changes across a write; an in-place + // rewrite keeps the same one. + val inodeBefore = java.nio.file.Files.readAttributes( + configFile.toPath(), java.nio.file.attribute.BasicFileAttributes::class.java + ).fileKey() + assertNotNull(inodeBefore, "File system does not expose fileKey(); cannot verify the swap") + storage.saveString(KEY_HOSTNAME, "second.keepersecurity.com") + val inodeAfter = java.nio.file.Files.readAttributes( + configFile.toPath(), java.nio.file.attribute.BasicFileAttributes::class.java + ).fileKey() + assertNotEquals( + inodeBefore, inodeAfter, + "Config was rewritten in place rather than swapped in via rename, so the window " + + "where the file is visible with partial or loosely-permissioned content is open" + ) + assertEquals( + java.nio.file.attribute.PosixFilePermissions.fromString("rw-------"), + java.nio.file.Files.getPosixFilePermissions(configFile.toPath()), + "Permissions must survive the swap" + ) + + // The staging file must not survive a successful write. + val leftovers = configFile.absoluteFile.parentFile + .list { _, name -> name.startsWith("ksm_") && name.endsWith(".tmp") } + ?: emptyArray() + assertTrue(leftovers.isEmpty(), "Staging files were left behind: ${leftovers.toList()}") + } finally { + configFile.delete() + } + } + + @Test + fun testConfigWriteFailureReportsTheUnderlyingCause() { + // The write path translates IOException into SecretsManagerException. It must not claim a + // permissions problem for every failure, and it must keep the original exception as the + // cause so the stack trace survives. + val missingDir = File( + File(System.getProperty("java.io.tmpdir")), + "ksm-absent-${System.nanoTime()}" + ) + assertFalse(missingDir.exists(), "Test precondition: the directory must not exist") + val storage = LocalConfigStorage(File(missingDir, "config.json").absolutePath) + + val ex = assertFailsWith { + storage.saveString(KEY_HOSTNAME, "fake.keepersecurity.com") + } + assertNotNull(ex.cause, "The original IOException must be retained as the cause") + assertTrue( + ex.cause is java.io.IOException, + "Cause must be the file system failure. Got: ${ex.cause?.javaClass?.name}" + ) + assertFalse( + ex.message!!.contains("is not writable"), + "A missing directory must not be reported as a permissions problem. Got: ${ex.message}" + ) + assertTrue( + ex.message!!.contains(missingDir.name), + "The message must name the directory it could not use. Got: ${ex.message}" + ) + } + @Test fun testRecordCreateEmptyCustomSerialized() { - // KSM-823: RecordCreate with no custom fields must include "custom": [] in JSON payload + // RecordCreate with no custom fields must include "custom": [] in JSON payload val recordData = KeeperRecordData( title = "Test Record", type = "login", @@ -195,7 +348,7 @@ internal class SecretsManagerTest { @Test fun testKeeperFileDataMissingLastModified() { - // GH-973 / KSM-854: lastModified entirely absent — must deserialize without throwing + // lastModified is absent. The SDK must deserialize without throwing. val json = """{"title":"test.txt","name":"test.txt","type":"text/plain","size":1024}""" val result = Json.decodeFromString(json) assertEquals(0L, result.lastModified) @@ -212,7 +365,7 @@ internal class SecretsManagerTest { @Test fun testKeeperFileDataFractionalLastModified() { - // Regression guard for KSM-673: fractional lastModified (iOS client format) + // Regression guard for fractional lastModified (iOS client format) val json = """{"title":"test.txt","name":"test.txt","size":1024,"lastModified":1760646182.790214}""" val result = Json.decodeFromString(json) assertEquals(1760646182L, result.lastModified) @@ -220,7 +373,7 @@ internal class SecretsManagerTest { @Test fun testKeeperFileNullUrl() { - // KSM-765: KeeperFile.url must be nullable; server may omit url for files without a download link + // KeeperFile.url must be nullable. The server may omit url for files without a download link. val fileData = KeeperFileData("test.txt", "test.txt", "text/plain", 1024) val file = KeeperFile(ByteArray(32), "uid123", fileData, null, null) assertNull(file.url) @@ -228,7 +381,7 @@ internal class SecretsManagerTest { @Test fun testBase64EmptyStringThrowsTypedException() { - // KSM-985: empty string must throw a typed Keeper exception, not an NPE from inside java.util.Base64 + // Empty string must throw a typed Keeper exception, not an NPE from inside java.util.Base64. val base64Ex = assertFailsWith { base64ToBytes("") } assertTrue(base64Ex.message?.isNotEmpty() == true) val webSafe64Ex = assertFailsWith { webSafe64ToBytes("") } @@ -270,6 +423,338 @@ internal class SecretsManagerTest { assertEquals(folderUid, record.folderUid) } + @Test + fun testIsEditableForwardedToKeeperRecord() { + // The server's per-record write-permission flag must survive decryptRecord() unchanged in + // both directions. Asserting only the true case would pass against a hardcoded value. + val transmissionKey = ByteArray(32) { it.toByte() } + TestStubs.transmissionKeyStub = { transmissionKey } + + val appKey = getRandomBytes(32) + val recordKey = getRandomBytes(32) + val encRecordKey = bytesToBase64(encrypt(recordKey, appKey)) + val encData = bytesToBase64(encrypt(stringToBytes( + """{"title":"Test","type":"login","fields":[],"custom":[]}"""), recordKey)) + + fun response(isEditable: Boolean) = encrypt(stringToBytes( + """{"encryptedAppKey":null,"folders":null,"records":[{"recordUid":"uid1","recordKey":"$encRecordKey","data":"$encData","revision":1,"isEditable":$isEditable,"files":null,"innerFolderUid":null}]}""" + ), transmissionKey) + + val storage = InMemoryStorage() + initializeStorage(storage, "US:FAKE_CLIENT_KEY") + storage.saveBytes(KEY_APP_KEY, appKey) + + // fetchAndDecryptSecrets swallows per-record failures to stderr, so assert the record + // survived before indexing: a decrypt regression must not surface as IndexOutOfBounds. + for (expected in listOf(true, false)) { + val options = SecretsManagerOptions( + storage, + queryFunction = { _, _, _ -> KeeperHttpResponse(200, response(expected)) } + ) + val records = getSecrets(options).records + assertEquals(1, records.size, "record must decrypt for the isEditable:$expected case") + assertEquals(expected, records[0].isEditable, + "isEditable:$expected from server must reach KeeperRecord") + } + } + + @Test + fun testGetFoldersSkipsUndecryptableFolder() { + // A single folder whose key cannot be decrypted must not abort getFolders(). The bad + // folder is skipped and the remaining good folder is still returned. + val transmissionKey = ByteArray(32) { it.toByte() } + TestStubs.transmissionKeyStub = { transmissionKey } + + val appKey = getRandomBytes(32) + val goodFolderKey = getRandomBytes(32) + + val encGoodFolderKey = bytesToBase64(encrypt(goodFolderKey, appKey)) + val encGoodData = bytesToBase64(encrypt(stringToBytes("""{"name":"Good Folder"}"""), goodFolderKey, true)) + val badFolderKey = bytesToBase64(ByteArray(16) { it.toByte() }) + + val responseJson = """{"encryptedAppKey":null,"folders":[{"folderUid":"good-uid","folderKey":"$encGoodFolderKey","data":"$encGoodData","parent":null,"records":null},{"folderUid":"bad-uid","folderKey":"$badFolderKey","data":null,"parent":null,"records":null}],"records":null}""" + val encryptedResponse = encrypt(stringToBytes(responseJson), transmissionKey) + + val storage = InMemoryStorage() + initializeStorage(storage, "US:FAKE_CLIENT_KEY") + storage.saveBytes(KEY_APP_KEY, appKey) + + val options = SecretsManagerOptions(storage, queryFunction = { _, _, _ -> KeeperHttpResponse(200, encryptedResponse) }) + val folders = getFolders(options) + + assertEquals(1, folders.size, "the undecryptable folder must be skipped, not abort the whole call") + assertEquals("good-uid", folders[0].folderUid) + assertEquals("Good Folder", folders[0].name) + } + + @Test(timeout = 5_000) + fun testGetFoldersSkipsFolderParentCycle() { + val transmissionKey = ByteArray(32) { it.toByte() } + TestStubs.transmissionKeyStub = { transmissionKey } + + val appKey = getRandomBytes(32) + val dummyFolderKey = bytesToBase64(getRandomBytes(60)) + + // Two folders whose parent references form a cycle: neither has a null parent, + // so getSharedFolderKey can never reach a root. Both must be skipped. + val responseJson = """{"encryptedAppKey":null,"folders":[{"folderUid":"folder-a","folderKey":"$dummyFolderKey","data":null,"parent":"folder-b","records":null},{"folderUid":"folder-b","folderKey":"$dummyFolderKey","data":null,"parent":"folder-a","records":null}],"records":null}""" + val encryptedResponse = encrypt(stringToBytes(responseJson), transmissionKey) + + val storage = InMemoryStorage() + initializeStorage(storage, "US:FAKE_CLIENT_KEY") + storage.saveBytes(KEY_APP_KEY, appKey) + + val options = SecretsManagerOptions(storage, queryFunction = { _, _, _ -> KeeperHttpResponse(200, encryptedResponse) }) + val folders = getFolders(options) + + assertEquals(0, folders.size, "both cyclic folders must be skipped; the call must complete without hanging") + } + + // Builds a get_folders response with one decryptable folder and one that is not, so the skip + // path in fetchAndDecryptFolders can be driven without a live backend. + private fun undecryptableFolderOptions(loggingEnabled: Boolean): SecretsManagerOptions { + val transmissionKey = ByteArray(32) { it.toByte() } + TestStubs.transmissionKeyStub = { transmissionKey } + val appKey = getRandomBytes(32) + val goodFolderKey = getRandomBytes(32) + val encGoodFolderKey = bytesToBase64(encrypt(goodFolderKey, appKey)) + val encGoodData = bytesToBase64(encrypt(stringToBytes("""{"name":"Good Folder"}"""), goodFolderKey, true)) + val badFolderKey = bytesToBase64(ByteArray(16) { it.toByte() }) + val responseJson = """{"encryptedAppKey":null,"folders":[{"folderUid":"good-uid","folderKey":"$encGoodFolderKey","data":"$encGoodData","parent":null,"records":null},{"folderUid":"bad-uid","folderKey":"$badFolderKey","data":null,"parent":null,"records":null}],"records":null}""" + val encryptedResponse = encrypt(stringToBytes(responseJson), transmissionKey) + val storage = InMemoryStorage() + initializeStorage(storage, "US:FAKE_CLIENT_KEY") + storage.saveBytes(KEY_APP_KEY, appKey) + return SecretsManagerOptions( + storage, + queryFunction = { _, _, _ -> KeeperHttpResponse(200, encryptedResponse) }, + loggingEnabled = loggingEnabled + ) + } + + private fun captureStderr(block: () -> Unit): String { + val original = System.err + val buffer = java.io.ByteArrayOutputStream() + System.setErr(java.io.PrintStream(buffer, true, "UTF-8")) + try { + block() + } finally { + System.setErr(original) + } + return buffer.toString("UTF-8") + } + + @Test + fun testFolderSkipDiagnosticRespectsLoggingEnabled() { + // loggingEnabled gates the throttle and record-decryption diagnostics elsewhere in this + // file. A library that has been told not to log must not write to stderr from the folder + // skip path either. + val stderr = captureStderr { + val folders = getFolders(undecryptableFolderOptions(loggingEnabled = false)) + assertEquals(1, folders.size) + } + assertEquals("", stderr, "loggingEnabled = false must not write to stderr. Got: $stderr") + } + + @Test + fun testFolderSkipDiagnosticNamesTheExceptionType() { + // The message must carry the exception class, not just its message: the causes this skip + // path exists for (a null data field, a GCM tag mismatch) frequently have a null message, + // which on its own logs "skipped due to error: null" and tells an operator nothing. + val stderr = captureStderr { + val folders = getFolders(undecryptableFolderOptions(loggingEnabled = true)) + assertEquals(1, folders.size) + } + // Scope the assertion to the line about this folder and require the class name in the + // position the sibling handlers put it. A bare stderr.contains("Exception") would also + // pass on an exception message that happens to spell the word, or on output bleeding in + // from another test. + val line = stderr.lineSequence().firstOrNull { it.contains("bad-uid") } + assertNotNull(line, "The skipped folder must be named. Got: $stderr") + assertTrue( + Regex("""skipped due to error: \w*(Exception|Error)\w*, """).containsMatchIn(line), + "The exception class must directly follow the prefix so a null message is still " + + "diagnosable. Got: $line" + ) + } + + @Test + fun testDeleteFolderDecodesFoldersKeyAndSurfacesPerItemFailure() { + // The backend keys a delete_folder response under "folders", not "records". Decoding + // this response into the records-keyed type throws MissingFieldException instead of + // returning a value. + val stderr = captureStderr { + val response = deleteFolder(deleteFolderOptions(loggingEnabled = true), listOf("good-uid", "denied-uid")) + assertEquals(2, response.folders.size) + val denied = response.folders.first { it.folderUid == "denied-uid" } + assertEquals("access_denied", denied.responseCode) + assertEquals("User does not have permission to delete this folder", denied.errorMessage) + assertEquals("ok", response.folders.first { it.folderUid == "good-uid" }.responseCode) + } + val line = stderr.lineSequence().firstOrNull { it.contains("denied-uid") } + assertNotNull(line, "The failing folder must be named. Got: $stderr") + assertTrue(line.contains("access_denied")) + } + + @Test + fun testDeleteFolderDoesNotLogWhenLoggingDisabled() { + val stderr = captureStderr { + deleteFolder(deleteFolderOptions(loggingEnabled = false), listOf("good-uid", "denied-uid")) + } + assertEquals("", stderr, "loggingEnabled = false must not write to stderr. Got: $stderr") + } + + // Builds a delete_folder response with one ok folder and one denied folder, keyed under + // "folders" the way the backend actually sends it. + private fun deleteFolderOptions(loggingEnabled: Boolean): SecretsManagerOptions { + val transmissionKey = ByteArray(32) { it.toByte() } + TestStubs.transmissionKeyStub = { transmissionKey } + val responseJson = """{"folders":[{"folderUid":"good-uid","responseCode":"ok"},{"folderUid":"denied-uid","responseCode":"access_denied","errorMessage":"User does not have permission to delete this folder"}]}""" + val encryptedResponse = encrypt(stringToBytes(responseJson), transmissionKey) + val storage = InMemoryStorage() + initializeStorage(storage, "US:FAKE_CLIENT_KEY") + return SecretsManagerOptions( + storage, + queryFunction = { _, _, _ -> KeeperHttpResponse(200, encryptedResponse) }, + loggingEnabled = loggingEnabled + ) + } + + @Test + fun testDeleteSecretDecodesRecordsAndSurfacesPerItemFailure() { + val stderr = captureStderr { + val response = deleteSecret(deleteSecretOptions(loggingEnabled = true), listOf("good-uid", "denied-uid")) + assertEquals(2, response.records.size) + val denied = response.records.first { it.recordUid == "denied-uid" } + assertEquals("access_denied", denied.responseCode) + assertEquals("User does not have permission to delete this record", denied.errorMessage) + assertEquals("ok", response.records.first { it.recordUid == "good-uid" }.responseCode) + } + val line = stderr.lineSequence().firstOrNull { it.contains("denied-uid") } + assertNotNull(line, "The failing record must be named. Got: $stderr") + assertTrue(line.contains("access_denied")) + } + + @Test + fun testDeleteSecretDoesNotLogWhenLoggingDisabled() { + val stderr = captureStderr { + deleteSecret(deleteSecretOptions(loggingEnabled = false), listOf("good-uid", "denied-uid")) + } + assertEquals("", stderr, "loggingEnabled = false must not write to stderr. Got: $stderr") + } + + private fun deleteSecretOptions(loggingEnabled: Boolean): SecretsManagerOptions { + val transmissionKey = ByteArray(32) { it.toByte() } + TestStubs.transmissionKeyStub = { transmissionKey } + val responseJson = """{"records":[{"recordUid":"good-uid","responseCode":"ok"},{"recordUid":"denied-uid","responseCode":"access_denied","errorMessage":"User does not have permission to delete this record"}]}""" + val encryptedResponse = encrypt(stringToBytes(responseJson), transmissionKey) + val storage = InMemoryStorage() + initializeStorage(storage, "US:FAKE_CLIENT_KEY") + return SecretsManagerOptions( + storage, + queryFunction = { _, _, _ -> KeeperHttpResponse(200, encryptedResponse) }, + loggingEnabled = loggingEnabled + ) + } + + @Test + fun testGetSecretsSkipsUndecryptableRecordWithoutStderr() { + // A record whose key cannot be decrypted must be skipped without writing to stderr when + // loggingEnabled = false. Verifies the same loggingEnabled gate that fetchAndDecryptFolders + // has always applied (KSM-1081 follow-up). + val transmissionKey = ByteArray(32) { it.toByte() } + TestStubs.transmissionKeyStub = { transmissionKey } + val appKey = getRandomBytes(32) + val goodKey = getRandomBytes(32) + val encGoodKey = bytesToBase64(encrypt(goodKey, appKey)) + val encGoodData = bytesToBase64(encrypt(stringToBytes("""{"title":"Good","type":"login","fields":[],"custom":[]}"""), goodKey)) + val badRecordKey = bytesToBase64(ByteArray(16) { it.toByte() }) + val encBadData = bytesToBase64(encrypt(stringToBytes("""{"title":"Bad","type":"login","fields":[],"custom":[]}"""), getRandomBytes(32))) + val responseJson = """{"encryptedAppKey":null,"folders":null,"records":[{"recordUid":"good-uid","recordKey":"$encGoodKey","data":"$encGoodData","revision":1,"isEditable":true,"files":null,"innerFolderUid":null},{"recordUid":"bad-uid","recordKey":"$badRecordKey","data":"$encBadData","revision":1,"isEditable":true,"files":null,"innerFolderUid":null}]}""" + val encryptedResponse = encrypt(stringToBytes(responseJson), transmissionKey) + val storage = InMemoryStorage() + initializeStorage(storage, "US:FAKE_CLIENT_KEY") + storage.saveBytes(KEY_APP_KEY, appKey) + val options = SecretsManagerOptions(storage, queryFunction = { _, _, _ -> KeeperHttpResponse(200, encryptedResponse) }, loggingEnabled = false) + val stderr = captureStderr { + val secrets = getSecrets(options) + assertEquals(1, secrets.records.size, "undecryptable record must be skipped, not abort the call") + assertEquals("good-uid", secrets.records[0].recordUid) + } + assertEquals("", stderr, "loggingEnabled = false must suppress record-skip stderr. Got: $stderr") + } + + @Test + fun testGetSecretsSkipsUndecryptableFolderKeyWithoutStderr() { + // A folder in the getSecrets response whose key cannot be decrypted must be skipped without + // writing to stderr when loggingEnabled = false (KSM-1081 follow-up). + val transmissionKey = ByteArray(32) { it.toByte() } + TestStubs.transmissionKeyStub = { transmissionKey } + val appKey = getRandomBytes(32) + val badFolderKey = bytesToBase64(ByteArray(16) { it.toByte() }) + val responseJson = """{"encryptedAppKey":null,"folders":[{"folderUid":"bad-folder","folderKey":"$badFolderKey","data":null,"parent":null,"records":[]}],"records":null}""" + val encryptedResponse = encrypt(stringToBytes(responseJson), transmissionKey) + val storage = InMemoryStorage() + initializeStorage(storage, "US:FAKE_CLIENT_KEY") + storage.saveBytes(KEY_APP_KEY, appKey) + val options = SecretsManagerOptions(storage, queryFunction = { _, _, _ -> KeeperHttpResponse(200, encryptedResponse) }, loggingEnabled = false) + val stderr = captureStderr { + val secrets = getSecrets(options) + assertEquals(0, secrets.records.size) + } + assertEquals("", stderr, "loggingEnabled = false must suppress folder-key-skip stderr. Got: $stderr") + } + + @Test + fun testGetSecretsSkipsUndecryptableRecordInFolderWithoutStderr() { + // A record nested inside a folder whose key cannot be decrypted must be skipped without + // writing to stderr when loggingEnabled = false (KSM-1081 follow-up). + val transmissionKey = ByteArray(32) { it.toByte() } + TestStubs.transmissionKeyStub = { transmissionKey } + val appKey = getRandomBytes(32) + val folderKey = getRandomBytes(32) + val encFolderKey = bytesToBase64(encrypt(folderKey, appKey)) + val badRecordKey = bytesToBase64(ByteArray(16) { it.toByte() }) + val encBadData = bytesToBase64(encrypt(stringToBytes("""{"title":"Bad","type":"login","fields":[],"custom":[]}"""), getRandomBytes(32))) + val responseJson = """{"encryptedAppKey":null,"folders":[{"folderUid":"folder-uid","folderKey":"$encFolderKey","data":null,"parent":null,"records":[{"recordUid":"bad-record","recordKey":"$badRecordKey","data":"$encBadData","revision":1,"isEditable":true,"files":null,"innerFolderUid":null}]}],"records":null}""" + val encryptedResponse = encrypt(stringToBytes(responseJson), transmissionKey) + val storage = InMemoryStorage() + initializeStorage(storage, "US:FAKE_CLIENT_KEY") + storage.saveBytes(KEY_APP_KEY, appKey) + val options = SecretsManagerOptions(storage, queryFunction = { _, _, _ -> KeeperHttpResponse(200, encryptedResponse) }, loggingEnabled = false) + val stderr = captureStderr { + val secrets = getSecrets(options) + assertEquals(0, secrets.records.size) + } + assertEquals("", stderr, "loggingEnabled = false must suppress record-in-folder-skip stderr. Got: $stderr") + } + + @Test + fun testGetSecretsSkipsUndecryptableFileWithoutStderr() { + // A file attachment whose key cannot be decrypted must be skipped without writing to stderr + // when loggingEnabled = false. The parent record must still be returned (KSM-1081 follow-up). + val transmissionKey = ByteArray(32) { it.toByte() } + TestStubs.transmissionKeyStub = { transmissionKey } + val appKey = getRandomBytes(32) + val recordKey = getRandomBytes(32) + val encRecordKey = bytesToBase64(encrypt(recordKey, appKey)) + val encData = bytesToBase64(encrypt(stringToBytes("""{"title":"With File","type":"login","fields":[],"custom":[]}"""), recordKey)) + val badFileKey = bytesToBase64(ByteArray(16) { it.toByte() }) + val encFileData = bytesToBase64(encrypt(stringToBytes("""{"name":"file.txt","size":4,"lastModified":0,"type":"text/plain"}"""), getRandomBytes(32))) + val responseJson = """{"encryptedAppKey":null,"folders":null,"records":[{"recordUid":"rec-uid","recordKey":"$encRecordKey","data":"$encData","revision":1,"isEditable":true,"files":[{"fileUid":"file-uid","fileKey":"$badFileKey","data":"$encFileData","url":null,"thumbnailUrl":null}],"innerFolderUid":null}]}""" + val encryptedResponse = encrypt(stringToBytes(responseJson), transmissionKey) + val storage = InMemoryStorage() + initializeStorage(storage, "US:FAKE_CLIENT_KEY") + storage.saveBytes(KEY_APP_KEY, appKey) + val options = SecretsManagerOptions(storage, queryFunction = { _, _, _ -> KeeperHttpResponse(200, encryptedResponse) }, loggingEnabled = false) + val stderr = captureStderr { + val secrets = getSecrets(options) + assertEquals(1, secrets.records.size, "record must survive even when its file is undecryptable") + assertEquals(0, secrets.records[0].files?.size ?: 0, "undecryptable file must be skipped") + } + assertEquals("", stderr, "loggingEnabled = false must suppress file-skip stderr. Got: $stderr") + } + // @Test // uncomment to debug the integration test fun integrationTest() { val trustAllPostFunction: ( diff --git a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/ThrottleTest.kt b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/ThrottleTest.kt index 1dc98b5d7..dd051402e 100644 --- a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/ThrottleTest.kt +++ b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/ThrottleTest.kt @@ -5,7 +5,7 @@ import java.net.HttpURLConnection.HTTP_FORBIDDEN import java.net.HttpURLConnection.HTTP_OK import kotlin.test.* -// Throttle retry with exponential backoff (KSM-876 / KSM-878). Unit tests exercise the internal +// Throttle retry with exponential backoff. Unit tests exercise the internal // helpers; e2e tests drive getSecrets through postQuery with a mocked queryFunction returning // HTTP 403 {"error":"throttled"} responses and a recording throttleSleepMillis so retries never // actually wait. diff --git a/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/TimeoutTest.kt b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/TimeoutTest.kt new file mode 100644 index 000000000..b1eb0b352 --- /dev/null +++ b/sdk/java/core/src/test/kotlin/com/keepersecurity/secretsManager/core/TimeoutTest.kt @@ -0,0 +1,125 @@ +package com.keepersecurity.secretsManager.core + +import kotlinx.serialization.ExperimentalSerializationApi +import java.net.ServerSocket +import java.net.Socket +import java.net.SocketTimeoutException +import java.util.concurrent.ExecutionException +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import java.util.concurrent.TimeoutException +import java.util.concurrent.atomic.AtomicReference +import kotlin.test.* + +// Connect/read timeouts on the built-in HTTP transport. The behavioural test points +// postFunction at a socket that accepts the TCP connection and then goes silent, so the client +// blocks reading the ServerHello, which is exactly the stall readTimeout has to bound. The call +// runs under a watchdog on a daemon thread: without the timeout the read never returns, and a +// plain assertion would hang the Gradle test JVM instead of failing it. +@ExperimentalSerializationApi +internal class TimeoutTest { + + private val stubTransmissionKey = TransmissionKey(7, ByteArray(32), ByteArray(32)) + private val stubPayload = EncryptedPayload(ByteArray(8), ByteArray(8)) + + // Generous relative to the 1s timeout under test: this is the "it hung" tripwire, not a + // latency assertion, so a loaded CI runner must not trip it. + private val watchdogSeconds = 20L + private val probeReadTimeoutMillis = 1_000 + + @Test + fun timeoutDefaults_matchDocumentedValues() { + val options = SecretsManagerOptions(InMemoryStorage()) + assertEquals(5_000, options.connectTimeoutMillis, "documented default connect timeout") + assertEquals(30_000, options.readTimeoutMillis, "documented default read timeout") + } + + @Test + fun timeoutOptions_acceptCustomValues() { + val options = SecretsManagerOptions( + InMemoryStorage(), + connectTimeoutMillis = 2_000, + readTimeoutMillis = 10_000 + ) + assertEquals(2_000, options.connectTimeoutMillis) + assertEquals(10_000, options.readTimeoutMillis) + } + + // toString is hand-written rather than generated, so every field added to the class has to be + // added here too or it silently disappears from the one output someone reads when a timeout or + // a proxy misbehaves. Asserts the redaction in the same place because it is the same method. + @Test + fun toString_reportsTimeoutsAndRedactsProxyUrl() { + val options = SecretsManagerOptions( + InMemoryStorage(), + connectTimeoutMillis = 1_234, + readTimeoutMillis = 5_678, + // Sentinels rather than realistic values: the rendered storage field carries the + // package name, so a substring like "secret" would match com.keepersecurity + // .secretsManager and pass the leak check for the wrong reason. + proxyUrl = "http://zzuser:zzpassword@zzproxyhost:8080" + ) + val rendered = options.toString() + assertTrue(rendered.contains("connectTimeoutMillis=1234"), "connect timeout missing from: $rendered") + assertTrue(rendered.contains("readTimeoutMillis=5678"), "read timeout missing from: $rendered") + assertTrue(rendered.contains("proxyUrl="), "proxyUrl not redacted in: $rendered") + assertFalse(rendered.contains("zzpassword"), "proxy password leaked into: $rendered") + assertFalse(rendered.contains("zzuser"), "proxy username leaked into: $rendered") + assertFalse(rendered.contains("zzproxyhost"), "proxy host leaked into: $rendered") + } + + @Test + fun readTimeout_boundsAStalledServer() { + ServerSocket(0).use { server -> + // The accepted socket is held for the whole timeout window on purpose. Dropped, it + // turns unreachable the moment the acceptor thread exits, and a GC cycle closes it + // underneath the handshake: the client then fails in tens of milliseconds with a + // handshake or reset error rather than timing out, and this test fails on the wrong + // exception. Reproduced on JDK 8 and 21 under forced GC. + val accepted = AtomicReference(null) + val acceptor = Thread { runCatching { accepted.set(server.accept()) } } + acceptor.isDaemon = true + acceptor.start() + + try { + val url = "https://127.0.0.1:${server.localPort}/" + val elapsedMillis = withWatchdog { + val start = System.nanoTime() + assertFailsWith { + postFunction( + url, + stubTransmissionKey, + stubPayload, + true, + readTimeoutMillis = probeReadTimeoutMillis + ) + } + (System.nanoTime() - start) / 1_000_000 + } + assertTrue( + elapsedMillis >= probeReadTimeoutMillis / 2, + "returned in ${elapsedMillis}ms, too fast to have been the read timeout" + ) + } finally { + acceptor.join(1_000) + accepted.get()?.close() + } + } + } + + // Runs [block] on a daemon thread, failing (rather than blocking) if it never returns. + private fun withWatchdog(block: () -> T): T { + val executor = Executors.newSingleThreadExecutor { runnable -> + Thread(runnable, "timeout-test").apply { isDaemon = true } + } + try { + return executor.submit(block).get(watchdogSeconds, TimeUnit.SECONDS) + } catch (_: TimeoutException) { + fail("call did not return within ${watchdogSeconds}s; the timeout was never applied") + } catch (e: ExecutionException) { + throw e.cause ?: e + } finally { + executor.shutdownNow() + } + } +}