Skip to content

fix: encrypting api key + session on the fly #294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
0x0elliot wants to merge 1 commit into
base: main
Choose a base branch
from
+355 −67
Open

fix: encrypting api key + session on the fly #294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 100 additions & 43 deletions db-connector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4218,6 +4218,10 @@
}

func init() {
// Skip import path check in test mode
if os.Getenv("SHUFFLE_TEST_MODE") == "true" {
return
}

isValid := checkImportPath()
if !isValid {
Expand Down Expand Up @@ -4899,14 +4903,13 @@

// Index = Username
func SetApikey(ctx context.Context, Userdata User) error {
log.Printf("[AUDIT] Setting API key %s", Userdata.ApiKey)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by an access to ApiKey
Loading
flows to a logging call.

Copilot Autofix

AI 5 days ago

In general, to fix clear‑text logging of sensitive information, you should avoid logging secrets entirely, or at minimum log only non‑sensitive derivatives (such as a truncated or hashed value) that cannot be used to authenticate. For API keys, the usual patterns are: log only the associated username/user ID; or log a short, non‑reversible fingerprint of the key (e.g., first/last few characters, or a cryptographic hash).

For this specific case, the best fix that preserves functionality is to change the log line in SetApikey so it no longer outputs the raw Userdata.ApiKey. Instead, we can log the username (already available as Userdata.Username) and optionally a short, non‑sensitive representation of the key. Since crypto/sha256 is already imported in this file, we can compute a SHA‑256 hash of the API key and log only the hex‑encoded hash as a stable identifier, or we can log the last 4 characters to help operators correlate events without exposing the full secret. To avoid introducing new helpers outside the shown snippet, we’ll implement the masking inline in SetApikey using simple string operations and keep imports untouched.

Concretely: in db-connector.go, around line 4906, replace log.Printf("[AUDIT] Setting API key %s", Userdata.ApiKey) with a version that either omits the key and logs only the username, or logs a masked form such as **** plus the last 4 characters. This change does not alter any business logic—the API key is still stored as before—but removes clear‑text exposure in logs. No additional methods or imports are strictly required; we can do masking with the standard len and slicing operations already available.

Suggested changeset 1
db-connector.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/db-connector.go b/db-connector.go
--- a/db-connector.go
+++ b/db-connector.go
@@ -4903,7 +4903,11 @@
 
 // Index = Username
 func SetApikey(ctx context.Context, Userdata User) error {
-	log.Printf("[AUDIT] Setting API key %s", Userdata.ApiKey)
+	maskedKey := "****"
+	if len(Userdata.ApiKey) > 4 {
+		maskedKey = "****" + Userdata.ApiKey[len(Userdata.ApiKey)-4:]
+	}
+	log.Printf("[AUDIT] Setting API key for user %s (key suffix %s)", Userdata.Username, maskedKey)
 
 	newapiUser := new(Userapi)
 	newapiUser.Username = strings.ToLower(Userdata.Username)
EOF
@@ -4903,7 +4903,11 @@

// Index = Username
func SetApikey(ctx context.Context, Userdata User) error {
log.Printf("[AUDIT] Setting API key %s", Userdata.ApiKey)
maskedKey := "****"
if len(Userdata.ApiKey) > 4 {
maskedKey = "****" + Userdata.ApiKey[len(Userdata.ApiKey)-4:]
}
log.Printf("[AUDIT] Setting API key for user %s (key suffix %s)", Userdata.Username, maskedKey)

newapiUser := new(Userapi)
newapiUser.Username = strings.ToLower(Userdata.Username)
Copilot is powered by AI and may make mistakes. Always verify output.

// Non indexed User data
newapiUser := new(Userapi)
newapiUser.ApiKey = Userdata.ApiKey
newapiUser.Username = strings.ToLower(Userdata.Username)
newapiUser.ApiKey = Userdata.ApiKey
nameKey := "apikey"

// New struct, to not add body, author etc
if project.DbType == "opensearch" {
data, err := json.Marshal(Userdata)
if err != nil {
Expand Down Expand Up @@ -5105,10 +5108,7 @@
return *api, nil
}

// Index = Username
func SetSession(ctx context.Context, user User, value string) error {
//parsedKey := strings.ToLower(user.Username)
// Non indexed User data
parsedKey := user.Id
user.Session = value

Expand Down Expand Up @@ -6221,7 +6221,7 @@
if !strings.Contains(err.Error(), "doesn't exist") {
log.Printf("[WARNING] Error getting org %s in fixUserOrg: %s", orgId, err)
}

return
}

Expand Down Expand Up @@ -9738,17 +9738,34 @@
}
}

// Query for the specific API-key in users
sessionsToSearch := []string{sessionId}
encryptedSession, encErr := HandleKeyEncryption([]byte(sessionId), "session", true)
if encErr == nil {
sessionsToSearch = append([]string{string(encryptedSession)}, sessionsToSearch...)
} else {
log.Printf("[WARNING] Failed encrypting session: %s", encErr)
}

nameKey := "Users"
var users []User
if project.DbType == "opensearch" {
shouldClauses := make([]map[string]interface{}, len(sessionsToSearch))
for i, sess := range sessionsToSearch {
shouldClauses[i] = map[string]interface{}{
"match": map[string]interface{}{
"session": sess,
},
}
}

var buf bytes.Buffer
query := map[string]interface{}{
"from": 0,
"size": 1000,
"query": map[string]interface{}{
"match": map[string]interface{}{
"session": sessionId,
"bool": map[string]interface{}{
"should": shouldClauses,
"minimum_should_match": 1,
},
},
}
Expand All @@ -9770,7 +9787,7 @@
return User{}, nil
}

log.Printf("[ERROR] Error getting response from Opensearch (get api keys): %s", err)
log.Printf("[ERROR] Error getting response from Opensearch (get session): %s", err)
return User{}, err
}

Expand Down Expand Up @@ -9813,27 +9830,38 @@

users = []User{}
for _, hit := range wrapped.Hits.Hits {
if hit.Source.Session != sessionId {
// Check if session matches any of our search keys
matched := false
for _, sess := range sessionsToSearch {
if hit.Source.Session == sess {
matched = true
break
}
}
if !matched {
continue
}

users = append(users, hit.Source)
}

} else {
//log.Printf("[DEBUG] Searching for session %s", sessionId)
q := datastore.NewQuery(nameKey).Filter("session =", sessionId).Limit(1)
_, err := project.Dbclient.GetAll(ctx, q, &users)
if err != nil && len(users) == 0 {
if !strings.Contains(err.Error(), `cannot load field`) {
log.Printf("[WARNING] Error getting session: %s", err)
return User{}, err
// Datastore: try encrypted first, then plain (no IN filter support)
for _, sess := range sessionsToSearch {
q := datastore.NewQuery(nameKey).Filter("session =", sess).Limit(1)
_, err := project.Dbclient.GetAll(ctx, q, &users)
if err != nil && len(users) == 0 {
if !strings.Contains(err.Error(), `cannot load field`) {
continue
}
}
if len(users) > 0 {
break
}
}
}

if len(users) == 0 {
return User{}, errors.New("No users found for this apikey (1)")
return User{}, errors.New("No users found for this session")
}

if project.CacheDb {
Expand All @@ -9853,17 +9881,34 @@
}

func GetApikey(ctx context.Context, apikey string) (User, error) {
// Query for the specific API-key in users
// Build list of keys to search: encrypted (new) + plain (backwards compat)
keysToSearch := []string{apikey}
encryptedKey, encErr := HandleKeyEncryption([]byte(apikey), "apikey", true)
if encErr == nil {
keysToSearch = append([]string{string(encryptedKey)}, keysToSearch...)
}

nameKey := "Users"
var users []User
if project.DbType == "opensearch" {
// Build OR query for both encrypted and plain apikey
shouldClauses := make([]map[string]interface{}, len(keysToSearch))
for i, key := range keysToSearch {
shouldClauses[i] = map[string]interface{}{
"match": map[string]interface{}{
"apikey": key,
},
}
}

var buf bytes.Buffer
query := map[string]interface{}{
"from": 0,
"size": 1000,
"query": map[string]interface{}{
"match": map[string]interface{}{
"apikey": apikey,
"bool": map[string]interface{}{
"should": shouldClauses,
"minimum_should_match": 1,
},
},
}
Expand Down Expand Up @@ -9928,20 +9973,32 @@

users = []User{}
for _, hit := range wrapped.Hits.Hits {
if hit.Source.ApiKey != apikey {
// Check if apikey matches any of our search keys
matched := false
for _, key := range keysToSearch {
if hit.Source.ApiKey == key {
matched = true
break
}
}
if !matched {
continue
}

users = append(users, hit.Source)
}

} else {
q := datastore.NewQuery(nameKey).Filter("apikey =", apikey).Limit(1)
_, err := project.Dbclient.GetAll(ctx, q, &users)
if err != nil && len(users) == 0 {
if !strings.Contains(err.Error(), `cannot load field`) {
log.Printf("[WARNING] Error getting apikey: %s", err)
return User{}, err
// Datastore: try encrypted first, then plain (no IN filter support)
for _, key := range keysToSearch {
q := datastore.NewQuery(nameKey).Filter("apikey =", key).Limit(1)
_, err := project.Dbclient.GetAll(ctx, q, &users)
if err != nil && len(users) == 0 {
if !strings.Contains(err.Error(), `cannot load field`) {
continue
}
}
if len(users) > 0 {
break
}
}
}
Expand Down Expand Up @@ -13965,7 +14022,7 @@

category = strings.ReplaceAll(strings.ToLower(category), " ", "_")
if len(category) > 0 && category != "default" {
// FIXME: If they key itself is 'test_protected' and category
// FIXME: If they key itself is 'test_protected' and category
// is 'protected' this breaks... Keeping it for now.
if !strings.HasSuffix(id, fmt.Sprintf("_%s", category)) {
id = fmt.Sprintf("%s_%s", id, category)
Expand Down Expand Up @@ -14225,18 +14282,18 @@
} else {
//log.Printf("\n\n[INFO] Should check for SSO during setup - finding main org\n\n")
/*
orgs, err := GetAllOrgs(ctx)
if err == nil {
for _, org := range orgs {
if len(org.ManagerOrgs) == 0 && len(org.SSOConfig.SSOEntrypoint) > 0 {
log.Printf("[INFO] Set initial SSO url for logins to %s", org.SSOConfig.SSOEntrypoint)
SSOUrl = org.SSOConfig.SSOEntrypoint
break
orgs, err := GetAllOrgs(ctx)
if err == nil {
for _, org := range orgs {
if len(org.ManagerOrgs) == 0 && len(org.SSOConfig.SSOEntrypoint) > 0 {
log.Printf("[INFO] Set initial SSO url for logins to %s", org.SSOConfig.SSOEntrypoint)
SSOUrl = org.SSOConfig.SSOEntrypoint
break
}
}
} else {
log.Printf("[WARNING] Error loading orgs: %s", err)
}
} else {
log.Printf("[WARNING] Error loading orgs: %s", err)
}
*/
}
} else {
Expand Down
Loading
Loading