diff --git a/.gitignore b/.gitignore index dfe1cdb0..74afde7f 100644 --- a/.gitignore +++ b/.gitignore @@ -10,4 +10,4 @@ go.work.sum .kiro/ credentials-fetcher /opensource/bin -/opensource/build \ No newline at end of file +/opensource/build diff --git a/LICENSE.txt b/LICENSE.txt new file mode 100644 index 00000000..6b0b1270 --- /dev/null +++ b/LICENSE.txt @@ -0,0 +1,203 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + diff --git a/cmd/credentials-fetcher/main.go b/cmd/credentials-fetcher/main.go index e98127a2..66e15a03 100644 --- a/cmd/credentials-fetcher/main.go +++ b/cmd/credentials-fetcher/main.go @@ -18,6 +18,15 @@ import ( var log = logger.GetInstance() func main() { + // Health check mode — just verify the socket exists and exit + if len(os.Args) > 1 && os.Args[1] == "--healthcheck" { + socketPath := "/var/credentials-fetcher/socket/credentials_fetcher.sock" + if _, err := os.Stat(socketPath); err != nil { + os.Exit(1) + } + os.Exit(0) + } + log.Info("Starting Credentials Fetcher Daemon", "version", constants.Version) // Ensure logger is closed on exit @@ -46,18 +55,21 @@ func main() { if isSystemd { log.Info("Running under systemd") } else { - log.Info("Not running under systemd, watchdog notifications will be simulated") + log.Info("Not running under systemd, skipping watchdog") } - // Create the watchdog - wd, err := watchdog.GetInstance() - if err != nil { - log.Error("Failed to create watchdog", "error", err) - os.Exit(1) + // Create the watchdog (only used under systemd) + var wd *watchdog.Watchdog + if isSystemd { + var err error + wd, err = watchdog.GetInstance() + if err != nil { + log.Error("Failed to create watchdog", "error", err) + os.Exit(1) + } + log.Info("Watchdog created successfully") } - log.Info("Watchdog created successfully") - // Create the gRPC server server := grpc.NewCredentialsFetcherServer(constants.DefaultKrbFilesDir, constants.DefaultAWSSecretName) @@ -67,9 +79,12 @@ func main() { var wg sync.WaitGroup wg.Add(constants.NumberofWaitGroups) - // Start the watchdog in its own goroutine + // Start the watchdog in its own goroutine (only under systemd) go func() { defer wg.Done() + if !isSystemd { + return + } log.Info("Starting watchdog goroutine") if err := wd.Start(ctx); err != nil { log.Error("Watchdog error", "error", err) @@ -107,6 +122,9 @@ func main() { } // Continue checking at regular intervals + cleanupTicker := time.NewTicker(1 * time.Hour) + defer cleanupTicker.Stop() + for { select { case <-ctx.Done(): @@ -118,6 +136,8 @@ func main() { log.Error("Ticket renewal check failed", "error", err) // Continue running even if a check fails } + case <-cleanupTicker.C: + krbClient.CleanupOrphanedTickets(constants.DefaultKrbFilesDir) } } }() diff --git a/configuration/conf/credentials-fetcher-logrotate b/configuration/conf/credentials-fetcher-logrotate new file mode 100644 index 00000000..80fcfe7a --- /dev/null +++ b/configuration/conf/credentials-fetcher-logrotate @@ -0,0 +1,8 @@ +/var/credentials-fetcher/logging/credentials-fetcher.log { + size 50M + rotate 3 + compress + missingok + notifempty + copytruncate +} diff --git a/constants/constants.go b/constants/constants.go index 19b75e18..045ce448 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -77,8 +77,11 @@ const ( MaxPasswordLength = 104 MaxDomainLength = 253 - // InvalidUsernameChars contains characters not allowed in username - InvalidUsernameChars = "&:][+|;$*?<>!/\\\\'`~" + // InvalidSAMAccountNameChars contains characters not allowed in AD sAMAccountName logon names. + // References: + // - https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-2000-server/bb726984(v=technet.10) + // - https://learn.microsoft.com/en-us/troubleshoot/windows-server/active-directory/naming-conventions-for-computer-domain-site-ou + InvalidSAMAccountNameChars = "\"/:;|=,+*?<>\\][&$!'`~(){} " // DomainRegexPattern is the regex pattern for validating domain names DomainRegexPattern = `^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])(\.([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9]))*$` diff --git a/credentials-fetcher.spec b/credentials-fetcher.spec index 59f86160..c8637d46 100644 --- a/credentials-fetcher.spec +++ b/credentials-fetcher.spec @@ -3,7 +3,7 @@ %global major_version 2 %global minor_version 0 -%global patch_version 1 +%global patch_version 2 Name: credentials-fetcher Version: %{major_version}.%{minor_version}.%{patch_version} @@ -84,6 +84,7 @@ cp ./configuration/conf/krb5.conf %{buildroot}/usr/sbin/krb5.conf rm -rf ${RPM_BUILD_ROOT} %files +%license LICENSE.txt /usr/sbin/credentials-fetcher /usr/sbin/krb5.conf %config(noreplace) /etc/credentials-fetcher.conf @@ -98,6 +99,7 @@ rm -rf ${RPM_BUILD_ROOT} %post chmod 644 %{_unitdir}/%{SERVICE_NAME} /usr/bin/systemctl daemon-reload +/usr/bin/systemctl enable %{SERVICE_NAME} # Since `ecs.service` gets a new dependency on `credentials-fetcher.service`, it stops on the initial reload. Start it back up if enabled /usr/bin/systemctl is-enabled --quiet ecs.service 2>/dev/null && /usr/bin/systemctl restart ecs.service || : @@ -118,6 +120,16 @@ if [ $1 -eq 0 ]; then fi %changelog +* Fri May 29 2026 Muskan Lalit +- Set DomainlessUser for AddKerberosArnLease to enable renewals in Fargate managed mode +- Support blue/green username rotation for non-domain-joined mode +- Eliminate GMSA ticket expiry gap during renewal +- Add log file rotation at 10MB +- Remove orphaned tickets after 7 days past renewal +- Skip Secrets Manager fallback when DomainlessUser is set +- Fix invalid char blocklist in AD account name and UTF-8 BOM parsing in credential spec +- Add Fedora packaging files + * Mon Feb 23 2026 Samiullah Mohammed - 2.0.1 - Update ticket renewal logic to fetch username from secret diff --git a/docs/blue-green-username-rotation.md b/docs/blue-green-username-rotation.md new file mode 100644 index 00000000..82ae53af --- /dev/null +++ b/docs/blue-green-username-rotation.md @@ -0,0 +1,299 @@ +# Blue/Green Username Rotation of [Standard User](https://learn.microsoft.com/en-us/virtualization/windowscontainers/manage-containers/manage-serviceaccounts#use-case-for-creating-gmsa-account-for-non-domain-joined-container-hosts) username + +## What is Blue/Green Rotation? + +Blue/green rotation is a credential update strategy borrowed from +blue/green deployments. When the username in AWS Secrets Manager is +changed — whether manually by a user or by an automated rotation policy — +the existing Kerberos tickets on disk are still keyed to the **old** (blue) +username. The service needs to match those old tickets and recreate them +with the **new** (green) username and password. The "blue" and "green" +labels simply distinguish the current-on-disk identity from the newly +desired identity during the transition. Note that when the username is +rotated, the password is also rotated as part of the new account. + +## Problem Statement + +When customers rotate gMSA credentials in AWS Secrets Manager, the +`RenewNonDomainJoinedKerberosLease` RPC needs to know both the **old** and +**new** usernames so it can: + +1. **Match** existing Kerberos tickets (keyed by the old username). +2. **Recreate** those tickets with the new username and password. + +The obvious alternative — having credentials-fetcher read the username +directly from Secrets Manager — requires the EC2 **instance role** to have +`secretsmanager:GetSecretValue` permission. In many ECS deployments only the +**task execution role** has that permission, making the direct-read approach +unusable +([aws/credentials-fetcher#218 (comment)](https://github.com/aws/credentials-fetcher/issues/218#issuecomment-4180459269)). + +## Short-Term Solution: Colon-Separated Username Format + +Active Directory forbids `:` in usernames, so the format is unambiguous. +The caller passes both usernames in the existing `username` field of the +`RenewNonDomainJoinedKerberosLeaseRequest`: + +``` +oldUser:newUser +``` + +- `oldUser` (blue) — used to match existing tickets on disk. +- `newUser` (green) — used to create replacement tickets with the new + credentials. + +When no `:` is present the request is treated as a normal (non-rotation) +renewal. + +### Behaviour + +| Input | Match by | Create with | Metadata updated? | +|-------|----------|-------------|-------------------| +| `alice` | `alice` | `alice` | No (normal renewal) | +| `alice:bob` | `alice` | `bob` | Yes — `DomainlessUser` rewritten to `bob` | +| `alice:alice` | `alice` | `alice` | No (same username, treated as normal renewal) | + +On rotation the service: + +1. Finds all tickets whose `DomainlessUser` (or Secrets Manager–extracted + username) matches the old name. +2. Updates `DomainlessUser` to the new name in memory. +3. Persists the updated metadata JSON to disk (hard failure if write fails). +4. Recreates the Kerberos tickets with the new username/password via + `CreateKerberosTickets`. + +### Validation + +Each side of the `:` is validated independently — the raw `old:new` string +is never passed to AD account-name validation (which would reject the `:`). + +## Long-Term Solution + +The colon-separated format is a pragmatic short-term fix that avoids a +protobuf API change. The proper long-term fix is to extend the gRPC API +with `lease_id` and `distinguished_name` fields: + +```protobuf +message RenewNonDomainJoinedKerberosLeaseRequest { + string username = 1; + string password = 2; + string domain = 3; + string lease_id = 4; // target a specific lease directly + string distinguished_name = 5; // supply DN instead of resolving it +} +``` + +With `lease_id` the service can locate the exact tickets to renew without +relying on username matching, which eliminates the need for the +`oldUser:newUser` convention entirely. Supplying `distinguished_name` +removes the dependency on ECS config / Secrets Manager / LDAP for DN +resolution during renewal. + +Until then, the `oldUser:newUser` convention is documented here and in the +`ParseBlueGreenUsername` function in +`internal/utils/grpc_utils/grpc_utils.go`. + +## Manual Integration Test + +The following test was performed against credentials-fetcher 2.0-2 on an +AL2023 EC2 instance (non-domain-joined ECS cluster) with an AWS Managed +Microsoft AD (`contoso.com`). + +### Prerequisites + +- gMSA account `WebApp01` created in AD with group `WebAppAccounts_OU` +- Both `StandardUser01` and `StandardUser02` exist in AD and are members of + `WebAppAccounts_OU` +- Secret `aws/directoryservice/contoso/standarduser` in Secrets Manager + +### Step 1: Create a lease with StandardUser01 + +```python +response = stub.AddNonDomainJoinedKerberosLease( + credentialsfetcher_pb2.CreateNonDomainJoinedKerberosLeaseRequest( + credspec_contents=[credspec_json], + username="StandardUser01", + password="", + domain="contoso.com" + ) +) +``` + +**Result:** +```json +{ + "success": true, + "lease_id": "48bc39b8e422f64cde60", + "created_kerberos_file_paths": ["/var/credentials-fetcher/krbdir/48bc39b8e422f64cde60/WebApp01"] +} +``` + +Metadata on disk shows `"domainless_user": "StandardUser01"`. + +### Step 2: Update Secrets Manager to blue-green format + +```json +{ + "password": "", + "domainName": "contoso.com", + "username": "StandardUser01:StandardUser02" +} +``` + +### Step 3: Renew with blue-green username + +```python +response = stub.RenewNonDomainJoinedKerberosLease( + credentialsfetcher_pb2.RenewNonDomainJoinedKerberosLeaseRequest( + username="StandardUser01:StandardUser02", + password="", + domain="contoso.com" + ) +) +``` + +**Result:** +```json +{ + "success": true, + "renewed_kerberos_file_paths": ["/var/credentials-fetcher/krbdir/48bc39b8e422f64cde60/WebApp01/krb5cc"] +} +``` + +### Step 4: Verify metadata rotation + +```json +{ + "krb_ticket_info": [ + { + "credential_arn": "arn:aws:secretsmanager:us-west-2:XXXXXXXXXXXX:secret:aws/directoryservice/contoso/standarduser-XXXXXX", + "distinguished_name": "CN=WebApp01,OU=MYOU,OU=Users,OU=contoso,DC=contoso,DC=com", + "domain_name": "contoso.com", + "domainless_user": "StandardUser02", + "krb_file_path": "/var/credentials-fetcher/krbdir/48bc39b8e422f64cde60/WebApp01/krb5cc", + "service_account_name": "WebApp01" + } + ] +} +``` + +`domainless_user` was rewritten from `StandardUser01` → `StandardUser02`, +confirming the blue-green rotation worked end-to-end. + + +## ECS Renewal Flow + +In ECS mode, the ECS agent manages the full lifecycle of Kerberos tickets: + +1. **Task start:** ECS agent reads the secret from Secrets Manager and calls + `AddNonDomainJoinedKerberosLease` with the username and password. +2. **Periodic renewal:** ECS agent re-reads the secret from Secrets Manager + on each renewal cycle (~hourly) and calls + `RenewNonDomainJoinedKerberosLease` with the current username and + password. +3. **Task stop:** ECS agent calls `DeleteKerberosLease` to clean up. + +Because the ECS agent re-reads the secret on every renewal, blue-green +rotation works without task restart: + +``` +Time 0: Secret = "StandardUser01", password = pw1 + → AddNonDomainJoinedKerberosLease(username="StandardUser01", ...) + → metadata: domainless_user = "StandardUser01" + +Time 1: Customer rotates secret to "StandardUser01:StandardUser02", password = pw2 + → RenewNonDomainJoinedKerberosLease(username="StandardUser01:StandardUser02", ...) + → Matches tickets by "StandardUser01", recreates with "StandardUser02" + → metadata: domainless_user = "StandardUser02" + +Time 2+: Secret still "StandardUser01:StandardUser02" + → RenewNonDomainJoinedKerberosLease(username="StandardUser01:StandardUser02", ...) + → Matches tickets by "StandardUser02" (active username fallback) + → Normal kinit renewal (no recreation needed) +``` + +The ECS agent source (`credentialspec_linux.go`) fetches fresh credentials +from Secrets Manager before each renewal call via +`asm.GetSecretFromASM(domainlessGmsaUserArn, asmClient)`. + +## Post-Rotation Renewal (Active Username Fallback) + +After a successful blue-green rotation, the secret may remain in +`oldUser:newUser` format indefinitely (customers are not required to +update it back to a single username). The renewal flow handles this: + +1. Parse `oldUser:newUser` → `matchUsername=oldUser`, + `activeUsername=newUser`. +2. Scan metadata for tickets with `domainless_user == oldUser` → none + found (rotation already applied). +3. **Fallback:** scan for tickets with `domainless_user == activeUsername` + → found. +4. Since tickets already have the active username, no rotation is needed — + renew normally via direct kinit. + +This fallback is implemented in `RenewNonDomainJoinedKerberosLease` with +the `needsRotation` flag: + +```go +needsRotation := false +if isRotation { + for _, ticketInfo := range matchingTicketInfos { + if ticketInfo.DomainlessUser == matchUsername { + needsRotation = true + break + } + } +} +``` + +### Mixed-State Handling + +In rare cases (e.g., partial failure during rotation), some tickets in a +metadata file may have the old username while others already have the new +username. The code handles this by only recreating tickets that still need +rotation: + +```go +if needsRotation { + for _, ticketInfo := range matchingTicketInfos { + if ticketInfo.DomainlessUser == matchUsername { + ticketInfo.DomainlessUser = activeUsername + ticketsToRecreate = append(ticketsToRecreate, ticketInfo) + } else { + // Already rotated — renew normally + krbClient.RenewKerberosTicket(ctx, ticketInfo.KrbFilePath) + } + } +} +``` + +This ensures already-valid tickets are not unnecessarily destroyed and +recreated, and a failure to recreate one ticket does not block renewal of +others. + + +### Edge Cases + +| Scenario | Input | Behaviour | +|----------|-------|-----------| +| Normal renewal (no rotation) | `alice` | Match by `alice`, renew directly | +| First rotation | `alice:bob` | Match by `alice`, recreate with `bob`, update metadata | +| Post-rotation renewal | `alice:bob` (metadata has `bob`) | Match by `bob` (fallback), renew directly | +| Reverse rotation (green→blue) | `bob:alice` (metadata has `bob`) | Match by `bob`, recreate with `alice`, update metadata | +| Post-reverse renewal | `bob:alice` (metadata has `alice`) | Match by `alice` (fallback), renew directly | +| Same username both sides | `alice:alice` | Treated as normal renewal (no rotation) | +| Multiple rotations | `alice:bob` then `bob:charlie` | Each rotation matches old, recreates with new | +| Unrelated username | `charlie:dave` (metadata has `alice`) | No match → renewal fails | + +**Green→Blue (reverse rotation):** The system supports rotating back to +the original user. If the secret is changed from `alice:bob` to +`bob:alice`, the next renewal will: + +1. Parse `bob:alice` → `matchUsername=bob`, `activeUsername=alice` +2. Find tickets with `domainless_user=bob` (from the previous rotation) +3. Recreate them with `alice` credentials +4. Update metadata to `domainless_user=alice` + +Subsequent renewals with `bob:alice` will match by `alice` (active +username fallback) and renew normally. The rotation is fully +bidirectional. diff --git a/fedora/credentials-fetcher.spec b/fedora/credentials-fetcher.spec new file mode 100644 index 00000000..eb8be924 --- /dev/null +++ b/fedora/credentials-fetcher.spec @@ -0,0 +1,124 @@ +# Generated by go2rpm 1.19.0 +# Check assumes a lot of network setup that is not valid +%bcond check 0 + +# Define _unitdir if not already defined +%{!?_unitdir: %global _unitdir /usr/lib/systemd/system} + +# Define _libexec if not already defined +%{!?_libexec: %global _libexec /usr/libexec} + +%define SERVICE_NAME credentials-fetcher.service + +# https://github.com/aws/credentials-fetcher +%global goipath github.com/aws/credentials-fetcher +Version: 2.0.1 + +%gometa -L -f + +Name: credentials-fetcher +Release: %autorelease +Summary: Credentials Fetcher Service is used to connect to Active Directory from Linux Instances + +# Generated by go-vendor-tools +License: Apache-2.0 AND BSD-3-Clause AND ISC AND MIT +URL: %{gourl} +Source0: %{gosource} +# Generated by go-vendor-tools +Source1: %{archivename}-vendor.tar.bz2 +Source2: go-vendor-tools.toml +# LICENSE.txt is at the root of the repository +Source3: LICENSE.txt + +BuildRequires: go-vendor-tools +BuildRequires: krb5-devel + +%description +Credentials-fetcher is a Linux daemon that retrieves gMSA credentials from +Active Directory over LDAP. It creates and refreshes kerberos tickets from gMSA +credentials. Kerberos tickets can be used by containers to run apps/services +that authenticate using Active Directory. + +This (v2) is the Golang refactor of the original credentials-fetcher. + +%prep +%goprep -p1 +tar -xf %{S:1} +cp %{SOURCE3} . +cp %{SOURCE3} cmd/credentials-fetcher/ + +%generate_buildrequires +%go_vendor_license_buildrequires -c %{S:2} + +%build +%global gomodulesmode GO111MODULE=on +%gobuild -o %{gobuilddir}/bin/credentials-fetcherd ./cmd/credentials-fetcher/main.go + +%install +%go_vendor_license_install -c %{S:2} +install -m 0755 -vd %{buildroot}/usr/sbin +install -m 0755 -vp %{gobuilddir}/bin/* %{buildroot}/usr/sbin/ + +# Create additional directory structure in buildroot +mkdir -p %{buildroot}%{_unitdir}/ecs.service.d +mkdir -p %{buildroot}/var/credentials-fetcher/{krbdir,socket,logging} +mkdir -p %{buildroot}/etc/ +mkdir -p %{buildroot}%{_libexec} + +# Copy service file to buildroot +cp ./configuration/bin/credentials-fetcher.service %{buildroot}%{_unitdir}/ +cp ./configuration/bin/ecs-require-credentials-fetcher.conf %{buildroot}%{_unitdir}/ecs.service.d/ + +# Copy startup-order userdata script into libexec +install -m 0755 ./scripts/credentials-fetcher-startup-order.sh %{buildroot}%{_libexec}/ + +# Copy config files to buildroot +cp ./configuration/conf/credentials-fetcher.conf %{buildroot}/etc/ +# Place krb5.conf in /usr/sbin to avoid conflict with system krb5-libs package +# umm... not in Fedora we don't... +# cp ./configuration/conf/krb5.conf %{buildroot}/usr/sbin/krb5.conf + +%check +%go_vendor_license_check -c %{S:2} +%if %{with check} +%gotest ./... +%endif + +%post +chmod 644 %{_unitdir}/%{SERVICE_NAME} +/usr/bin/systemctl daemon-reload +/usr/bin/systemctl enable %{SERVICE_NAME} +# Since `ecs.service` gets a new dependency on `credentials-fetcher.service`, it stops on the initial reload. Start it back up if enabled +/usr/bin/systemctl is-enabled --quiet ecs.service 2>/dev/null && /usr/bin/systemctl restart ecs.service || : + +%postun +/usr/bin/systemctl daemon-reload +# If this is a full removal, and *NOT* an upgrade: +if [ $1 -eq 0 ]; then + # If the user ran our systemd dependency script, there will be an out-of-package systemd drop-in for ECS agent. + # Remove this, and also clean up the drop-in directory, but only if it is empty after removing ours. + if [ -d "/usr/lib/systemd/system/ecs.service.d" ]; then + rm /usr/lib/systemd/system/ecs.service.d/require-credentials-fetcher.conf + if [ -z "$( ls -A '/usr/lib/systemd/system/ecs.service.d' )" ]; then + rm -rf /usr/lib/systemd/system/ecs.service.d + fi + fi + # Service continues running after a full removal, so stop it + /usr/bin/systemctl stop credentials-fetcher.service +fi + +%files -f %{go_vendor_license_filelist} +%license LICENSE.txt +%doc docs README.md +/usr/sbin/credentials-fetcherd +%config(noreplace) /etc/credentials-fetcher.conf +%{_unitdir}/credentials-fetcher.service +%{_unitdir}/ecs.service.d/ecs-require-credentials-fetcher.conf +%dir /var/credentials-fetcher +%dir /var/credentials-fetcher/krbdir +%dir /var/credentials-fetcher/socket +%dir /var/credentials-fetcher/logging +%{_libexec}/credentials-fetcher-startup-order.sh + +%changelog +%autochangelog diff --git a/fedora/go-vendor-tools.toml b/fedora/go-vendor-tools.toml new file mode 100644 index 00000000..598286cc --- /dev/null +++ b/fedora/go-vendor-tools.toml @@ -0,0 +1,12 @@ +[archive] + +[licensing] +detector = "askalono" +[[licensing.licenses]] +path = "vendor/gopkg.in/yaml.v3/LICENSE" +sha256sum = "d18f6323b71b0b768bb5e9616e36da390fbd39369a81807cca352de4e4e6aa0b" +expression = "MIT AND (MIT AND Apache-2.0)" +[[licensing.licenses]] +path = "cmd/credentials-fetcher/LICENSE.txt" +sha256sum = "cfc7749b96f63bd31c3c42b5c471bf756814053e847c10f3eb003417bc523d30" +expression = "Apache-2.0" diff --git a/go.mod b/go.mod index cc6774d9..deda3b54 100644 --- a/go.mod +++ b/go.mod @@ -1,42 +1,42 @@ module golang.a2z.com/CredentialsFetcherV2 -go 1.23.0 +go 1.24.0 toolchain go1.24.2 require ( - github.com/aws/aws-sdk-go-v2 v1.27.2 + github.com/aws/aws-sdk-go-v2 v1.41.5 github.com/aws/aws-sdk-go-v2/config v1.27.9 github.com/aws/aws-sdk-go-v2/credentials v1.17.9 - github.com/aws/aws-sdk-go-v2/service/s3 v1.53.0 + github.com/aws/aws-sdk-go-v2/service/s3 v1.97.3 github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.28.5 github.com/coreos/go-systemd/v22 v22.5.0 github.com/stretchr/testify v1.8.4 - google.golang.org/grpc v1.72.0 - google.golang.org/protobuf v1.36.6 + google.golang.org/grpc v1.80.0 + google.golang.org/protobuf v1.36.11 ) require ( - github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.1 // indirect + github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.8 // indirect github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.0 // indirect - github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.9 // indirect - github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.9 // indirect + github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.21 // indirect + github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.21 // indirect github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect - github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.6 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.1 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.3.6 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.6 // indirect - github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.17.6 // indirect + github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.22 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.7 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.13 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.21 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.21 // indirect github.com/aws/aws-sdk-go-v2/service/sso v1.20.3 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.23.3 // indirect github.com/aws/aws-sdk-go-v2/service/sts v1.28.5 // indirect - github.com/aws/smithy-go v1.20.2 // indirect + github.com/aws/smithy-go v1.24.2 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/stretchr/objx v0.5.0 // indirect - golang.org/x/net v0.38.0 // indirect - golang.org/x/sys v0.31.0 // indirect - golang.org/x/text v0.23.0 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20250218202821-56aae31c358a // indirect + golang.org/x/net v0.49.0 // indirect + golang.org/x/sys v0.40.0 // indirect + golang.org/x/text v0.33.0 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260120221211-b8f7ae30c516 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index b8ecfa01..3ce51b4a 100644 --- a/go.sum +++ b/go.sum @@ -1,31 +1,31 @@ -github.com/aws/aws-sdk-go-v2 v1.27.2 h1:pLsTXqX93rimAOZG2FIYraDQstZaaGVVN4tNw65v0h8= -github.com/aws/aws-sdk-go-v2 v1.27.2/go.mod h1:ffIFB97e2yNsv4aTSGkqtHnppsIJzw7G7BReUZ3jCXM= -github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.1 h1:gTK2uhtAPtFcdRRJilZPx8uJLL2J85xK11nKtWL0wfU= -github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.1/go.mod h1:sxpLb+nZk7tIfCWChfd+h4QwHNUR57d8hA1cleTkjJo= +github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= +github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= +github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.8 h1:eBMB84YGghSocM7PsjmmPffTa+1FBUeNvGvFou6V/4o= +github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.8/go.mod h1:lyw7GFp3qENLh7kwzf7iMzAxDn+NzjXEAGjKS2UOKqI= github.com/aws/aws-sdk-go-v2/config v1.27.9 h1:gRx/NwpNEFSk+yQlgmk1bmxxvQ5TyJ76CWXs9XScTqg= github.com/aws/aws-sdk-go-v2/config v1.27.9/go.mod h1:dK1FQfpwpql83kbD873E9vz4FyAxuJtR22wzoXn3qq0= github.com/aws/aws-sdk-go-v2/credentials v1.17.9 h1:N8s0/7yW+h8qR8WaRlPQeJ6czVMNQVNtNdUqf6cItao= github.com/aws/aws-sdk-go-v2/credentials v1.17.9/go.mod h1:446YhIdmSV0Jf/SLafGZalQo+xr2iw7/fzXGDPTU1yQ= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.0 h1:af5YzcLf80tv4Em4jWVD75lpnOHSBkPUZxZfGkrI3HI= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.0/go.mod h1:nQ3how7DMnFMWiU1SpECohgC82fpn4cKZ875NDMmwtA= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.9 h1:cy8ahBJuhtM8GTTSyOkfy6WVPV1IE+SS5/wfXUYuulw= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.9/go.mod h1:CZBXGLaJnEZI6EVNcPd7a6B5IC5cA/GkRWtu9fp3S6Y= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.9 h1:A4SYk07ef04+vxZToz9LWvAXl9LW0NClpPpMsi31cz0= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.9/go.mod h1:5jJcHuwDagxN+ErjQ3PU3ocf6Ylc/p9x+BLO/+X4iXw= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.21 h1:Rgg6wvjjtX8bNHcvi9OnXWwcE0a2vGpbwmtICOsvcf4= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.21/go.mod h1:A/kJFst/nm//cyqonihbdpQZwiUhhzpqTsdbhDdRF9c= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.21 h1:PEgGVtPoB6NTpPrBgqSE5hE/o47Ij9qk/SEZFbUOe9A= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.21/go.mod h1:p+hz+PRAYlY3zcpJhPwXlLC4C+kqn70WIHwnzAfs6ps= github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 h1:hT8rVHwugYE2lEfdFE0QWVo81lF7jMrYJVDWI+f+VxU= github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0/go.mod h1:8tu/lYfQfFe6IGnaOdrpVgEL2IrrDOf6/m9RQum4NkY= -github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.6 h1:+/uB/M07Isd7UajQIYW2M4lDc/302gIWu1zMe0d7uKo= -github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.6/go.mod h1:7Gw/GeEezsEzpU/f1JWzSb1Y4M05taehNadic8jfF8U= -github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.1 h1:EyBZibRTVAs6ECHZOw5/wlylS9OcTzwyjeQMudmREjE= -github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.1/go.mod h1:JKpmtYhhPs7D97NL/ltqz7yCkERFW5dOlHyVl66ZYF8= -github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.3.6 h1:NkHCgg0Ck86c5PTOzBZ0JRccI51suJDg5lgFtxBu1ek= -github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.3.6/go.mod h1:mjTpxjC8v4SeINTngrnKFgm2QUi+Jm+etTbCxh8W4uU= -github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.6 h1:b+E7zIUHMmcB4Dckjpkapoy47W6C9QBv/zoUP+Hn8Kc= -github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.6/go.mod h1:S2fNV0rxrP78NhPbCZeQgY8H9jdDMeGtwcfZIRxzBqU= -github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.17.6 h1:T9S8fTr0gppdTU6rjrSBl9VFiabxUw41dqzUSozuQP8= -github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.17.6/go.mod h1:mQUaFeeWztcGGBaOxSykT9H/qF+FGw3sCGERh3APsRc= -github.com/aws/aws-sdk-go-v2/service/s3 v1.53.0 h1:r3o2YsgW9zRcIP3Q0WCmttFVhTuugeKIvT5z9xDspc0= -github.com/aws/aws-sdk-go-v2/service/s3 v1.53.0/go.mod h1:w2E4f8PUfNtyjfL6Iu+mWI96FGttE03z3UdNcUEC4tA= +github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.22 h1:rWyie/PxDRIdhNf4DzRk0lvjVOqFJuNnO8WwaIRVxzQ= +github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.22/go.mod h1:zd/JsJ4P7oGfUhXn1VyLqaRZwPmZwg44Jf2dS84Dm3Y= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.7 h1:5EniKhLZe4xzL7a+fU3C2tfUN4nWIqlLesfrjkuPFTY= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.7/go.mod h1:x0nZssQ3qZSnIcePWLvcoFisRXJzcTVvYpAAdYX8+GI= +github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.13 h1:JRaIgADQS/U6uXDqlPiefP32yXTda7Kqfx+LgspooZM= +github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.13/go.mod h1:CEuVn5WqOMilYl+tbccq8+N2ieCy0gVn3OtRb0vBNNM= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.21 h1:c31//R3xgIJMSC8S6hEVq+38DcvUlgFY0FM6mSI5oto= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.21/go.mod h1:r6+pf23ouCB718FUxaqzZdbpYFyDtehyZcmP5KL9FkA= +github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.21 h1:ZlvrNcHSFFWURB8avufQq9gFsheUgjVD9536obIknfM= +github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.21/go.mod h1:cv3TNhVrssKR0O/xxLJVRfd2oazSnZnkUeTf6ctUwfQ= +github.com/aws/aws-sdk-go-v2/service/s3 v1.97.3 h1:HwxWTbTrIHm5qY+CAEur0s/figc3qwvLWsNkF4RPToo= +github.com/aws/aws-sdk-go-v2/service/s3 v1.97.3/go.mod h1:uoA43SdFwacedBfSgfFSjjCvYe8aYBS7EnU5GZ/YKMM= github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.28.5 h1:1i3Pq5g1NaXI/u8lTHRVMHyCc0HoZzSk2EFmiy14Hbk= github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.28.5/go.mod h1:slgOMs1CQu8UVgwoFqEvCi71L4HVoZgM0r8MtcNP6Mc= github.com/aws/aws-sdk-go-v2/service/sso v1.20.3 h1:mnbuWHOcM70/OFUlZZ5rcdfA8PflGXXiefU/O+1S3+8= @@ -34,22 +34,24 @@ github.com/aws/aws-sdk-go-v2/service/ssooidc v1.23.3 h1:uLq0BKatTmDzWa/Nu4WO0M1A github.com/aws/aws-sdk-go-v2/service/ssooidc v1.23.3/go.mod h1:b+qdhjnxj8GSR6t5YfphOffeoQSQ1KmpoVVuBn+PWxs= github.com/aws/aws-sdk-go-v2/service/sts v1.28.5 h1:J/PpTf/hllOjx8Xu9DMflff3FajfLxqM5+tepvVXmxg= github.com/aws/aws-sdk-go-v2/service/sts v1.28.5/go.mod h1:0ih0Z83YDH/QeQ6Ori2yGE2XvWYv/Xm+cZc01LC6oK0= -github.com/aws/smithy-go v1.20.2 h1:tbp628ireGtzcHDDmLT/6ADHidqnwgF57XOXZe6tp4Q= -github.com/aws/smithy-go v1.20.2/go.mod h1:krry+ya/rV9RDcV/Q16kpu6ypI4K2czasz0NC3qS14E= +github.com/aws/smithy-go v1.24.2 h1:FzA3bu/nt/vDvmnkg+R8Xl46gmzEDam6mZ1hzmwXFng= +github.com/aws/smithy-go v1.24.2/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= +github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= +github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8iXXhfZs= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= -github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= +github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= -github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= -github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -62,30 +64,32 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA= -go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A= -go.opentelemetry.io/otel v1.34.0 h1:zRLXxLCgL1WyKsPVrgbSdMN4c0FMkDAskSTQP+0hdUY= -go.opentelemetry.io/otel v1.34.0/go.mod h1:OWFPOQ+h4G8xpyjgqo4SxJYdDQ/qmRH+wivy7zzx9oI= -go.opentelemetry.io/otel/metric v1.34.0 h1:+eTR3U0MyfWjRDhmFMxe2SsW64QrZ84AOhvqS7Y+PoQ= -go.opentelemetry.io/otel/metric v1.34.0/go.mod h1:CEDrp0fy2D0MvkXE+dPV7cMi8tWZwX3dmaIhwPOaqHE= -go.opentelemetry.io/otel/sdk v1.34.0 h1:95zS4k/2GOy069d321O8jWgYsW3MzVV+KuSPKp7Wr1A= -go.opentelemetry.io/otel/sdk v1.34.0/go.mod h1:0e/pNiaMAqaykJGKbi+tSjWfNNHMTxoC9qANsCzbyxU= -go.opentelemetry.io/otel/sdk/metric v1.34.0 h1:5CeK9ujjbFVL5c1PhLuStg1wxA7vQv7ce1EK0Gyvahk= -go.opentelemetry.io/otel/sdk/metric v1.34.0/go.mod h1:jQ/r8Ze28zRKoNRdkjCZxfs6YvBTG1+YIqyFVFYec5w= -go.opentelemetry.io/otel/trace v1.34.0 h1:+ouXS2V8Rd4hp4580a8q23bg0azF2nI8cqLYnC8mh/k= -go.opentelemetry.io/otel/trace v1.34.0/go.mod h1:Svm7lSjQD7kG7KJ/MUHPVXSDGz2OX4h0M2jHBhmSfRE= -golang.org/x/net v0.38.0 h1:vRMAPTMaeGqVhG5QyLJHqNDwecKTomGeqbnfZyKlBI8= -golang.org/x/net v0.38.0/go.mod h1:ivrbrMbzFq5J41QOQh0siUuly180yBYtLp+CKbEaFx8= -golang.org/x/sys v0.31.0 h1:ioabZlmFYtWhL+TRYpcnNlLwhyxaM9kWTDEmfnprqik= -golang.org/x/sys v0.31.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= -golang.org/x/text v0.23.0 h1:D71I7dUrlY+VX0gQShAThNGHFxZ13dGLBHQLVl1mJlY= -golang.org/x/text v0.23.0/go.mod h1:/BLNzu4aZCJ1+kcD0DNRotWKage4q2rGVAg4o22unh4= -google.golang.org/genproto/googleapis/rpc v0.0.0-20250218202821-56aae31c358a h1:51aaUVRocpvUOSQKM6Q7VuoaktNIaMCLuhZB6DKksq4= -google.golang.org/genproto/googleapis/rpc v0.0.0-20250218202821-56aae31c358a/go.mod h1:uRxBH1mhmO8PGhU89cMcHaXKZqO+OfakD8QQO0oYwlQ= -google.golang.org/grpc v1.72.0 h1:S7UkcVa60b5AAQTaO6ZKamFp1zMZSU0fGDK2WZLbBnM= -google.golang.org/grpc v1.72.0/go.mod h1:wH5Aktxcg25y1I3w7H69nHfXdOG3UiadoBtjh3izSDM= -google.golang.org/protobuf v1.36.6 h1:z1NpPI8ku2WgiWnf+t9wTPsn6eP1L7ksHUlkfLvd9xY= -google.golang.org/protobuf v1.36.6/go.mod h1:jduwjTPXsFjZGTmRluh+L6NjiWu7pchiJ2/5YcXBHnY= +go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= +go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= +go.opentelemetry.io/otel v1.39.0 h1:8yPrr/S0ND9QEfTfdP9V+SiwT4E0G7Y5MO7p85nis48= +go.opentelemetry.io/otel v1.39.0/go.mod h1:kLlFTywNWrFyEdH0oj2xK0bFYZtHRYUdv1NklR/tgc8= +go.opentelemetry.io/otel/metric v1.39.0 h1:d1UzonvEZriVfpNKEVmHXbdf909uGTOQjA0HF0Ls5Q0= +go.opentelemetry.io/otel/metric v1.39.0/go.mod h1:jrZSWL33sD7bBxg1xjrqyDjnuzTUB0x1nBERXd7Ftcs= +go.opentelemetry.io/otel/sdk v1.39.0 h1:nMLYcjVsvdui1B/4FRkwjzoRVsMK8uL/cj0OyhKzt18= +go.opentelemetry.io/otel/sdk v1.39.0/go.mod h1:vDojkC4/jsTJsE+kh+LXYQlbL8CgrEcwmt1ENZszdJE= +go.opentelemetry.io/otel/sdk/metric v1.39.0 h1:cXMVVFVgsIf2YL6QkRF4Urbr/aMInf+2WKg+sEJTtB8= +go.opentelemetry.io/otel/sdk/metric v1.39.0/go.mod h1:xq9HEVH7qeX69/JnwEfp6fVq5wosJsY1mt4lLfYdVew= +go.opentelemetry.io/otel/trace v1.39.0 h1:2d2vfpEDmCJ5zVYz7ijaJdOF59xLomrvj7bjt6/qCJI= +go.opentelemetry.io/otel/trace v1.39.0/go.mod h1:88w4/PnZSazkGzz/w84VHpQafiU4EtqqlVdxWy+rNOA= +golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= +golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= +golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= +golang.org/x/sys v0.40.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/text v0.33.0 h1:B3njUFyqtHDUI5jMn1YIr5B0IE2U0qck04r6d4KPAxE= +golang.org/x/text v0.33.0/go.mod h1:LuMebE6+rBincTi9+xWTY8TztLzKHc/9C1uBCG27+q8= +gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= +gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260120221211-b8f7ae30c516 h1:sNrWoksmOyF5bvJUcnmbeAmQi8baNhqg5IWaI3llQqU= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260120221211-b8f7ae30c516/go.mod h1:j9x/tPzZkyxcgEFkiKEEGxfvyumM01BEtsW8xzOahRQ= +google.golang.org/grpc v1.80.0 h1:Xr6m2WmWZLETvUNvIUmeD5OAagMw3FiKmMlTdViWsHM= +google.golang.org/grpc v1.80.0/go.mod h1:ho/dLnxwi3EDJA4Zghp7k2Ec1+c2jqup0bFkw07bwF4= +google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= +google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/auth/kerberos/krb_client.go b/internal/auth/kerberos/krb_client.go index d3957c30..a2f24754 100644 --- a/internal/auth/kerberos/krb_client.go +++ b/internal/auth/kerberos/krb_client.go @@ -38,6 +38,8 @@ var ( getMetadataFilePathsFunc = metadata_utils.GetMetadataFilePaths getFQDNListFunc = grpc_utils.GetFQDNList newLdapClientFunc = ldap.NewClient + // getTicketsFromMetadataFunc is mockable for testing CleanupOrphanedTickets + getTicketsFromMetadataFunc func(c *Client, metadataPath string) ([]*types.Ticket, []*types.TicketInfo, error) = (*Client).GetTicketsFromMetadata ) // Client provides Kerberos authentication operations using CGO-based krb_utils package @@ -580,27 +582,34 @@ func (c *Client) CheckAndRenewTicket(ctx context.Context, ticketInfo *types.Tick "path", ticketInfo.KrbFilePath, "service_account", ticketInfo.ServiceAccountName) + domainlessUser := ticketInfo.DomainlessUser + + // Agent-managed tickets (created via AddKerberosArnLease or AddNonDomainJoinedKerberosLease) + // are renewed by the agent via gRPC calls. Skip internal renewal for these. + if strings.Contains(domainlessUser, "awsdomainlessusersecret") { + log.Info("Skipping internal renewal for agent-managed ticket", + "path", ticketInfo.KrbFilePath, + "domainless_user", domainlessUser) + return nil + } + // Get the ticket information ticket, _, err := c.GetTicket(ticketInfo.KrbFilePath) if err != nil { return fmt.Errorf("failed to get ticket information: %w", err) } - domainlessUser := ticketInfo.DomainlessUser - // Check if the ticket is ready for renewal and either: // 1. Not a domainless user (empty string), OR - // 2. A domainless user created using the Domain Joined API + // 2. A standalone domainless user (config flag enabled) isNotDomainlessUser := domainlessUser == "" - isDomainlessUserWithSecret := strings.Contains(domainlessUser, "awsdomainlessusersecret") isDomainlessUserStandalone := config_utils.IsRunRenewalNonDomainJoinedEnabled() - if !isDomainlessUserStandalone && !isNotDomainlessUser && !isDomainlessUserWithSecret { + if !isDomainlessUserStandalone && !isNotDomainlessUser { log.Info("Skipping renewal for domainless user not created using Domain Join API", "path", ticketInfo.KrbFilePath, "principal", ticket.Principal, "domainless_user", domainlessUser, - "isDomainlessUserWithSecret", isDomainlessUserWithSecret, "isDomainlessUserStandalone", isDomainlessUserStandalone) } else if !krb_utils.IsTicketReadyForRenewal(ticket) { log.Info("Ticket does not need renewal yet", @@ -615,8 +624,19 @@ func (c *Client) CheckAndRenewTicket(ctx context.Context, ticketInfo *types.Tick // Try renewal first using CGO-based implementation if err := c.RenewKerberosTicket(ctx, ticketInfo.KrbFilePath); err == nil { - log.Info("Direct renewal of the ticket successful using CGO") - return nil + // Verify the renewed ticket actually has an extended expiry + renewedTicket, _, err := c.GetTicket(ticketInfo.KrbFilePath) + if err != nil { + log.Warn("CGO renewal succeeded but failed to re-read ticket, falling back to recreation", "error", err) + } else if renewedTicket.ExpirationTime.After(ticket.ExpirationTime) { + log.Info("Direct renewal of the ticket successful using CGO", + "new_expiry", renewedTicket.ExpirationTime.Format(time.RFC3339)) + return nil + } else { + log.Warn("CGO renewal reported success but ticket expiry was not extended, falling back to recreation", + "old_expiry", ticket.ExpirationTime.Format(time.RFC3339), + "current_expiry", renewedTicket.ExpirationTime.Format(time.RFC3339)) + } } else { log.Warn("Direct renewal failed, attempting ticket recreation", "error", err) } @@ -634,6 +654,7 @@ func (c *Client) CheckAndRenewTicket(ctx context.Context, ticketInfo *types.Tick // recreateTicketWithRetries attempts to recreate a Kerberos ticket with the specified number of retries. func (c *Client) recreateTicketWithRetries(ctx context.Context, ticketInfo *types.TicketInfo, numRetries int, isDomainlessUserStandalone bool) error { domainlessUser := ticketInfo.DomainlessUser + maxRetries := numRetries + 1 // allow at most one extension beyond the original retry count for i := 0; i <= numRetries; i++ { // Try to recreate the ticket using gMSA password @@ -650,9 +671,12 @@ func (c *Client) recreateTicketWithRetries(ctx context.Context, ticketInfo *type // Try alternative methods based on user type if err := c.tryAlternativeTicketCreation(ctx, ticketInfo, domainlessUser, isDomainlessUserStandalone); err == nil { - // Success with alternative method - log.Info("Renewal successful after recreating user principal or machine principal Kerberos ticket") - return nil + // User/machine ticket recreated — ensure at least one more GMSA attempt + log.Info("Successfully recreated user principal or machine principal Kerberos ticket, retrying GMSA ticket creation") + if i == numRetries && numRetries < maxRetries { + numRetries++ // allow one final GMSA attempt with fresh credentials + } + continue } } } @@ -829,3 +853,64 @@ func (c *Client) GenerateKrbTicketUsingSecretVault(ctx context.Context, domain, return nil } + +// orphanedTicketGracePeriod is how long past the renew_until time before a ticket +// is considered orphaned and eligible for cleanup (7 days). +const orphanedTicketGracePeriod = 7 * 24 * time.Hour + +// CleanupOrphanedTickets removes lease directories containing tickets that have +// been expired beyond the grace period. This handles tickets left behind after +// instance reboots where the ECS agent loses track of running tasks. +func (c *Client) CleanupOrphanedTickets(krbFilesDir string) { + log.Info("Running orphaned ticket cleanup", "directory", krbFilesDir) + + metadataFiles, err := metadata_utils.GetMetadataFilePaths(krbFilesDir) + if err != nil { + log.Error("Failed to get metadata files for cleanup", "error", err) + return + } + + if len(metadataFiles) == 0 { + return + } + + now := time.Now() + cleaned := 0 + + for _, metadataPath := range metadataFiles { + tickets, _, err := getTicketsFromMetadataFunc(c, metadataPath) + if err != nil { + log.Warn("Failed to read tickets for cleanup check", "path", metadataPath, "error", err) + continue + } + + // Check if all tickets in this lease are expired beyond grace period + allExpired := true + for _, ticket := range tickets { + if ticket.RenewUntil.IsZero() { + allExpired = false + break + } + if now.Before(ticket.RenewUntil.Add(orphanedTicketGracePeriod)) { + allExpired = false + break + } + } + + if allExpired && len(tickets) > 0 { + // Extract lease directory from metadata path + leaseDir := filepath.Dir(metadataPath) + log.Info("Removing orphaned lease", "path", leaseDir, + "oldest_renew_until", tickets[0].RenewUntil.Format(time.RFC3339)) + if err := os.RemoveAll(leaseDir); err != nil { + log.Error("Failed to remove orphaned lease directory", "path", leaseDir, "error", err) + } else { + cleaned++ + } + } + } + + if cleaned > 0 { + log.Info("Orphaned ticket cleanup complete", "removed", cleaned) + } +} diff --git a/internal/auth/kerberos/krb_client_test.go b/internal/auth/kerberos/krb_client_test.go index 26b648ef..affb150e 100644 --- a/internal/auth/kerberos/krb_client_test.go +++ b/internal/auth/kerberos/krb_client_test.go @@ -5,12 +5,14 @@ import ( "errors" "fmt" "os" + "path/filepath" "strings" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "golang.a2z.com/CredentialsFetcherV2/internal/utils/krb_utils" "golang.a2z.com/CredentialsFetcherV2/internal/utils/types" ) @@ -1016,11 +1018,11 @@ func TestCheckAndRenewTicket(t *testing.T) { KrbFilePath: "/path/to/krb5cc_test", DomainlessUser: "", // Regular user (not domainless) }, - mockKlistOutput: []byte(generateKlistOutput(24)), // Expires in 24 hours (no renewal needed) + mockKlistOutput: []byte(generateKlistOutput(24)), // Expires in 24 hours mockKlistErr: nil, - mockIsTicketReadyForRenewal: false, // The ticket expires tomorrow, so no renewal needed + mockIsTicketReadyForRenewal: false, mockRenewErr: nil, - expectedRenewCalls: 0, // No renewal expected + expectedRenewCalls: 0, // No renewal expected (threshold not met) expectedError: false, }, { @@ -1081,7 +1083,7 @@ func TestCheckAndRenewTicket(t *testing.T) { mock.Anything, // context "klist", // command "-c", tc.ticketInfo.KrbFilePath, // args - ).Return(tc.mockKlistOutput, tc.mockKlistErr) + ).Return(tc.mockKlistOutput, tc.mockKlistErr).Once() // Set up expectations for renewal if needed if tc.expectedRenewCalls > 0 { @@ -1090,6 +1092,17 @@ func TestCheckAndRenewTicket(t *testing.T) { config.RenewTicket == true && config.Verify == true })).Return(tc.mockRenewErr) + + // After successful renewal, CheckAndRenewTicket re-reads the ticket + // to verify the expiry was actually extended + if tc.mockRenewErr == nil { + renewedOutput := []byte(generateKlistOutput(10)) // Extended expiry + mockExecutor.On("Execute", + mock.Anything, + "klist", + "-c", tc.ticketInfo.KrbFilePath, + ).Return(renewedOutput, nil).Once() + } } // Create a client with mocks @@ -1124,14 +1137,14 @@ func TestCheckAndRenewTicketDomainlessUser(t *testing.T) { expectedError bool }{ { - name: "Domainless user with secret - processes renewal", + name: "Domainless user with secret - skips internal renewal (agent-managed)", ticketInfo: &types.TicketInfo{ ServiceAccountName: "testuser", DomainName: "example.com", KrbFilePath: "/path/to/krb5cc_test", DomainlessUser: "awsdomainlessusersecret:my-secret", }, - expectedRenewCalls: 1, + expectedRenewCalls: 0, // Agent-managed tickets skip internal renewal expectedError: false, }, } @@ -1142,19 +1155,34 @@ func TestCheckAndRenewTicketDomainlessUser(t *testing.T) { mockExecutor := new(MockExecutor) mockKrb5Client := new(MockKrb5Client) - // Set up expectations for GetTicket (klist command) - mockExecutor.On("Execute", - mock.Anything, // context - "klist", // command - "-c", tc.ticketInfo.KrbFilePath, // args - ).Return([]byte(generateKlistOutput(0)), nil) // Expires in 30 minutes (needs renewal) + // Set up expectations for GetTicket (klist command) - only called for non-agent-managed tickets + if tc.expectedRenewCalls > 0 { + mockExecutor.On("Execute", + mock.Anything, // context + "klist", // command + "-c", tc.ticketInfo.KrbFilePath, // args + ).Return([]byte(generateKlistOutput(0)), nil).Once() // Expires in 30 minutes (needs renewal) + } // Set up expectations for successful renewal - mockKrb5Client.On("GenerateTicket", mock.MatchedBy(func(config *krb_utils.KinitConfig) bool { - return config.CCachePath == tc.ticketInfo.KrbFilePath && - config.RenewTicket == true && - config.Verify == true - })).Return(nil) + if tc.expectedRenewCalls > 0 { + mockKrb5Client.On("GenerateTicket", mock.MatchedBy(func(config *krb_utils.KinitConfig) bool { + return config.CCachePath == tc.ticketInfo.KrbFilePath && + config.RenewTicket == true && + config.Verify == true + })).Return(nil) + } + + // After successful renewal, CheckAndRenewTicket re-reads the ticket + // to verify the expiry was actually extended + if tc.expectedRenewCalls > 0 { + renewedOutput := []byte(generateKlistOutput(10)) // Extended expiry + mockExecutor.On("Execute", + mock.Anything, + "klist", + "-c", tc.ticketInfo.KrbFilePath, + ).Return(renewedOutput, nil).Once() + } // Create a client with mocks client := &Client{ @@ -1259,3 +1287,57 @@ func TestCheckAndRenewTicketRenewalFailureFallback(t *testing.T) { mockExecutor.AssertExpectations(t) }) } + +func TestCleanupOrphanedTickets(t *testing.T) { + client := NewClient() + + t.Run("Removes leases expired beyond grace period", func(t *testing.T) { + tmpDir := t.TempDir() + + // Create an orphaned lease directory with metadata + orphanLease := filepath.Join(tmpDir, "orphan123") + require.NoError(t, os.MkdirAll(filepath.Join(orphanLease, "WebApp01"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(orphanLease, "WebApp01", "krb5cc"), []byte("fake"), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(orphanLease, "orphan123_metadata.json"), []byte(`{"krb_ticket_info":[]}`), 0644)) + + // Create an active lease directory with metadata + activeLease := filepath.Join(tmpDir, "active456") + require.NoError(t, os.MkdirAll(filepath.Join(activeLease, "WebApp01"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(activeLease, "WebApp01", "krb5cc"), []byte("fake"), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(activeLease, "active456_metadata.json"), []byte(`{"krb_ticket_info":[]}`), 0644)) + + // Mock GetTicketsFromMetadata to return controllable RenewUntil values + originalFunc := getTicketsFromMetadataFunc + defer func() { getTicketsFromMetadataFunc = originalFunc }() + + getTicketsFromMetadataFunc = func(_ *Client, metadataPath string) ([]*types.Ticket, []*types.TicketInfo, error) { + if strings.Contains(metadataPath, "orphan123") { + return []*types.Ticket{{ + RenewUntil: time.Now().Add(-8 * 24 * time.Hour), + }}, nil, nil + } + return []*types.Ticket{{ + RenewUntil: time.Now().Add(10 * time.Hour), + }}, nil, nil + } + + client.CleanupOrphanedTickets(tmpDir) + + // Orphaned lease should be removed + _, err := os.Stat(orphanLease) + assert.True(t, os.IsNotExist(err), "Orphaned lease should be removed") + + // Active lease should still exist + _, err = os.Stat(activeLease) + assert.NoError(t, err, "Active lease should not be removed") + }) + + t.Run("Does nothing with empty directory", func(t *testing.T) { + tmpDir := t.TempDir() + client.CleanupOrphanedTickets(tmpDir) + }) + + t.Run("Does nothing with non-existent directory", func(t *testing.T) { + client.CleanupOrphanedTickets("/tmp/nonexistent-krbdir-xyz") + }) +} diff --git a/internal/grpc/api/AddKerberosArnLease.go b/internal/grpc/api/AddKerberosArnLease.go index 8013f28d..ce0bfa48 100644 --- a/internal/grpc/api/AddKerberosArnLease.go +++ b/internal/grpc/api/AddKerberosArnLease.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "strings" + "sync" "golang.a2z.com/CredentialsFetcherV2/constants" @@ -76,6 +77,7 @@ type KerberosArnLeaseHandler struct { krbFilesDir string krbClient *kerberos.Client shellExecutor cmdexec.Executor + renewMu sync.Mutex // serialize renewal attempts across gRPC calls } // NewKerberosArnLeaseHandler creates a new KerberosArnLeaseHandler @@ -407,6 +409,9 @@ func (h *KerberosArnLeaseHandler) createKerberosTickets(ctx context.Context, cfg return fmt.Errorf("failed to create gMSA ticket: %v", err) } + // Update DomainlessUser to include secret ARN for renewal identification + krbTicket.DomainlessUser = "awsdomainlessusersecret:" + secretsArn + log.Info("Successfully created Kerberos ticket for", "user", krbTicket.DomainlessUser) grpc_utils.SecureClearString(&username) diff --git a/internal/grpc/api/AddKerberosLease.go b/internal/grpc/api/AddKerberosLease.go index e01c8fec..89e28115 100644 --- a/internal/grpc/api/AddKerberosLease.go +++ b/internal/grpc/api/AddKerberosLease.go @@ -7,6 +7,7 @@ import ( "path/filepath" "golang.a2z.com/CredentialsFetcherV2/internal/auth/kerberos" + "golang.a2z.com/CredentialsFetcherV2/internal/utils/debug_utils" pb "golang.a2z.com/CredentialsFetcherV2/internal/grpc/proto" "golang.a2z.com/CredentialsFetcherV2/internal/utils/grpc_utils" @@ -156,12 +157,14 @@ func (h *DomainJoinedKerberosLeaseHandler) CreateKerberosTickets(ctx context.Con // Now proceed with creating tickets for each gMSA account krbFilePath, err := h.SetupKerberosFileForTicket(ticketInfo) + if err == nil { + err = debug_utils.SimulateDebugError(debug_utils.SimulateSetupKerberosFile) + } if err != nil { - // Clean up any created files on error for _, path := range createdKrbFilePaths { - err := h.CleanupKerberosFiles(path) // #nosec G104 - if err != nil { - return nil, err + if cleanupErr := h.CleanupKerberosFiles(path); cleanupErr != nil { + log.Error("Failed to clean up Kerberos files during error handling", + "path", path, "error", cleanupErr) } } return nil, err @@ -169,17 +172,20 @@ func (h *DomainJoinedKerberosLeaseHandler) CreateKerberosTickets(ctx context.Con // Create krb ticket for this gmsa account using the ticketInfo err = h.CreateTicketForGMSA(ctx, ticketInfo) + if err == nil { + err = debug_utils.SimulateDebugError(debug_utils.SimulateCreateTicketGMSA) + } if err != nil { - log.Error("Failed to create Kerberos ticket for gMSA account", "error", err) - // Clean up Kerberos files if there's an error - err := h.CleanupKerberosFiles(krbFilePath) // #nosec G104 - if err != nil { - return nil, err + log.Error("Failed to create Kerberos ticket for gMSA account", + "service_account", ticketInfo.ServiceAccountName, "error", err) + if cleanupErr := h.CleanupKerberosFiles(krbFilePath); cleanupErr != nil { + log.Error("Failed to clean up Kerberos files during error handling", + "path", krbFilePath, "error", cleanupErr) } for _, path := range createdKrbFilePaths { - err := h.CleanupKerberosFiles(path) // #nosec G104 - if err != nil { - return nil, err + if cleanupErr := h.CleanupKerberosFiles(path); cleanupErr != nil { + log.Error("Failed to clean up Kerberos files during error handling", + "path", path, "error", cleanupErr) } } return nil, fmt.Errorf("failed to create Kerberos ticket for gMSA account: %v", err) diff --git a/internal/grpc/api/AddNonDomainJoinedKerberosLease.go b/internal/grpc/api/AddNonDomainJoinedKerberosLease.go index 0bff99e3..6c97ba85 100644 --- a/internal/grpc/api/AddNonDomainJoinedKerberosLease.go +++ b/internal/grpc/api/AddNonDomainJoinedKerberosLease.go @@ -7,6 +7,7 @@ import ( "path/filepath" "golang.a2z.com/CredentialsFetcherV2/internal/utils/config_utils" + "golang.a2z.com/CredentialsFetcherV2/internal/utils/debug_utils" "golang.a2z.com/CredentialsFetcherV2/constants" "golang.a2z.com/CredentialsFetcherV2/internal/auth/kerberos" @@ -33,7 +34,7 @@ type KerberosTicketOperations interface { SetupKerberosFileForTicket(ticketInfo *types.TicketInfo) (string, error) // GetDistinguishedName gets the distinguished name from ECS config or secrets manager - GetDistinguishedName(ticketInfo *types.TicketInfo) (string, error) + GetDistinguishedName(ticketInfo *types.TicketInfo) string // CreateKerberosTickets creates Kerberos tickets for each ticket info CreateKerberosTickets(ctx context.Context, domain, username, password string, ticketInfoList []*types.TicketInfo) ([]string, error) @@ -78,8 +79,25 @@ func (h *NonDomainJoinedKerberosHandler) AddNonDomainJoinedKerberosLease(ctx con grpc_utils.SecureClearString(&req.Password) }() - // Validate request - if err := h.ValidateCredentials(req.Username, req.Password, req.Domain); err != nil { + // Parse blue/green username rotation format ("oldUser:newUser"). + // When usernames are rotated in AWS Secrets Manager, the caller may supply + // "oldUser:newUser" so that the service creates tickets under the new (green) + // username. For the Add flow only the active (green) username is used. + _, activeUsername, isRotation := grpc_utils.ParseBlueGreenUsername(req.Username) + if isRotation { + if activeUsername == "" { + return nil, fmt.Errorf("blue/green rotation format requires a non-empty new username (\"oldUser:newUser\")") + } + log.Info("Blue/green username detected in Add request, using active username", + "active_username", activeUsername) + } + + // Use the active (green) username for all downstream operations — validation, + // credential spec processing, and ticket creation. + username := activeUsername + + // Validate request using the resolved username + if err := h.ValidateCredentials(username, req.Password, req.Domain); err != nil { return nil, err } @@ -96,13 +114,13 @@ func (h *NonDomainJoinedKerberosHandler) AddNonDomainJoinedKerberosLease(ctx con } // Process credential specs - ticketInfoList, err := h.ProcessCredentialSpecs(req.CredspecContents, req.Username, leaseID) + ticketInfoList, err := h.ProcessCredentialSpecs(req.CredspecContents, username, leaseID) if err != nil { return nil, err } // Create Kerberos tickets - createdKrbFilePaths, err := h.CreateKerberosTickets(ctx, req.Domain, req.Username, req.Password, ticketInfoList) + createdKrbFilePaths, err := h.CreateKerberosTickets(ctx, req.Domain, username, req.Password, ticketInfoList) if err != nil { return nil, err } @@ -190,32 +208,25 @@ func (h *NonDomainJoinedKerberosHandler) CreateKerberosTickets(ctx context.Conte for _, ticketInfo := range ticketInfoList { krbFilePath, err := h.SetupKerberosFileForTicket(ticketInfo) + if err == nil { + err = debug_utils.SimulateDebugError(debug_utils.SimulateSetupKerberosFile) + } if err != nil { - // Clean up any created files on error for _, path := range createdKrbFilePaths { - err := h.CleanupKerberosFiles(path) - if err != nil { - return nil, err + if cleanupErr := h.CleanupKerberosFiles(path); cleanupErr != nil { + log.Error("Failed to clean up Kerberos files during error handling", + "path", path, "error", cleanupErr) } } return nil, err } - // Get distinguished name - distinguishedName, err := h.GetDistinguishedName(ticketInfo) - if err != nil { - // Clean up any created files on error - err := h.CleanupKerberosFiles(krbFilePath) - if err != nil { - return nil, err - } - for _, path := range createdKrbFilePaths { - err := h.CleanupKerberosFiles(path) - if err != nil { - return nil, err - } - } - return nil, err + // Get distinguished name (non-fatal: empty string is acceptable, + // downstream LDAP lookup in CreateTicketForGMSA will resolve it) + distinguishedName := h.GetDistinguishedName(ticketInfo) + if distinguishedName == "" { + log.Warn("Distinguished name lookup returned empty, will rely on downstream LDAP resolution", + "service_account", ticketInfo.ServiceAccountName) } // Update ticketInfo with the distinguished name @@ -224,17 +235,19 @@ func (h *NonDomainJoinedKerberosHandler) CreateKerberosTickets(ctx context.Conte // Create krb ticket for this gmsa account using the ticketInfo err = h.krbClient.CreateTicketForGMSA(ticketInfo) + if err == nil { + err = debug_utils.SimulateDebugError(debug_utils.SimulateCreateTicketGMSA) + } if err != nil { log.Error("Failed to create Kerberos ticket for gMSA account", "error", err) - // Clean up Kerberos files if there's an error - err := h.CleanupKerberosFiles(krbFilePath) - if err != nil { - return nil, err + if cleanupErr := h.CleanupKerberosFiles(krbFilePath); cleanupErr != nil { + log.Error("Failed to clean up Kerberos files during error handling", + "path", krbFilePath, "error", cleanupErr) } for _, path := range createdKrbFilePaths { - err := h.CleanupKerberosFiles(path) - if err != nil { - return nil, err + if cleanupErr := h.CleanupKerberosFiles(path); cleanupErr != nil { + log.Error("Failed to clean up Kerberos files during error handling", + "path", path, "error", cleanupErr) } } return nil, fmt.Errorf("failed to create Kerberos ticket for gMSA account: %v", err) @@ -286,32 +299,34 @@ func (h *NonDomainJoinedKerberosHandler) SetupKerberosFileForTicket(ticketInfo * return krbDirectoryPath, nil } -// GetDistinguishedName gets the distinguished name from ECS config or secrets manager -func (h *NonDomainJoinedKerberosHandler) GetDistinguishedName(ticketInfo *types.TicketInfo) (string, error) { +// GetDistinguishedName attempts to retrieve the distinguished name from ECS config +// or secrets manager. It never returns an error — when both sources fail it returns +// an empty string, allowing downstream code (ensureDistinguishedName in krb_client) +// to resolve it via LDAP. +func (h *NonDomainJoinedKerberosHandler) GetDistinguishedName(ticketInfo *types.TicketInfo) string { // Get distinguished name from ECS config - distinguishedName, err := config_utils.RetrieveVariableFromECSConfig(constants.EnvCFDistinguishedName) - if err != nil { - log.Error("Failed to retrieve distinguished name from ECS config", "error", err) - return "", fmt.Errorf("failed to retrieve distinguished name from ECS config: %v", err) + distinguishedName, ecsErr := config_utils.RetrieveVariableFromECSConfig(constants.EnvCFDistinguishedName) + if ecsErr != nil { + log.Warn("Could not retrieve distinguished name from ECS config, will try other sources", "error", ecsErr) + distinguishedName = "" } if distinguishedName == "" { // Get distinguished name from secrets manager if not found in ECS config - secretDn, err := grpc_utils.GetBaseDnFromSecret(ticketInfo.CredentialArn) - if err != nil { - log.Error("Failed to get distinguished name from secret", "error", err) - return "", fmt.Errorf("failed to get distinguished name from secret: %v", err) - } - - if secretDn != "" { + secretDn, secretErr := grpc_utils.GetBaseDnFromSecret(ticketInfo.CredentialArn) + if secretErr != nil { + log.Warn("Could not get distinguished name from secret", "error", secretErr) + } else if secretDn != "" { distinguishedName = secretDn log.Info("Retrieved distinguished name from secrets manager", "distinguished_name", distinguishedName) - } else { - log.Warn("Distinguished name not found in ECS config or secrets manager") } } - return distinguishedName, nil + if distinguishedName == "" { + log.Warn("Distinguished name not found in ECS config or secrets manager, downstream LDAP lookup will be attempted") + } + + return distinguishedName } // CleanupKerberosFiles removes the Kerberos files if there's an error diff --git a/internal/grpc/api/AddNonDomainJoinedKerberosLease_test.go b/internal/grpc/api/AddNonDomainJoinedKerberosLease_test.go index 12c5bf37..00c7f955 100644 --- a/internal/grpc/api/AddNonDomainJoinedKerberosLease_test.go +++ b/internal/grpc/api/AddNonDomainJoinedKerberosLease_test.go @@ -89,6 +89,26 @@ func TestValidateCredentials(t *testing.T) { }, expectedError: true, }, + { + name: "Blue/green username rejected by ValidateCredentials (raw colon)", + request: &pb.CreateNonDomainJoinedKerberosLeaseRequest{ + Username: "olduser:newuser", + Password: "testpassword", + Domain: "example.com", + CredspecContents: []string{"credspec1"}, + }, + expectedError: true, // ValidateCredentials rejects ':' — caller must use ParseBlueGreenUsername first + }, + { + name: "Customer rotation username SvcAccountGR:SvcAccountBL rejected by ValidateCredentials", + request: &pb.CreateNonDomainJoinedKerberosLeaseRequest{ + Username: "SvcAccountGR:SvcAccountBL", + Password: "testpassword", + Domain: "contoso.com", + CredspecContents: []string{"credspec1"}, + }, + expectedError: true, + }, } for _, tt := range tests { @@ -121,6 +141,130 @@ func TestValidateCredentials(t *testing.T) { } } +func TestAddNonDomainJoinedKerberosLease_BlueGreenUsername(t *testing.T) { + t.Run("Blue/green username - active username stored in metadata", func(t *testing.T) { + tmpDir := t.TempDir() + handler := NewNonDomainJoinedKerberosHandler(tmpDir, "test-secret", nil, nil, cmdexec.NewExecutor()) + + // ProcessCredentialSpecs should store the active (green) username, not "old:new" + credspec := `{ + "DomainJoinConfig": { + "Sid": "S-1-5-21-123456789-123456789-123456789", + "MachineAccountName": "WebApp01", + "Guid": "12345678-1234-1234-1234-123456789012", + "DnsName": "example.com", + "NetBiosName": "EXAMPLE" + }, + "ActiveDirectoryConfig": { + "GroupManagedServiceAccounts": [ + { + "Name": "WebApp01", + "Scope": "example.com" + } + ], + "HostAccountConfig": { + "PluginGUID": "12345678-1234-1234-1234-123456789012", + "PluginInput": { + "CredentialArn": "arn:aws:secretsmanager:us-west-2:123456789012:secret:example-secret" + }, + "PortableCcgVersion": "1" + } + } + }` + + // When Add is called with "olduser:newuser", only "newuser" should be passed + // to ProcessCredentialSpecs (i.e. stored as DomainlessUser in metadata). + ticketInfoList, err := handler.ProcessCredentialSpecs([]string{credspec}, "newuser", "test-lease") + assert.NoError(t, err) + assert.NotNil(t, ticketInfoList) + for _, ti := range ticketInfoList { + assert.Equal(t, "newuser", ti.DomainlessUser, "DomainlessUser should be the active (green) username only") + assert.NotContains(t, ti.DomainlessUser, ":", "DomainlessUser must not contain ':'") + } + }) + + t.Run("Customer rotation SvcAccountGR:SvcAccountBL resolves to SvcAccountBL", func(t *testing.T) { + tmpDir := t.TempDir() + handler := NewNonDomainJoinedKerberosHandler(tmpDir, "test-secret", nil, nil, cmdexec.NewExecutor()) + + credspec := `{ + "DomainJoinConfig": { + "Sid": "S-1-5-21-123456789-987654321-111222333", + "MachineAccountName": "gSvcAccount", + "Guid": "12345678-1234-1234-1234-123456789012", + "DnsName": "contoso.com", + "NetBiosName": "CORE" + }, + "ActiveDirectoryConfig": { + "GroupManagedServiceAccounts": [ + { + "Name": "gSvcAccount", + "Scope": "contoso.com" + } + ], + "HostAccountConfig": { + "PluginGUID": "{859E1386-BDB4-49E8-85C7-3070B13920E1}", + "PluginInput": { + "CredentialArn": "arn:aws:secretsmanager:us-east-1:123456789012:secret:/gmsa/SvcAccount-AbCdEf" + }, + "PortableCcgVersion": "1" + } + } + }` + + // After ParseBlueGreenUsername, only "SvcAccountBL" should reach ProcessCredentialSpecs + ticketInfoList, err := handler.ProcessCredentialSpecs([]string{credspec}, "SvcAccountBL", "test-lease") + assert.NoError(t, err) + assert.NotNil(t, ticketInfoList) + assert.Len(t, ticketInfoList, 1) + assert.Equal(t, "SvcAccountBL", ticketInfoList[0].DomainlessUser) + assert.NotContains(t, ticketInfoList[0].DomainlessUser, ":") + assert.Equal(t, "gSvcAccount", ticketInfoList[0].ServiceAccountName) + assert.Equal(t, "contoso.com", ticketInfoList[0].DomainName) + }) + + t.Run("Blue/green format with empty new username returns error", func(t *testing.T) { + tmpDir := t.TempDir() + handler := NewNonDomainJoinedKerberosHandler(tmpDir, "test-secret", nil, nil, cmdexec.NewExecutor()) + + req := &pb.CreateNonDomainJoinedKerberosLeaseRequest{ + Username: "olduser:", + Password: "testpassword", + Domain: "example.com", + CredspecContents: []string{"credspec1"}, + } + + resp, err := handler.AddNonDomainJoinedKerberosLease(context.Background(), req) + assert.Error(t, err) + assert.Nil(t, resp) + assert.Contains(t, err.Error(), "blue/green rotation format requires a non-empty new username") + }) + + t.Run("Plain username without colon still works", func(t *testing.T) { + tmpDir := t.TempDir() + handler := NewNonDomainJoinedKerberosHandler(tmpDir, "test-secret", nil, nil, cmdexec.NewExecutor()) + + // ProcessCredentialSpecs with a plain username should just store it directly + credspec := `{ + "DomainJoinConfig": { + "Sid": "S-1-5-21-123456789-123456789-123456789", + "MachineAccountName": "WebApp01", + "DnsName": "example.com" + }, + "ActiveDirectoryConfig": { + "GroupManagedServiceAccounts": [{"Name": "WebApp01", "Scope": "example.com"}], + "HostAccountConfig": {"PluginGUID": "12345678-1234-1234-1234-123456789012", "PluginInput": {"CredentialArn": "arn:aws:secretsmanager:us-west-2:123456789012:secret:test"}, "PortableCcgVersion": "1"} + } + }` + + ticketInfoList, err := handler.ProcessCredentialSpecs([]string{credspec}, "plainuser", "test-lease") + assert.NoError(t, err) + for _, ti := range ticketInfoList { + assert.Equal(t, "plainuser", ti.DomainlessUser) + } + }) +} + // Test for NewNonDomainJoinedKerberosHandler func TestNewNonDomainJoinedKerberosHandler(t *testing.T) { // Call the function with minimal dependencies diff --git a/internal/grpc/api/RenewKerberosArnLease.go b/internal/grpc/api/RenewKerberosArnLease.go index 639ed219..b0a10502 100644 --- a/internal/grpc/api/RenewKerberosArnLease.go +++ b/internal/grpc/api/RenewKerberosArnLease.go @@ -3,6 +3,7 @@ package api import ( "context" "fmt" + "time" "github.com/aws/aws-sdk-go-v2/aws" pb "golang.a2z.com/CredentialsFetcherV2/internal/grpc/proto" @@ -15,6 +16,9 @@ import ( // RenewKerberosArnLease renews Kerberos tickets using AWS credentials func (h *KerberosArnLeaseHandler) RenewKerberosArnLease(ctx context.Context, req *pb.RenewKerberosArnLeaseRequest) (*pb.RenewKerberosArnLeaseResponse, error) { + h.renewMu.Lock() + defer h.renewMu.Unlock() + log := logger.GetInstance() log.Info("Processing RenewKerberosArnLease Fargate request") @@ -40,13 +44,57 @@ func (h *KerberosArnLeaseHandler) RenewKerberosArnLease(ctx context.Context, req return response, err } - // Get and process metadata files - err = h.processMetadataFiles(ctx, cfg) - if err != nil { - response.Status = "failed" - return response, err + // Get and process metadata files with a timeout to prevent CGO/LDAP hangs. + // If the operation hangs beyond 30 seconds, wait for it to finish, then retry once. + const maxAttempts = 2 + const renewTimeout = 30 * time.Second + + type result struct{ err error } + var prevDone <-chan result + for attempt := 1; attempt <= maxAttempts; attempt++ { + // Wait for previous attempt's goroutine to avoid concurrent processMetadataFiles + if prevDone != nil { + select { + case <-prevDone: + case <-time.After(2 * renewTimeout): // Bound wait to prevent indefinite blocking + log.Warn("Previous renewal goroutine still running after extended timeout") + case <-ctx.Done(): + response.Status = "failed" + return response, ctx.Err() + } + } + + renewCtx, cancel := context.WithTimeout(ctx, renewTimeout) + done := make(chan result, 1) + go func() { + done <- result{h.processMetadataFiles(renewCtx, cfg)} + }() + + select { + case r := <-done: + cancel() + if r.err != nil { + response.Status = "failed" + return response, r.err + } + // Success + goto renewed + case <-renewCtx.Done(): + cancel() + prevDone = done // track so next iteration waits + log.Warn("RenewKerberosArnLease timed out, retrying", + "attempt", attempt, + "max_attempts", maxAttempts) + } } + // All attempts timed out + log.Error("RenewKerberosArnLease failed after all retry attempts") + response.Status = "failed" + return response, fmt.Errorf("renewal timed out after %d attempts", maxAttempts) + +renewed: + response.Status = "successful" return response, nil } @@ -121,7 +169,7 @@ func (h *KerberosArnLeaseHandler) processTicket(ctx context.Context, cfg aws.Con } // Get and validate credentials - username, password, domain, err := h.getAndValidateCredentials(ctx, cfg, krbTicketArn) + username, password, domain, distinguishedName, err := h.getAndValidateCredentials(ctx, cfg, krbTicketArn) if err != nil { return err } @@ -129,6 +177,7 @@ func (h *KerberosArnLeaseHandler) processTicket(ctx context.Context, cfg aws.Con // Update ticket info with retrieved credentials ticketInfo.DomainlessUser = username ticketInfo.DomainName = domain + ticketInfo.DistinguishedName = distinguishedName // Renew the Kerberos tickets if err := h.renewKerberosTickets(ticketInfo, username, password, domain); err != nil { @@ -168,63 +217,69 @@ func (h *KerberosArnLeaseHandler) getAndParseCredSpec(ctx context.Context, cfg a } // getAndValidateCredentials retrieves and validates credentials from Secrets Manager -func (h *KerberosArnLeaseHandler) getAndValidateCredentials(ctx context.Context, cfg aws.Config, krbTicketArn *types.KerberosTicketArnMapping) (string, string, string, error) { +func (h *KerberosArnLeaseHandler) getAndValidateCredentials(ctx context.Context, cfg aws.Config, krbTicketArn *types.KerberosTicketArnMapping) (string, string, string, string, error) { log := logger.GetInstance() // Get secrets ARN secretsArn := krbTicketArn.CredentialDomainlessUserArn if secretsArn == "" { log.Error("Invalid Secrets Manager ARN") - return "", "", "", fmt.Errorf("invalid secrets manager ARN") + return "", "", "", "", fmt.Errorf("invalid secrets manager ARN") } // Retrieve credentials from Secrets Manager secretMap, err := aws_utils.GetSecretFromSecretsManagerWithConfig(ctx, cfg, secretsArn) if err != nil { log.Error("Failed to retrieve credentials from secrets manager", "error", err) - return "", "", "", fmt.Errorf("failed to retrieve credentials from secrets manager: %w", err) + return "", "", "", "", fmt.Errorf("failed to retrieve credentials from secrets manager: %w", err) } // Extract credentials from the secret - username, password, domain, _, err := aws_utils.ExtractCredentialsFromSecret(secretMap) + username, password, domain, distinguishedName, err := aws_utils.ExtractCredentialsFromSecret(secretMap) if err != nil { log.Error("Failed to extract credentials from secret", "error", err) - return "", "", "", fmt.Errorf("failed to extract credentials from secret: %w", err) + return "", "", "", "", fmt.Errorf("failed to extract credentials from secret: %w", err) } // Validate domain and username if !aws_utils.IsValidDomain(domain) || aws_utils.ContainsInvalidCharactersInADAccountName(username) { log.Error("Invalid domainName/username") - return "", "", "", fmt.Errorf("invalid domainName/username") + return "", "", "", "", fmt.Errorf("invalid domainName/username") } // Validate credential lengths if !h.validateCredentials(username, password, domain) { - return "", "", "", fmt.Errorf("invalid credentials") + return "", "", "", "", fmt.Errorf("invalid credentials") } - return username, password, domain, nil + return username, password, domain, distinguishedName, nil } // renewKerberosTickets creates/renews Kerberos tickets for the user and gMSA account func (h *KerberosArnLeaseHandler) renewKerberosTickets(ticketInfo *types.TicketInfo, username, password, domain string) error { log := logger.GetInstance() - // Generate Kerberos ticket using username and password - if err := h.krbClient.CreateTicketUsingUsernamePassword( - domain, - username, - password, - ); err != nil { - log.Error("Failed to generate Kerberos ticket for domainless user", "error", err) - return fmt.Errorf("failed to generate Kerberos ticket for domainless user: %w", err) - } + const numRetries = 1 + for i := 0; i <= numRetries; i++ { + // Generate Kerberos ticket using username and password (user TGT) + if err := h.krbClient.CreateTicketUsingUsernamePassword( + domain, + username, + password, + ); err != nil { + log.Error("Failed to generate Kerberos ticket for domainless user", "error", err, "attempt", i+1) + return fmt.Errorf("failed to generate Kerberos ticket for domainless user: %w", err) + } - // Create ticket for gMSA account - if err := h.krbClient.CreateTicketForGMSA(ticketInfo); err != nil { - log.Error("Failed to create gMSA ticket", "error", err) - return fmt.Errorf("failed to create gMSA ticket: %w", err) + // Create ticket for gMSA account (LDAP search + ticket creation) + if err := h.krbClient.CreateTicketForGMSA(ticketInfo); err != nil { + log.Error("Failed to create gMSA ticket, will regenerate user TGT and retry", + "error", err, "attempt", i+1, "max_attempts", numRetries+1) + continue + } + + return nil } - return nil + return fmt.Errorf("failed to create gMSA ticket after %d attempts", numRetries+1) } diff --git a/internal/grpc/api/RenewNonDomainJoinedKerberosLease.go b/internal/grpc/api/RenewNonDomainJoinedKerberosLease.go index 6b564515..92a5b505 100644 --- a/internal/grpc/api/RenewNonDomainJoinedKerberosLease.go +++ b/internal/grpc/api/RenewNonDomainJoinedKerberosLease.go @@ -3,8 +3,10 @@ package api import ( "context" "fmt" + "strings" "golang.a2z.com/CredentialsFetcherV2/internal/utils/aws_utils" + "golang.a2z.com/CredentialsFetcherV2/internal/utils/grpc_utils" "golang.a2z.com/CredentialsFetcherV2/internal/utils/metadata_utils" "golang.a2z.com/CredentialsFetcherV2/internal/utils/types" @@ -30,10 +32,34 @@ func (h *NonDomainJoinedKerberosHandler) RenewNonDomainJoinedKerberosLease(ctx c grpc_utils.SecureClearString(&req.Password) }() - // Validate request - if err := h.ValidateCredentials(req.Username, req.Password, req.Domain); err != nil { + // Parse blue/green username rotation format ("oldUser:newUser"). + // When usernames are rotated in AWS Secrets Manager, the caller supplies + // the old and new usernames separated by ':' so we can match existing + // tickets (by the old name) and recreate them with the new credentials. + matchUsername, activeUsername, isRotation := grpc_utils.ParseBlueGreenUsername(req.Username) + if isRotation { + if matchUsername == "" || activeUsername == "" { + return nil, fmt.Errorf("blue/green rotation format requires both old and new usernames (\"oldUser:newUser\")") + } + if matchUsername == activeUsername { + // Same username on both sides — treat as a normal renewal. + isRotation = false + } else { + log.Info("Detected username with colon separator", + "match_username", matchUsername, "active_username", activeUsername) + } + } + + // Validate each username part individually (the raw "old:new" string + // would fail ValidateAccountName because ':' is invalid in AD usernames) + if err := h.ValidateCredentials(activeUsername, req.Password, req.Domain); err != nil { return nil, err } + if isRotation { + if err := grpc_utils.ValidateAccountName(matchUsername); err != nil { + return nil, fmt.Errorf("invalid old username: %v", err) + } + } // Get all metadata files from the Kerberos files directory metadataFiles, err := metadata_utils.GetMetadataFilePaths(h.krbFilesDir) @@ -50,6 +76,14 @@ func (h *NonDomainJoinedKerberosHandler) RenewNonDomainJoinedKerberosLease(ctx c var renewedKrbFilePaths []string var ticketsToRecreate []*types.TicketInfo + // Pending metadata updates for rotation — written only after successful + // ticket recreation to avoid inconsistent state on failure. + type pendingMetadataUpdate struct { + path string + ticketInfoList []*types.TicketInfo + } + var pendingUpdates []pendingMetadataUpdate + // Process each metadata file for _, metadataPath := range metadataFiles { // Read ticket info from metadata file @@ -59,17 +93,23 @@ func (h *NonDomainJoinedKerberosHandler) RenewNonDomainJoinedKerberosLease(ctx c continue // Skip this file and try the next one } - // Filter ticket infos that match the provided username + // Filter ticket infos that match the provided (or old/blue) username var matchingTicketInfos []*types.TicketInfo for _, ticketInfo := range ticketInfoList { - // First, try direct match with DomainlessUser - if ticketInfo.DomainlessUser == req.Username { + if ticketInfo.DomainlessUser == matchUsername { + matchingTicketInfos = append(matchingTicketInfos, ticketInfo) + continue + } + + // If rotation is active and the ticket already has the new (green) username, + // the rotation was already completed — treat as a normal renewal match. + if isRotation && ticketInfo.DomainlessUser == activeUsername { matchingTicketInfos = append(matchingTicketInfos, ticketInfo) continue } - // If no direct match and CredentialArn is available, extract username from the secret - if ticketInfo.CredentialArn != "" { + // If DomainlessUser is empty and CredentialArn is available, extract username from the secret + if ticketInfo.DomainlessUser == "" && ticketInfo.CredentialArn != "" { secretMap, err := aws_utils.GetSecretFromSecretsManagerWithContext(ctx, ticketInfo.CredentialArn) if err != nil { log.Warn("Failed to retrieve secret from CredentialArn", "arn", ticketInfo.CredentialArn, "error", err) @@ -82,8 +122,7 @@ func (h *NonDomainJoinedKerberosHandler) RenewNonDomainJoinedKerberosLease(ctx c continue } - // Check if the extracted username matches the request username - if username == req.Username { + if username == matchUsername || (isRotation && username == activeUsername) { matchingTicketInfos = append(matchingTicketInfos, ticketInfo) log.Info("Matched ticket via CredentialArn username", "username", username, "arn", ticketInfo.CredentialArn) } @@ -91,21 +130,66 @@ func (h *NonDomainJoinedKerberosHandler) RenewNonDomainJoinedKerberosLease(ctx c } if len(matchingTicketInfos) == 0 { - log.Info("No matching tickets found for user", "username", req.Username, "metadata_path", metadataPath) + log.Info("No matching tickets found for user", + "match_username", matchUsername, "metadata_path", metadataPath) continue // Skip to next metadata file } - // First try to renew each matching ticket directly using krbClient.RenewKerberosTicket - for _, ticketInfo := range matchingTicketInfos { - err = h.krbClient.RenewKerberosTicket(ctx, ticketInfo.KrbFilePath) - if err == nil { - // Successfully renewed the ticket - renewedKrbFilePaths = append(renewedKrbFilePaths, ticketInfo.KrbFilePath) - log.Info("Successfully renewed Kerberos ticket directly", "path", ticketInfo.KrbFilePath) - } else { - log.Warn("Direct renewal failed, will recreate the ticket", "error", err, "path", ticketInfo.KrbFilePath) - // Add to list of tickets that need recreation - ticketsToRecreate = append(ticketsToRecreate, ticketInfo) + // Determine if this is an actual rotation or if rotation was already completed. + // If all matched tickets already have the active username, treat as normal renewal. + needsRotation := false + if isRotation { + for _, ticketInfo := range matchingTicketInfos { + if ticketInfo.DomainlessUser == matchUsername { + needsRotation = true + break + } + } + } + + // When rotating usernames, only tickets that still have the old username + // need recreation. Tickets already carrying the active username are renewed + // normally — this handles the mixed-state case where some tickets in a + // metadata file were already rotated. + if needsRotation { + log.Info("Blue/green username rotation detected", + "match_username", matchUsername, "active_username", activeUsername) + for _, ticketInfo := range matchingTicketInfos { + if ticketInfo.DomainlessUser == matchUsername { + // Update DomainlessUser to the new (green) username so that + // subsequent renewals and metadata lookups use the new name. + ticketInfo.DomainlessUser = activeUsername + ticketsToRecreate = append(ticketsToRecreate, ticketInfo) + } else { + // Already rotated — renew normally + err = h.krbClient.RenewKerberosTicket(ctx, ticketInfo.KrbFilePath) + if err == nil { + renewedKrbFilePaths = append(renewedKrbFilePaths, ticketInfo.KrbFilePath) + log.Info("Successfully renewed already-rotated ticket directly", "path", ticketInfo.KrbFilePath) + } else { + log.Warn("Direct renewal failed for already-rotated ticket, will recreate", + "error", err, "path", ticketInfo.KrbFilePath) + ticketsToRecreate = append(ticketsToRecreate, ticketInfo) + } + } + } + // Queue metadata write — deferred until after successful ticket recreation + // to avoid inconsistent state if CreateKerberosTickets fails. + pendingUpdates = append(pendingUpdates, pendingMetadataUpdate{metadataPath, ticketInfoList}) + log.Info("Username rotation: tickets queued for recreation", + "recreate_count", len(ticketsToRecreate), "already_renewed", len(renewedKrbFilePaths)) + } else { + // Normal (non-rotation) path: try direct renewal first + for _, ticketInfo := range matchingTicketInfos { + err = h.krbClient.RenewKerberosTicket(ctx, ticketInfo.KrbFilePath) + if err == nil { + renewedKrbFilePaths = append(renewedKrbFilePaths, ticketInfo.KrbFilePath) + log.Info("Successfully renewed Kerberos ticket directly", "path", ticketInfo.KrbFilePath) + } else { + log.Warn("Direct renewal failed, will recreate the ticket", + "error", err, "path", ticketInfo.KrbFilePath) + ticketsToRecreate = append(ticketsToRecreate, ticketInfo) + } } } } @@ -114,8 +198,8 @@ func (h *NonDomainJoinedKerberosHandler) RenewNonDomainJoinedKerberosLease(ctx c if len(ticketsToRecreate) > 0 { log.Info("Attempting to recreate tickets that couldn't be renewed directly", "count", len(ticketsToRecreate)) - // Use CreateKerberosTickets to recreate the tickets - recreatedPaths, err := h.CreateKerberosTickets(ctx, req.Domain, req.Username, req.Password, ticketsToRecreate) + // Use the active (green) username for ticket creation + recreatedPaths, err := h.CreateKerberosTickets(ctx, req.Domain, activeUsername, req.Password, ticketsToRecreate) if err != nil { log.Error("Failed to recreate Kerberos tickets", "error", err) // If we have some successfully renewed tickets, return those @@ -131,6 +215,26 @@ func (h *NonDomainJoinedKerberosHandler) RenewNonDomainJoinedKerberosLease(ctx c // Add recreated paths to renewed paths renewedKrbFilePaths = append(renewedKrbFilePaths, recreatedPaths...) log.Info("Successfully recreated Kerberos tickets", "count", len(recreatedPaths)) + + // Persist rotation metadata only after tickets were successfully recreated. + // This ensures on-disk metadata stays consistent with actual ticket state. + // Attempt all writes and collect errors so the caller gets an accurate + // picture (some metadata files may have been updated successfully). + var metadataErrors []string + for _, pu := range pendingUpdates { + if err := metadata_utils.UpdateMetadataJSON(pu.path, pu.ticketInfoList); err != nil { + metadataErrors = append(metadataErrors, fmt.Sprintf("%s: %v", pu.path, err)) + log.Error("Failed to persist username rotation to metadata", "path", pu.path, "error", err) + } + } + if len(metadataErrors) > 0 { + // Return the successfully renewed paths alongside the error + // so the caller knows which tickets were actually recreated. + return &pb.RenewNonDomainJoinedKerberosLeaseResponse{ + RenewedKerberosFilePaths: renewedKrbFilePaths, + }, fmt.Errorf("metadata persistence failed for %d file(s): %s", + len(metadataErrors), strings.Join(metadataErrors, "; ")) + } } if len(renewedKrbFilePaths) == 0 { diff --git a/internal/grpc/api/RenewNonDomainJoinedKerberosLease_test.go b/internal/grpc/api/RenewNonDomainJoinedKerberosLease_test.go index 87bec6d5..ad3d542c 100644 --- a/internal/grpc/api/RenewNonDomainJoinedKerberosLease_test.go +++ b/internal/grpc/api/RenewNonDomainJoinedKerberosLease_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" "golang.a2z.com/CredentialsFetcherV2/internal/auth/kerberos" "golang.a2z.com/CredentialsFetcherV2/internal/utils/cmdexec" + "golang.a2z.com/CredentialsFetcherV2/internal/utils/grpc_utils" ) // TestRenewNonDomainJoinedKerberosLease_ValidateCredentials tests the validation of credentials @@ -71,3 +72,251 @@ func TestRenewNonDomainJoinedKerberosLease_ValidateCredentials(t *testing.T) { }) } } + +// TestRenewSkipsSecretsManagerFallbackWhenDomainlessUserSet verifies that when +// DomainlessUser is populated in metadata but doesn't match the request username, +// the code does NOT fall back to reading the secret from Secrets Manager. +// This is critical for ECS mode where the instance role doesn't have +// secretsmanager:GetSecretValue permission. +func TestRenewSkipsSecretsManagerFallbackWhenDomainlessUserSet(t *testing.T) { + // Simulate ticket metadata with DomainlessUser set to a different user + // than the one in the renew request. The code should skip this ticket + // without attempting to read CredentialArn from Secrets Manager. + + testCases := []struct { + name string + domainlessUser string + credentialArn string + matchUsername string + shouldMatch bool + shouldCallSecret bool + }{ + { + name: "DomainlessUser set and matches - direct match, no secret call", + domainlessUser: "StandardUser01", + credentialArn: "arn:aws:secretsmanager:us-west-2:123456789012:secret:test", + matchUsername: "StandardUser01", + shouldMatch: true, + shouldCallSecret: false, + }, + { + name: "DomainlessUser set but doesn't match - skip, no secret call", + domainlessUser: "StandardUser02", + credentialArn: "arn:aws:secretsmanager:us-west-2:123456789012:secret:test", + matchUsername: "StandardUser01", + shouldMatch: false, + shouldCallSecret: false, + }, + { + name: "DomainlessUser empty, CredentialArn set - should attempt secret call", + domainlessUser: "", + credentialArn: "arn:aws:secretsmanager:us-west-2:123456789012:secret:test", + matchUsername: "StandardUser01", + shouldMatch: false, + shouldCallSecret: true, + }, + { + name: "DomainlessUser empty, CredentialArn empty - no match, no secret call", + domainlessUser: "", + credentialArn: "", + matchUsername: "StandardUser01", + shouldMatch: false, + shouldCallSecret: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Test the matching logic directly + matched := tc.domainlessUser == tc.matchUsername + assert.Equal(t, tc.shouldMatch, matched, "Direct match result") + + // Test whether Secrets Manager fallback would be triggered + wouldCallSecret := tc.domainlessUser == "" && tc.credentialArn != "" + assert.Equal(t, tc.shouldCallSecret, wouldCallSecret, + "Secrets Manager fallback should only trigger when DomainlessUser is empty and CredentialArn is set") + }) + } +} + +// TestBlueGreenRenewalAfterRotationCompleted verifies that when the secret still +// has oldUser:newUser format but tickets have already been rotated to newUser, +// renewal continues to work by matching the active username. +func TestBlueGreenRenewalAfterRotationCompleted(t *testing.T) { + testCases := []struct { + name string + username string // from GRPC request (secret value) + domainlessUser string // in metadata + shouldMatch bool + needsRotation bool + }{ + { + name: "Rotation needed - ticket has old username", + username: "StandardUser01:StandardUser02", + domainlessUser: "StandardUser01", + shouldMatch: true, + needsRotation: true, + }, + { + name: "Rotation already done - ticket has new username", + username: "StandardUser01:StandardUser02", + domainlessUser: "StandardUser02", + shouldMatch: true, + needsRotation: false, + }, + { + name: "No rotation - single username matches", + username: "StandardUser01", + domainlessUser: "StandardUser01", + shouldMatch: true, + needsRotation: false, + }, + { + name: "No rotation - single username doesn't match", + username: "StandardUser01", + domainlessUser: "StandardUser02", + shouldMatch: false, + needsRotation: false, + }, + { + name: "Rotation format - neither old nor new matches", + username: "StandardUser01:StandardUser02", + domainlessUser: "StandardUser03", + shouldMatch: false, + needsRotation: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + matchUsername, activeUsername, isRotation := grpc_utils.ParseBlueGreenUsername(tc.username) + + // Simulate the matching logic + matched := tc.domainlessUser == matchUsername || + (isRotation && tc.domainlessUser == activeUsername) + assert.Equal(t, tc.shouldMatch, matched, "Ticket match result") + + // Simulate needsRotation determination + needsRotation := isRotation && matched && tc.domainlessUser == matchUsername + assert.Equal(t, tc.needsRotation, needsRotation, "Needs rotation") + + // Suppress unused variable warnings + _ = activeUsername + }) + } +} + +// TestBlueGreenMixedStateRotation verifies that in a mixed state where some +// tickets have the old username and some have the new username, the production +// matching logic correctly classifies each ticket. +func TestBlueGreenMixedStateRotation(t *testing.T) { + username := "StandardUser01:StandardUser02" + matchUsername, activeUsername, isRotation := grpc_utils.ParseBlueGreenUsername(username) + + assert.True(t, isRotation) + assert.Equal(t, "StandardUser01", matchUsername) + assert.Equal(t, "StandardUser02", activeUsername) + + tickets := []struct { + domainlessUser string + expectMatch bool + expectRotate bool // true = needs recreation, false = renew normally + }{ + {"StandardUser01", true, true}, // old username → needs rotation + {"StandardUser02", true, false}, // already has active username → renew normally + {"StandardUser01", true, true}, // old username → needs rotation + {"StandardUser02", true, false}, // already has active username → renew normally + {"StandardUser03", false, false}, // unrelated → no match + } + + var matchCount, rotateCount, renewCount int + for _, ticket := range tickets { + // Use the same matching logic as production code + matched := ticket.domainlessUser == matchUsername || + (isRotation && ticket.domainlessUser == activeUsername) + assert.Equal(t, ticket.expectMatch, matched, "Match for %s", ticket.domainlessUser) + + if matched { + matchCount++ + // Production logic: only rotate if DomainlessUser == matchUsername + if ticket.domainlessUser == matchUsername { + rotateCount++ + assert.True(t, ticket.expectRotate) + } else { + renewCount++ + assert.False(t, ticket.expectRotate) + } + } + } + + assert.Equal(t, 4, matchCount, "4 tickets should match (2 old + 2 active)") + assert.Equal(t, 2, rotateCount, "Only old-username tickets need rotation") + assert.Equal(t, 2, renewCount, "Active-username tickets renewed normally") +} + +// TestBlueGreenUsernameParsingInRenewFlow tests that the blue/green username +// rotation format ("oldUser:newUser") used during Secrets Manager credential +// rotation is properly parsed and validated in the renew flow. +func TestBlueGreenUsernameParsingInRenewFlow(t *testing.T) { + handler := &NonDomainJoinedKerberosHandler{ + krbFilesDir: "/tmp/krb", + awsSMSecretName: "test-secret", + krbClient: kerberos.NewClient(), + shellExecutor: cmdexec.NewExecutor(), + } + + t.Run("Blue/green format - both usernames valid", func(t *testing.T) { + matchUser, activeUser, isRotation := grpc_utils.ParseBlueGreenUsername("olduser:newuser") + assert.True(t, isRotation) + assert.Equal(t, "olduser", matchUser) + assert.Equal(t, "newuser", activeUser) + + // Both parts should pass individual validation + assert.NoError(t, handler.ValidateCredentials(activeUser, "password", "example.com")) + assert.NoError(t, grpc_utils.ValidateAccountName(matchUser)) + }) + + t.Run("Customer rotation SvcAccountGR:SvcAccountBL", func(t *testing.T) { + matchUser, activeUser, isRotation := grpc_utils.ParseBlueGreenUsername("SvcAccountGR:SvcAccountBL") + assert.True(t, isRotation) + assert.Equal(t, "SvcAccountGR", matchUser) + assert.Equal(t, "SvcAccountBL", activeUser) + + // Active (new) username passes validation + assert.NoError(t, handler.ValidateCredentials(activeUser, "password", "contoso.com")) + // Old username also passes validation independently + assert.NoError(t, grpc_utils.ValidateAccountName(matchUser)) + + // Raw combined string must NOT pass ValidateAccountName (contains ':') + err := grpc_utils.ValidateAccountName("SvcAccountGR:SvcAccountBL") + assert.Error(t, err) + assert.Contains(t, err.Error(), "username contains invalid character: :") + }) + + t.Run("Blue/green format - new username invalid", func(t *testing.T) { + _, activeUser, isRotation := grpc_utils.ParseBlueGreenUsername("olduser:invalid user") + assert.True(t, isRotation) + + // Active (new) username has a space → should fail validation + err := handler.ValidateCredentials(activeUser, "password", "example.com") + assert.Error(t, err) + }) + + t.Run("Blue/green format - old username invalid", func(t *testing.T) { + matchUser, _, isRotation := grpc_utils.ParseBlueGreenUsername("old user:newuser") + assert.True(t, isRotation) + + // Old username has a space → should fail validation + err := grpc_utils.ValidateAccountName(matchUser) + assert.Error(t, err) + }) + + t.Run("Normal username (no colon) - passes validation", func(t *testing.T) { + matchUser, activeUser, isRotation := grpc_utils.ParseBlueGreenUsername("singleuser") + assert.False(t, isRotation) + assert.Equal(t, "singleuser", matchUser) + assert.Equal(t, "singleuser", activeUser) + + assert.NoError(t, handler.ValidateCredentials(activeUser, "password", "example.com")) + }) +} diff --git a/internal/grpc/server_test.go b/internal/grpc/server_test.go index afdb9f5d..eb355a4a 100644 --- a/internal/grpc/server_test.go +++ b/internal/grpc/server_test.go @@ -146,6 +146,69 @@ func TestCredentialsFetcherServer_AddNonDomainJoinedKerberosLease(t *testing.T) assert.Nil(t, resp) } +func TestCredentialsFetcherServer_AddNonDomainJoinedKerberosLease_BlueGreenUsername(t *testing.T) { + // Setup server + conn, _, cleanup := setupGrpcServer(t) + defer cleanup() + + // Create client + client := pb.NewCredentialsFetcherServiceClient(conn) + + t.Run("Customer rotation SvcAccountGR:SvcAccountBL passes validation", func(t *testing.T) { + req := &pb.CreateNonDomainJoinedKerberosLeaseRequest{ + CredspecContents: []string{`{"DomainJoinConfig":{"Sid":"S-1-5-21-123456789-987654321-111222333","MachineAccountName":"gSvcAccount","Guid":"12345678-1234-1234-1234-123456789012","DnsName":"contoso.com","NetBiosName":"CORE"},"ActiveDirectoryConfig":{"GroupManagedServiceAccounts":[{"Name":"gSvcAccount","Scope":"contoso.com"}],"HostAccountConfig":{"PluginGUID":"{859E1386-BDB4-49E8-85C7-3070B13920E1}","PluginInput":{"CredentialArn":"arn:aws:secretsmanager:us-east-1:123456789012:secret:/gmsa/SvcAccount-AbCdEf"},"PortableCcgVersion":"1"}}}`}, + Username: "SvcAccountGR:SvcAccountBL", + Password: "test-password", + Domain: "contoso.com", + } + resp, err := client.AddNonDomainJoinedKerberosLease(context.Background(), req) + + // The request should get past username validation (no "invalid character" error). + // It will fail later at Kerberos ticket creation since we have no real KDC, + // but the important thing is it does NOT fail with "username contains invalid character: :" + if err != nil { + assert.NotContains(t, err.Error(), "username contains invalid character") + assert.NotContains(t, err.Error(), "invalid username") + } else { + assert.NotNil(t, resp) + } + }) + + t.Run("Empty new username in blue/green format returns error", func(t *testing.T) { + req := &pb.CreateNonDomainJoinedKerberosLeaseRequest{ + CredspecContents: []string{"credspec"}, + Username: "SvcAccountGR:", + Password: "test-password", + Domain: "contoso.com", + } + resp, err := client.AddNonDomainJoinedKerberosLease(context.Background(), req) + assert.Error(t, err) + assert.Contains(t, err.Error(), "blue/green rotation format requires a non-empty new username") + assert.Nil(t, resp) + }) +} + +func TestCredentialsFetcherServer_RenewNonDomainJoinedKerberosLease_BlueGreenUsername(t *testing.T) { + conn, _, cleanup := setupGrpcServer(t) + defer cleanup() + + client := pb.NewCredentialsFetcherServiceClient(conn) + + req := &pb.RenewNonDomainJoinedKerberosLeaseRequest{ + Username: "SvcAccountGR:SvcAccountBL", + Password: "test-password", + Domain: "contoso.com", + } + _, err := client.RenewNonDomainJoinedKerberosLease(context.Background(), req) + + // Should NOT fail with "username contains invalid character" — the colon is parsed out. + // Will fail with "no metadata files found" since no tickets exist in the temp dir. + assert.Error(t, err) + assert.NotContains(t, err.Error(), "username contains invalid character") + assert.NotContains(t, err.Error(), "invalid username") + assert.Contains(t, err.Error(), "no metadata files found") +} + func TestCredentialsFetcherServer_RenewNonDomainJoinedKerberosLease(t *testing.T) { // Setup server conn, _, cleanup := setupGrpcServer(t) diff --git a/internal/logger/logger.go b/internal/logger/logger.go index cbd5a5e0..e0e9e593 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -1,10 +1,12 @@ package logger import ( + "fmt" "io" "log/slog" "os" "sync" + "time" "golang.a2z.com/CredentialsFetcherV2/constants" ) @@ -21,7 +23,9 @@ type Logger interface { // logger implements the Logger interface type logger struct { *slog.Logger - logFile *os.File + logFile *os.File + done chan struct{} + closeOnce sync.Once } var ( @@ -58,24 +62,35 @@ func newLogger() Logger { // Setup log file logFile, err := setupLogFile() - var writer io.Writer = os.Stdout + var writer io.Writer if err != nil { - // Log error to stdout and continue with stdout-only logging - slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: logLevel})). - Error("Failed to setup log file, continuing with stdout-only logging", "error", err) + // Log error to stderr and fall back to stderr-only logging + slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: logLevel})). + Error("Failed to setup log file, continuing with stderr-only logging", "error", err) + writer = os.Stderr } else if logFile != nil { - // Create MultiWriter for dual output - writer = io.MultiWriter(os.Stdout, logFile) + // Write to log file and stderr (stderr is safe for containers; stdout causes pipe deadlock) + writer = io.MultiWriter(os.Stderr, logFile) + } else { + writer = os.Stderr } handler := slog.NewTextHandler(writer, &slog.HandlerOptions{ Level: logLevel, }) - return &logger{ + l := &logger{ Logger: slog.New(handler), logFile: logFile, + done: make(chan struct{}), } + + // Start periodic log rotation check + if logFile != nil { + go l.logRotationLoop() + } + + return l } // setupLogFile creates the log directory and opens the log file @@ -86,6 +101,9 @@ func setupLogFile() (*os.File, error) { return nil, err } + // Truncate log file if it exceeds max size (10 MB) + truncateLogFileIfNeeded(constants.LogFilePath) + // Open/create log file with append mode and 0644 permissions logFile, err := os.OpenFile(constants.LogFilePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) // #nosec G302 if err != nil { @@ -95,6 +113,24 @@ func setupLogFile() (*os.File, error) { return logFile, nil } +// maxLogFileSize is the maximum log file size before truncation (10 MB) +const maxLogFileSize = 10 * 1024 * 1024 + +// truncateLogFileIfNeeded truncates the log file if it exceeds maxLogFileSize. +// This matches the behavior of the C++ credentials-fetcher (v1.3.8). +func truncateLogFileIfNeeded(path string) { + info, err := os.Stat(path) + if err != nil { + return // file doesn't exist yet, nothing to truncate + } + if info.Size() > maxLogFileSize { + if err := os.Truncate(path, 0); err != nil { + // Log to stderr since the log file itself may be the problem + fmt.Fprintf(os.Stderr, "credentials-fetcher: failed to truncate log file %s: %v\n", path, err) + } + } +} + func (l *logger) Debug(msg string, args ...any) { l.Logger.Debug(msg, args...) } @@ -113,8 +149,27 @@ func (l *logger) Error(msg string, args ...any) { // Close closes the log file if it's open func (l *logger) Close() error { + l.closeOnce.Do(func() { + if l.done != nil { + close(l.done) + } + }) if l.logFile != nil { return l.logFile.Close() } return nil } + +// logRotationLoop periodically checks log file size and truncates if needed +func (l *logger) logRotationLoop() { + ticker := time.NewTicker(5 * time.Minute) + defer ticker.Stop() + for { + select { + case <-ticker.C: + truncateLogFileIfNeeded(constants.LogFilePath) + case <-l.done: + return + } + } +} diff --git a/internal/logger/logger_test.go b/internal/logger/logger_test.go index 9b05a5ab..44a8ff6f 100644 --- a/internal/logger/logger_test.go +++ b/internal/logger/logger_test.go @@ -157,9 +157,9 @@ func TestLogLevelFromEnvironment(t *testing.T) { } // Capture stdout - oldStdout := os.Stdout + oldStderr := os.Stderr r, w, _ := os.Pipe() - os.Stdout = w + os.Stderr = w // Create logger and log a message log := GetInstance() @@ -174,7 +174,7 @@ func TestLogLevelFromEnvironment(t *testing.T) { if err := w.Close(); err != nil { t.Fatalf("Failed to close writer: %v", err) } - os.Stdout = oldStdout + os.Stderr = oldStderr // Read captured output var buf bytes.Buffer @@ -215,7 +215,7 @@ func TestLogLevelFromEnvironment(t *testing.T) { } } -// TestDualOutputProperty tests that log messages appear in both stdout and file +// TestDualOutputProperty tests that log messages appear in both stderr and file func TestDualOutputProperty(t *testing.T) { tempDir := t.TempDir() testLogFile := filepath.Join(tempDir, "test.log") @@ -225,9 +225,9 @@ func TestDualOutputProperty(t *testing.T) { defer func() { _ = logFile.Close() }() // Capture stdout - oldStdout := os.Stdout + oldStderr := os.Stderr stdoutReader, stdoutWriter, _ := os.Pipe() - os.Stdout = stdoutWriter + os.Stderr = stdoutWriter // Create MultiWriter multiWriter := io.MultiWriter(stdoutWriter, logFile) @@ -237,7 +237,7 @@ func TestDualOutputProperty(t *testing.T) { testLogger.Info("test message", "key", "value") _ = stdoutWriter.Close() - os.Stdout = oldStdout + os.Stderr = oldStderr // Read stdout output var stdoutBuf bytes.Buffer @@ -338,3 +338,59 @@ func TestFileLoggingAppendMode(t *testing.T) { assert.Contains(t, string(content), "initial log") assert.Contains(t, string(content), "appended log") } + +// TestTruncateLogFileIfNeeded tests log file truncation at max size +func TestTruncateLogFileIfNeeded(t *testing.T) { + t.Run("File under max size is not truncated", func(t *testing.T) { + tempDir := t.TempDir() + testLogFile := filepath.Join(tempDir, "test.log") + + // Write 1 MB of data + data := make([]byte, 1*1024*1024) + err := os.WriteFile(testLogFile, data, 0644) + require.NoError(t, err) + + truncateLogFileIfNeeded(testLogFile) + + info, err := os.Stat(testLogFile) + require.NoError(t, err) + assert.Equal(t, int64(1*1024*1024), info.Size(), "File should not be truncated") + }) + + t.Run("File over max size is truncated", func(t *testing.T) { + tempDir := t.TempDir() + testLogFile := filepath.Join(tempDir, "test.log") + + // Write 11 MB of data (over 10 MB limit) + data := make([]byte, 11*1024*1024) + err := os.WriteFile(testLogFile, data, 0644) + require.NoError(t, err) + + truncateLogFileIfNeeded(testLogFile) + + info, err := os.Stat(testLogFile) + require.NoError(t, err) + assert.Equal(t, int64(0), info.Size(), "File should be truncated to 0") + }) + + t.Run("Exactly at max size is not truncated", func(t *testing.T) { + tempDir := t.TempDir() + testLogFile := filepath.Join(tempDir, "test.log") + + // Write exactly 10 MB + data := make([]byte, maxLogFileSize) + err := os.WriteFile(testLogFile, data, 0644) + require.NoError(t, err) + + truncateLogFileIfNeeded(testLogFile) + + info, err := os.Stat(testLogFile) + require.NoError(t, err) + assert.Equal(t, int64(maxLogFileSize), info.Size(), "File at exact limit should not be truncated") + }) + + t.Run("Non-existent file does not error", func(t *testing.T) { + // Should not panic or error + truncateLogFileIfNeeded("/tmp/nonexistent-log-file-xyz.log") + }) +} diff --git a/internal/utils/aws_utils/aws_utils.go b/internal/utils/aws_utils/aws_utils.go index 374894e9..18597849 100644 --- a/internal/utils/aws_utils/aws_utils.go +++ b/internal/utils/aws_utils/aws_utils.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/service/secretsmanager" + "golang.a2z.com/CredentialsFetcherV2/constants" "golang.a2z.com/CredentialsFetcherV2/internal/logger" "golang.a2z.com/CredentialsFetcherV2/internal/utils/types" ) @@ -173,11 +174,11 @@ func IsValidDomain(domain string) bool { return len(parts) >= 2 } -// ContainsInvalidCharacters checks if a string contains invalid characters +// ContainsInvalidCharacters checks if a string contains invalid characters for AD sAMAccountName func ContainsInvalidCharacters(s string, logMessage string) bool { log := logger.GetInstance() - for _, char := range types.InvalidCharacters { - if strings.ContainsRune(s, char) { + for _, char := range s { + if strings.ContainsRune(constants.InvalidSAMAccountNameChars, char) { log.Error("Contains invalid credentials in ", logMessage) return true } @@ -190,9 +191,20 @@ func ContainsInvalidCharactersInADAccountName(username string) bool { return ContainsInvalidCharacters(username, "AD account name") } -// ContainsInvalidCharactersInCredentialSpec checks if a string contains invalid characters +// ContainsInvalidCharactersInCredentialSpec checks if a credential spec path contains invalid characters func ContainsInvalidCharactersInCredentialSpec(s string) bool { - return ContainsInvalidCharacters(s, "credential spec path") + log := logger.GetInstance() + if strings.Contains(s, "..") { + log.Error("Contains path traversal sequence in ", "credential spec path") + return true + } + for _, char := range types.InvalidCredSpecPathChars { + if strings.ContainsRune(s, char) { + log.Error("Contains invalid credentials in ", "credential spec path") + return true + } + } + return false } // parseRegionFromARN extracts the region from an AWS ARN diff --git a/internal/utils/aws_utils/aws_utils_test.go b/internal/utils/aws_utils/aws_utils_test.go index dcb34faa..806d103d 100644 --- a/internal/utils/aws_utils/aws_utils_test.go +++ b/internal/utils/aws_utils/aws_utils_test.go @@ -314,3 +314,50 @@ func TestGetSecretWithClient(t *testing.T) { }) } } + +func TestContainsInvalidCharactersInADAccountName(t *testing.T) { + tests := []struct { + name string + username string + want bool + }{ + {"Valid simple username", "webadmin", false}, + {"Valid username with period", "svc.myapp", false}, + {"Valid username with hyphen", "svc-myapp", false}, + {"Invalid with slash", "invalid/user", true}, + {"Invalid with backslash", "invalid\\user", true}, + {"Invalid with colon", "invalid:user", true}, + {"Invalid with space", "invalid user", true}, + {"Invalid with double quote", "invalid\"user", true}, + {"Invalid with equals", "invalid=user", true}, + {"Invalid with comma", "invalid,user", true}, + {"Invalid with parens", "invalid(user", true}, + {"Invalid with braces", "invalid{user", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, ContainsInvalidCharactersInADAccountName(tt.username)) + }) + } +} + +func TestContainsInvalidCharactersInCredentialSpec(t *testing.T) { + tests := []struct { + name string + path string + want bool + }{ + {"Valid mount path", "leaseID/credspec.json", false}, + {"Valid path with period", "lease123/my.credspec.json", false}, + {"Path traversal blocked", "leaseID/../etc/passwd", true}, + {"Double dot blocked", "lease..ID/file", true}, + {"Invalid with backslash", "leaseID\\file", true}, + {"Invalid with colon", "leaseID:file", true}, + {"Invalid with space", "lease ID/file", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, ContainsInvalidCharactersInCredentialSpec(tt.path)) + }) + } +} diff --git a/internal/utils/debug_utils/debug_error_injection.go b/internal/utils/debug_utils/debug_error_injection.go new file mode 100644 index 00000000..3ff4b5a6 --- /dev/null +++ b/internal/utils/debug_utils/debug_error_injection.go @@ -0,0 +1,56 @@ +package debug_utils + +import ( + "fmt" + "os" + "strings" + "sync/atomic" + + "golang.a2z.com/CredentialsFetcherV2/internal/logger" +) + +// Debug error injection simulation targets. +// Set CF_DEBUG_SIMULATE_ERROR to one of these values with LOG_LEVEL=debug to trigger. +const ( + SimulateSetupKerberosFile = "setup_kerberos_file" + SimulateCreateTicketGMSA = "create_ticket_gmsa" + SimulateGetDistinguishedName = "get_distinguished_name" + + envDebugSimulateError = "CF_DEBUG_SIMULATE_ERROR" + maxDebugErrorSimulations = 3 +) + +// debugErrorCount tracks how many times SimulateDebugError has been invoked. +var debugErrorCount atomic.Int32 + +// SimulateDebugError returns a simulated error when debug error injection is +// enabled for the given operation. It requires both LOG_LEVEL=debug and +// CF_DEBUG_SIMULATE_ERROR= to be set. Returns nil after being +// invoked more than 3 times to prevent infinite error loops. +func SimulateDebugError(operation string) error { + if os.Getenv("LOG_LEVEL") != "debug" { + return nil + } + + target := strings.TrimSpace(os.Getenv(envDebugSimulateError)) + if target == "" || target != operation { + return nil + } + + log := logger.GetInstance() + + if debugErrorCount.Add(1) > maxDebugErrorSimulations { + log.Warn("DEBUG ERROR INJECTION: invocation limit reached, skipping", + "operation", operation, "limit", maxDebugErrorSimulations) + return nil + } + + log.Warn("DEBUG ERROR INJECTION: simulating error", + "operation", operation) + return fmt.Errorf("simulated debug error in %s", operation) +} + +// ResetDebugErrorCount resets the invocation counter (for testing). +func ResetDebugErrorCount() { + debugErrorCount.Store(0) +} diff --git a/internal/utils/debug_utils/debug_error_injection_test.go b/internal/utils/debug_utils/debug_error_injection_test.go new file mode 100644 index 00000000..d145c490 --- /dev/null +++ b/internal/utils/debug_utils/debug_error_injection_test.go @@ -0,0 +1,170 @@ +package debug_utils + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSimulateDebugError(t *testing.T) { + tests := []struct { + name string + logLevel string + simTarget string + operation string + expectError bool + expectedMsg string + }{ + { + name: "no env vars set", + logLevel: "", + simTarget: "", + operation: SimulateSetupKerberosFile, + expectError: false, + }, + { + name: "debug mode but no simulate target", + logLevel: "debug", + simTarget: "", + operation: SimulateSetupKerberosFile, + expectError: false, + }, + { + name: "simulate target set but not in debug mode", + logLevel: "info", + simTarget: SimulateSetupKerberosFile, + operation: SimulateSetupKerberosFile, + expectError: false, + }, + { + name: "simulate target set but log level not set", + logLevel: "", + simTarget: SimulateSetupKerberosFile, + operation: SimulateSetupKerberosFile, + expectError: false, + }, + { + name: "debug mode with matching setup_kerberos_file target", + logLevel: "debug", + simTarget: SimulateSetupKerberosFile, + operation: SimulateSetupKerberosFile, + expectError: true, + expectedMsg: "simulated debug error in setup_kerberos_file", + }, + { + name: "debug mode with matching create_ticket_gmsa target", + logLevel: "debug", + simTarget: SimulateCreateTicketGMSA, + operation: SimulateCreateTicketGMSA, + expectError: true, + expectedMsg: "simulated debug error in create_ticket_gmsa", + }, + { + name: "debug mode with matching get_distinguished_name target", + logLevel: "debug", + simTarget: SimulateGetDistinguishedName, + operation: SimulateGetDistinguishedName, + expectError: true, + expectedMsg: "simulated debug error in get_distinguished_name", + }, + { + name: "debug mode with non-matching target", + logLevel: "debug", + simTarget: SimulateCreateTicketGMSA, + operation: SimulateSetupKerberosFile, + expectError: false, + }, + { + name: "debug mode with whitespace-padded target", + logLevel: "debug", + simTarget: " " + SimulateCreateTicketGMSA + " ", + operation: SimulateCreateTicketGMSA, + expectError: true, + expectedMsg: "simulated debug error in create_ticket_gmsa", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ResetDebugErrorCount() + + origLogLevel := os.Getenv("LOG_LEVEL") + origSimTarget := os.Getenv(envDebugSimulateError) + defer func() { + require.NoError(t, os.Setenv("LOG_LEVEL", origLogLevel)) + require.NoError(t, os.Setenv(envDebugSimulateError, origSimTarget)) + }() + + if tt.logLevel != "" { + require.NoError(t, os.Setenv("LOG_LEVEL", tt.logLevel)) + } else { + require.NoError(t, os.Unsetenv("LOG_LEVEL")) + } + if tt.simTarget != "" { + require.NoError(t, os.Setenv(envDebugSimulateError, tt.simTarget)) + } else { + require.NoError(t, os.Unsetenv(envDebugSimulateError)) + } + + err := SimulateDebugError(tt.operation) + if tt.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedMsg) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestSimulateDebugError_InvocationLimit(t *testing.T) { + ResetDebugErrorCount() + + origLogLevel := os.Getenv("LOG_LEVEL") + origSimTarget := os.Getenv(envDebugSimulateError) + defer func() { + require.NoError(t, os.Setenv("LOG_LEVEL", origLogLevel)) + require.NoError(t, os.Setenv(envDebugSimulateError, origSimTarget)) + }() + + require.NoError(t, os.Setenv("LOG_LEVEL", "debug")) + require.NoError(t, os.Setenv(envDebugSimulateError, SimulateCreateTicketGMSA)) + + for i := 1; i <= maxDebugErrorSimulations; i++ { + err := SimulateDebugError(SimulateCreateTicketGMSA) + assert.Error(t, err, "invocation %d should return error", i) + } + + for i := 1; i <= 3; i++ { + err := SimulateDebugError(SimulateCreateTicketGMSA) + assert.NoError(t, err, "invocation %d past limit should return nil", maxDebugErrorSimulations+i) + } +} + +func TestResetDebugErrorCount(t *testing.T) { + ResetDebugErrorCount() + + origLogLevel := os.Getenv("LOG_LEVEL") + origSimTarget := os.Getenv(envDebugSimulateError) + defer func() { + require.NoError(t, os.Setenv("LOG_LEVEL", origLogLevel)) + require.NoError(t, os.Setenv(envDebugSimulateError, origSimTarget)) + }() + + require.NoError(t, os.Setenv("LOG_LEVEL", "debug")) + require.NoError(t, os.Setenv(envDebugSimulateError, SimulateSetupKerberosFile)) + + for i := 0; i < maxDebugErrorSimulations; i++ { + err := SimulateDebugError(SimulateSetupKerberosFile) + assert.Error(t, err) + } + err := SimulateDebugError(SimulateSetupKerberosFile) + assert.NoError(t, err, "should stop after limit") + + ResetDebugErrorCount() + + err = SimulateDebugError(SimulateSetupKerberosFile) + assert.Error(t, err, "should simulate again after reset") +} diff --git a/internal/utils/grpc_utils/grpc_utils.go b/internal/utils/grpc_utils/grpc_utils.go index 37db0db3..c8d886fe 100644 --- a/internal/utils/grpc_utils/grpc_utils.go +++ b/internal/utils/grpc_utils/grpc_utils.go @@ -3,6 +3,7 @@ package grpc_utils import ( "context" "crypto/rand" + "encoding/hex" "encoding/json" "fmt" "regexp" @@ -96,9 +97,19 @@ func ParseCredSpec(credspecData string) (*types.CredentialSpec, error) { // parseJSON parses the credential spec JSON string into a map func parseJSON(credspecData string) (map[string]interface{}, error) { + // Strip UTF-8 BOM (EF BB BF) if present — Windows tools commonly emit BOM-prefixed files + credspecData = strings.TrimPrefix(credspecData, "\xef\xbb\xbf") + // Strip lone 0xEF byte from partial/corrupt BOM sequences + credspecData = strings.TrimPrefix(credspecData, "\xef") + var root map[string]interface{} if err := json.Unmarshal([]byte(credspecData), &root); err != nil { logger.GetInstance().Error("Failed to parse credential spec JSON", "error", err) + n := len(credspecData) + if n > 32 { + n = 32 + } + logger.GetInstance().Error("Credential spec hex prefix", "hex", hex.EncodeToString([]byte(credspecData[:n]))) return nil, fmt.Errorf("failed to parse credential spec JSON: %v", err) } return root, nil @@ -227,7 +238,7 @@ func ValidateAccountName(username string) error { // Check for invalid characters for _, char := range username { - if strings.ContainsRune(constants.InvalidUsernameChars, char) || char == ' ' { + if strings.ContainsRune(constants.InvalidSAMAccountNameChars, char) { log.Error("Username contains invalid character", "username", username, "invalid_char", string(char)) @@ -491,6 +502,30 @@ func ParseCredSpecDomainless(credspecData string, krbTicketInfo *types.TicketInf return nil } +// ParseBlueGreenUsername parses a username that may contain a blue/green rotation +// separator (':'). Active Directory forbids ':' in usernames, so the format +// "oldUser:newUser" is unambiguous. +// +// This is used during credential rotation when usernames are changed in +// AWS Secrets Manager. The caller supplies "oldUser:newUser" so that the +// service can match existing tickets by the old (blue) username and then +// recreate them with the new (green) username and password. +// +// Returns: +// - matchUsername: the username to match existing tickets against (old / "blue") +// - activeUsername: the username to use for ticket creation (new / "green") +// - isRotation: true when a ':' separator was found +// +// When no ':' is present the same value is returned for both fields and +// isRotation is false (normal, non-rotation renewal). +func ParseBlueGreenUsername(raw string) (matchUsername, activeUsername string, isRotation bool) { + idx := strings.Index(raw, ":") + if idx < 0 { + return raw, raw, false + } + return raw[:idx], raw[idx+1:], true +} + // SecureClearString securely clears a string by overwriting its contents before setting it to empty // This should be called before every RPC call ends to clear sensitive data func SecureClearString(s *string) { diff --git a/internal/utils/grpc_utils/grpc_utils_test.go b/internal/utils/grpc_utils/grpc_utils_test.go index 438902ff..009f1d2d 100644 --- a/internal/utils/grpc_utils/grpc_utils_test.go +++ b/internal/utils/grpc_utils/grpc_utils_test.go @@ -1,6 +1,8 @@ package grpc_utils import ( + "os" + "path/filepath" "strings" "testing" @@ -59,6 +61,14 @@ func TestParseCredSpec(t *testing.T) { assert.Equal(t, "arn:aws:secretsmanager:us-west-2:123456789012:secret:test-secret", credSpec.CredentialArn) }) + t.Run("Valid CredSpec with UTF-8 BOM", func(t *testing.T) { + bomCredSpec := "\xef\xbb\xbf" + validCredSpec + credSpec, err := ParseCredSpec(bomCredSpec) + require.NoError(t, err) + assert.Equal(t, "example.com", credSpec.DomainName) + assert.Equal(t, "WebApp01", credSpec.ServiceAccountName) + }) + t.Run("Empty CredSpec", func(t *testing.T) { _, err := ParseCredSpec("") assert.Error(t, err) @@ -270,6 +280,100 @@ func TestParseCredSpec(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "missing or invalid CredentialArn") }) + + // Real-world credspec with periods in gMSA name and non-domain-joined HostAccountConfig + periodGmsaCredSpec := `{ + "CmsPlugins": [ + "ActiveDirectory" + ], + "DomainJoinConfig": { + "Sid": "S-1-5-21-861567501-616249376-725345543", + "MachineAccountName": "AWS.msa.kiosk", + "Guid": "c08770b1-09d5-4b64-a409-dc8193361342", + "DnsTreeName": "ad.contoso.com", + "DnsName": "ad.contoso.com", + "NetBiosName": "CONTOSO" + }, + "ActiveDirectoryConfig": { + "GroupManagedServiceAccounts": [ + { + "Name": "AWS.msa.kiosk", + "Scope": "ad.contoso.com" + }, + { + "Name": "AWS.msa.kiosk", + "Scope": "CONTOSO" + } + ], + "HostAccountConfig": { + "PortableCcgVersion": "1", + "PluginGUID": "{859E1386-BDB4-49E8-85C7-3070B13920E1}", + "PluginInput": { + "CredentialArn": "arn:aws:secretsmanager:us-east-1:111122223333:secret:aws/kiosk/ecs/msa_kiosk" + } + } + } +}` + + t.Run("Real-world credspec with periods in gMSA name", func(t *testing.T) { + credSpec, err := ParseCredSpec(periodGmsaCredSpec) + require.NoError(t, err) + assert.Equal(t, "ad.contoso.com", credSpec.DomainName) + assert.Equal(t, "AWS.msa.kiosk", credSpec.ServiceAccountName) + assert.Equal(t, "arn:aws:secretsmanager:us-east-1:111122223333:secret:aws/kiosk/ecs/msa_kiosk", credSpec.CredentialArn) + }) + + t.Run("Real-world credspec with UTF-8 BOM", func(t *testing.T) { + bomCredSpec := "\xef\xbb\xbf" + periodGmsaCredSpec + credSpec, err := ParseCredSpec(bomCredSpec) + require.NoError(t, err) + assert.Equal(t, "ad.contoso.com", credSpec.DomainName) + assert.Equal(t, "AWS.msa.kiosk", credSpec.ServiceAccountName) + assert.Equal(t, "arn:aws:secretsmanager:us-east-1:111122223333:secret:aws/kiosk/ecs/msa_kiosk", credSpec.CredentialArn) + }) + + t.Run("contoso_WebApp01_ndj.json with UTF-8 BOM", func(t *testing.T) { + // Find repo root by walking up from the test file's package directory + repoRoot, err := filepath.Abs(filepath.Join("..", "..", "..")) + require.NoError(t, err) + + data, err := os.ReadFile(filepath.Join(repoRoot, "tests", "assets", "contoso_WebApp01_ndj.json")) + require.NoError(t, err) + + bomCredSpec := "\xef\xbb\xbf" + string(data) + credSpec, err := ParseCredSpec(bomCredSpec) + require.NoError(t, err) + assert.Equal(t, "contoso.com", credSpec.DomainName) + assert.Equal(t, "svc_test.app", credSpec.ServiceAccountName) + assert.Equal(t, "arn:aws:secretsmanager:us-west-2:123456789012:secret:aws/directoryservice/contoso/standarduser-AbCdEf", credSpec.CredentialArn) + }) + + t.Run("CredSpec with lone 0xEF byte prefix", func(t *testing.T) { + loneEF := "\xef" + validCredSpec + credSpec, err := ParseCredSpec(loneEF) + require.NoError(t, err) + assert.Equal(t, "example.com", credSpec.DomainName) + assert.Equal(t, "WebApp01", credSpec.ServiceAccountName) + }) + + t.Run("CredSpec with partial BOM 0xEF 0xBB prefix", func(t *testing.T) { + partialBOM := "\xef\xbb" + validCredSpec + _, err := ParseCredSpec(partialBOM) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to parse credential spec JSON") + }) + + t.Run("CredSpec without any BOM prefix parses normally", func(t *testing.T) { + credSpec, err := ParseCredSpec(validCredSpec) + require.NoError(t, err) + assert.Equal(t, "example.com", credSpec.DomainName) + }) + + t.Run("Invalid JSON includes hex prefix in error", func(t *testing.T) { + _, err := ParseCredSpec("\xef\xbb\xbf{bad json}") + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to parse credential spec JSON") + }) } func TestValidateAccountName(t *testing.T) { @@ -383,6 +487,56 @@ func TestValidateAccountName(t *testing.T) { username: "invalid~user", wantErr: true, }, + { + name: "Username with invalid character double quote", + username: "invalid\"user", + wantErr: true, + }, + { + name: "Username with invalid character =", + username: "invalid=user", + wantErr: true, + }, + { + name: "Username with invalid character ,", + username: "invalid,user", + wantErr: true, + }, + { + name: "Username with period is valid", + username: "svc.myapp", + wantErr: false, + }, + { + name: "Username with multiple periods is valid", + username: "corp.svc.account", + wantErr: false, + }, + { + name: "Username with hyphen and period is valid", + username: "svc.my-app", + wantErr: false, + }, + { + name: "Username with invalid character (", + username: "invalid(user", + wantErr: true, + }, + { + name: "Username with invalid character )", + username: "invalid)user", + wantErr: true, + }, + { + name: "Username with invalid character {", + username: "invalid{user", + wantErr: true, + }, + { + name: "Username with invalid character }", + username: "invalid}user", + wantErr: true, + }, } for _, tt := range tests { @@ -622,6 +776,75 @@ _ldap._tcp.dc._msdcs.example.com service = 0 100 389 dc2.example.com`, } } +func TestParseBlueGreenUsername(t *testing.T) { + tests := []struct { + name string + raw string + wantMatch string + wantActive string + wantIsRotation bool + }{ + { + name: "Normal username (no rotation)", + raw: "standarduser", + wantMatch: "standarduser", + wantActive: "standarduser", + wantIsRotation: false, + }, + { + name: "Blue/green rotation", + raw: "olduser:newuser", + wantMatch: "olduser", + wantActive: "newuser", + wantIsRotation: true, + }, + { + name: "Empty old username", + raw: ":newuser", + wantMatch: "", + wantActive: "newuser", + wantIsRotation: true, + }, + { + name: "Empty new username", + raw: "olduser:", + wantMatch: "olduser", + wantActive: "", + wantIsRotation: true, + }, + { + name: "Multiple colons (only first split)", + raw: "old:new:extra", + wantMatch: "old", + wantActive: "new:extra", + wantIsRotation: true, + }, + { + name: "Empty string", + raw: "", + wantMatch: "", + wantActive: "", + wantIsRotation: false, + }, + { + name: "Blue/green rotation with real-world naming convention", + raw: "SvcAccountGR:SvcAccountBL", + wantMatch: "SvcAccountGR", + wantActive: "SvcAccountBL", + wantIsRotation: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + matchUser, activeUser, isRotation := ParseBlueGreenUsername(tt.raw) + assert.Equal(t, tt.wantMatch, matchUser, "matchUsername mismatch") + assert.Equal(t, tt.wantActive, activeUser, "activeUsername mismatch") + assert.Equal(t, tt.wantIsRotation, isRotation, "isRotation mismatch") + }) + } +} + func TestSecureClearString(t *testing.T) { // Test with a non-empty string sensitiveData := "sensitive-password" diff --git a/internal/utils/krb_utils/krb_utils.go b/internal/utils/krb_utils/krb_utils.go index 1bf2a303..3de8307e 100644 --- a/internal/utils/krb_utils/krb_utils.go +++ b/internal/utils/krb_utils/krb_utils.go @@ -89,10 +89,10 @@ func ProcessCredentialSpecs(credspecContents []string, username, leaseID string, func CleanupKerberosFilesWithFS(fs FileSystem, krbFilePath string) error { log.Info("Cleaning up Kerberos files", "path", krbFilePath) - // First remove the krb5cc file - if err := fs.Remove(krbFilePath); err != nil && !os.IsNotExist(err) { - log.Error("Failed to remove Kerberos file", "path", krbFilePath, "error", err) - return fmt.Errorf("failed to remove Kerberos file: %v", err) + // Remove the file or directory (use RemoveAll to handle non-empty directories) + if err := fs.RemoveAll(krbFilePath); err != nil && !os.IsNotExist(err) { + log.Error("Failed to remove Kerberos path", "path", krbFilePath, "error", err) + return fmt.Errorf("failed to remove Kerberos path: %v", err) } // Get the service account directory (parent of krb5cc file) diff --git a/internal/utils/krb_utils/krb_utils_test.go b/internal/utils/krb_utils/krb_utils_test.go index f74f3ece..edece084 100644 --- a/internal/utils/krb_utils/krb_utils_test.go +++ b/internal/utils/krb_utils/krb_utils_test.go @@ -1307,10 +1307,14 @@ func (m *mockFileInfo) Sys() interface{} { return nil } // TestCleanupKerberosFilesWithFS_MockComplete covers all paths using mocked filesystem func TestCleanupKerberosFilesWithFS_MockComplete(t *testing.T) { t.Run("RemoveAll serviceAccountDir error", func(t *testing.T) { + removeAllCallCount := 0 mockFS := MockFS{ - RemoveFunc: func(name string) error { return nil }, RemoveAllFunc: func(path string) error { - if strings.Contains(path, "serviceaccount") { + removeAllCallCount++ + if removeAllCallCount == 1 { + return nil // first call removes krbFilePath + } + if removeAllCallCount == 2 && strings.Contains(path, "serviceaccount") { return errors.New("permission denied") } return nil @@ -1325,9 +1329,13 @@ func TestCleanupKerberosFilesWithFS_MockComplete(t *testing.T) { }) t.Run("RemoveAll lease directory error", func(t *testing.T) { + removeAllCallCount := 0 mockFS := MockFS{ - RemoveFunc: func(name string) error { return nil }, RemoveAllFunc: func(path string) error { + removeAllCallCount++ + if removeAllCallCount == 1 { + return nil // first call removes krbFilePath + } if strings.Contains(path, "lease") && !strings.Contains(path, "serviceaccount") { return errors.New("permission denied") } @@ -1344,7 +1352,6 @@ func TestCleanupKerberosFilesWithFS_MockComplete(t *testing.T) { t.Run("Stat returns non-NotExist error", func(t *testing.T) { mockFS := MockFS{ - RemoveFunc: func(name string) error { return nil }, StatFunc: func(name string) (os.FileInfo, error) { if strings.Contains(name, "serviceaccount") { return nil, errors.New("permission denied") @@ -1359,7 +1366,6 @@ func TestCleanupKerberosFilesWithFS_MockComplete(t *testing.T) { t.Run("Success path - all removals succeed", func(t *testing.T) { mockFS := MockFS{ - RemoveFunc: func(name string) error { return nil }, RemoveAllFunc: func(path string) error { return nil }, StatFunc: func(name string) (os.FileInfo, error) { return &mockFileInfo{name: "serviceaccount", isDir: true}, nil @@ -1372,7 +1378,6 @@ func TestCleanupKerberosFilesWithFS_MockComplete(t *testing.T) { t.Run("Directory does not exist", func(t *testing.T) { mockFS := MockFS{ - RemoveFunc: func(name string) error { return nil }, StatFunc: func(name string) (os.FileInfo, error) { return nil, os.ErrNotExist }, @@ -1382,16 +1387,16 @@ func TestCleanupKerberosFilesWithFS_MockComplete(t *testing.T) { assert.NoError(t, err) }) - t.Run("Remove krb5cc file error", func(t *testing.T) { + t.Run("RemoveAll krbFilePath error", func(t *testing.T) { mockFS := MockFS{ - RemoveFunc: func(name string) error { + RemoveAllFunc: func(path string) error { return errors.New("permission denied") }, } err := CleanupKerberosFilesWithFS(mockFS, "/test/lease/serviceaccount/krb5cc") assert.Error(t, err) - assert.Contains(t, err.Error(), "failed to remove Kerberos file") + assert.Contains(t, err.Error(), "failed to remove Kerberos path") }) } diff --git a/internal/utils/metadata_utils/metadata.go b/internal/utils/metadata_utils/metadata.go index fe67c427..0898d14c 100644 --- a/internal/utils/metadata_utils/metadata.go +++ b/internal/utils/metadata_utils/metadata.go @@ -127,9 +127,51 @@ func WriteMetaDataJSON(ticketInfoList []*types.TicketInfo, leaseID string, krbFi // Write the JSON to file // #nosec G306 - if err := os.WriteFile(filePath, jsonData, 0644); err != nil { + if err := os.WriteFile(filePath, jsonData, 0600); err != nil { return fmt.Errorf("failed to write JSON file: %v", err) } return nil } + +// UpdateMetadataJSON rewrites an existing metadata file in-place with updated ticket info. +// Used during credential rotation to persist username changes to disk. +func UpdateMetadataJSON(metadataPath string, ticketInfoList []*types.TicketInfo) error { + root := make(map[string]interface{}) + krbTicketInfoParent := make([]map[string]interface{}, 0, len(ticketInfoList)) + + for _, ticketInfo := range ticketInfoList { + ticket := map[string]interface{}{ + "krb_file_path": ticketInfo.KrbFilePath, + "service_account_name": ticketInfo.ServiceAccountName, + "domain_name": ticketInfo.DomainName, + "domainless_user": ticketInfo.DomainlessUser, + "distinguished_name": ticketInfo.DistinguishedName, + "credspec_info": ticketInfo.CredspecInfo, + } + if ticketInfo.CredentialArn != "" { + ticket["credential_arn"] = ticketInfo.CredentialArn + } + krbTicketInfoParent = append(krbTicketInfoParent, ticket) + } + + root["krb_ticket_info"] = krbTicketInfoParent + + jsonData, err := json.MarshalIndent(root, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal metadata to JSON: %v", err) + } + + // Atomic write: write to temp file then rename to avoid corruption on crash. + tmpPath := metadataPath + ".tmp" + // #nosec G306 + if err := os.WriteFile(tmpPath, jsonData, 0600); err != nil { + return fmt.Errorf("failed to write temporary metadata file: %v", err) + } + if err := os.Rename(tmpPath, metadataPath); err != nil { + _ = os.Remove(tmpPath) // best-effort cleanup + return fmt.Errorf("failed to rename metadata file: %v", err) + } + + return nil +} diff --git a/internal/utils/types/types.go b/internal/utils/types/types.go index 8cdfccdc..39f68b07 100644 --- a/internal/utils/types/types.go +++ b/internal/utils/types/types.go @@ -2,10 +2,21 @@ package types import ( "time" + + "golang.a2z.com/CredentialsFetcherV2/constants" ) -// InvalidCharacters defines characters not allowed in paths/names -var InvalidCharacters = []rune{'&', '|', ';', ':', '$', '*', '?', '<', '>', '!', ' ', '\\', '.', ']', '[', '+', '\'', '`', '~', '}', '{', '"', ')', '('} +// InvalidCredSpecPathChars defines characters not allowed in credential spec mount paths. +// Derived from constants.InvalidSAMAccountNameChars, excluding '/' since paths contain forward slashes. +var InvalidCredSpecPathChars []rune + +func init() { + for _, r := range constants.InvalidSAMAccountNameChars { + if r != '/' { + InvalidCredSpecPathChars = append(InvalidCredSpecPathChars, r) + } + } +} const ( InstallPathForAwsCli = "/usr/bin/aws" diff --git a/tests/assets/contoso_WebApp01_ndj.json b/tests/assets/contoso_WebApp01_ndj.json new file mode 100644 index 00000000..ee562d9f --- /dev/null +++ b/tests/assets/contoso_WebApp01_ndj.json @@ -0,0 +1,30 @@ +{ + "CmsPlugins": ["ActiveDirectory"], + "DomainJoinConfig": { + "Sid": "S-1-5-21-2421564706-1737585382-3854682907", + "MachineAccountName": "WebApp01", + "Guid": "6a91814c-e151-4fb0-96f0-f517566fc883", + "DnsTreeName": "contoso.com", + "DnsName": "contoso.com", + "NetBiosName": "contoso" + }, + "ActiveDirectoryConfig": { + "GroupManagedServiceAccounts": [ + { + "Name": "svc_test.app", + "Scope": "contoso.com" + }, + { + "Name": "svc_test.app", + "Scope": "contoso" + } + ], + "HostAccountConfig": { + "PortableCcgVersion": "1", + "PluginGUID": "{859E1386-BDB4-49E8-85C7-3070B13920E1}", + "PluginInput": { + "CredentialArn": "arn:aws:secretsmanager:us-west-2:123456789012:secret:aws/directoryservice/contoso/standarduser-AbCdEf" + } + } + } +}