Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

New application configuration: hoist.sensitiveParamTerms #407

Merged
merged 2 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
This replaces the static map `BaseService.clusterConfigs`.
* Misc. improvements to logging and performance of `Cache` and `Timer`.

### 🎁 New Features
* New configuration property `hoist.sensitiveParamTerms` allows customization of environment
variables to be obscured in the admin client.

## 22.0.0 - 2024-09-18

### 💥 Breaking Changes (upgrade difficulty: 🟢 LOW)
Expand Down
4 changes: 0 additions & 4 deletions grails-app/domain/io/xh/hoist/config/AppConfig.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import org.jasypt.util.text.BasicTextEncryptor
import org.jasypt.util.text.TextEncryptor

import static grails.async.Promises.task
import static io.xh.hoist.util.Utils.isSensitiveParamName

class AppConfig implements JSONFormat, LogSupport {

Expand Down Expand Up @@ -121,9 +120,6 @@ class AppConfig implements JSONFormat, LogSupport {
// so we don't assume it can be parsed into the required type. Log failures on trace for
// minimal visibility, but otherwise act as if the override value is not set.
try {
if (isSensitiveParamName(name) && valueType != 'pwd') {
throw new RuntimeException("Refusing to return potentially sensitive override value from plain-text config - must be of type 'pwd'")
}
return parseValue(overrideValue, opts)
} catch (Throwable e) {
logTrace("InstanceConfig override found for '$name' but cannot be parsed - override will be ignored", e.message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

package io.xh.hoist.websocket

import grails.async.Promises
import grails.core.GrailsApplication
import grails.events.EventPublisher
import groovy.transform.CompileStatic
import io.xh.hoist.BaseService
Expand All @@ -23,6 +21,7 @@ import java.util.concurrent.ConcurrentHashMap

import static grails.async.Promises.task
import static grails.async.Promises.waitAll
import static io.xh.hoist.util.Utils.grailsConfig


/**
Expand Down Expand Up @@ -50,7 +49,6 @@ import static grails.async.Promises.waitAll
@CompileStatic
class WebSocketService extends BaseService implements EventPublisher {

GrailsApplication grailsApplication
IdentityService identityService

static final String HEARTBEAT_TOPIC = 'xhHeartbeat'
Expand All @@ -62,7 +60,7 @@ class WebSocketService extends BaseService implements EventPublisher {
private Map<WebSocketSession, HoistWebSocketChannel> _channels = new ConcurrentHashMap<>()

boolean isEnabled() {
return grailsApplication.config.getProperty('hoist.enableWebSockets', Boolean)
grailsConfig.getProperty('hoist.enableWebSockets', Boolean)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ class ApplicationConfig {
static void defaultConfig(Script script) {
withDelegate(script) {

// Read by WebSocketService to determine if WS support should generally be enabled.
hoist {
// Read by WebSocketService to determine if WS support should generally be enabled.
enableWebSockets = true
sensitiveParamTerms = ['password', 'passwrd', 'pwd', 'secret', 'tkn', 'token']
}

spring {
Expand Down
33 changes: 24 additions & 9 deletions src/main/groovy/io/xh/hoist/util/Utils.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import com.esotericsoftware.kryo.Kryo
import com.esotericsoftware.kryo.io.Input
import com.esotericsoftware.kryo.io.Output
import com.esotericsoftware.kryo.serializers.JavaSerializer
import grails.config.Config
import grails.util.Environment
import grails.util.Holders
import grails.util.Metadata
Expand Down Expand Up @@ -151,21 +152,24 @@ class Utils {
//------------------
// Other Singletons
//------------------
/** Get the grails application context. */
static ApplicationContext getAppContext() {
return Holders.applicationContext
}

/** Get the grails application config. */
static Config getGrailsConfig() {
Holders.grailsApplication.config
}

/** Primary JDBC datasource, default backing DB for app Domain objects. */
static DataSource getDataSource() {
return (DataSource) appContext.dataSource
}

/** Primary dataSource configuration. */
static Map getDataSourceConfig() {
Holders.grailsApplication
.config
.getProperty('dataSource', Map.class)
.collectEntries { it }
grailsConfig.getProperty('dataSource', Map.class).collectEntries { it }
}

static ExceptionHandler getExceptionHandler() {
Expand All @@ -181,13 +185,14 @@ class Utils {
}

/**
* True if the given parameter name is likely sensitive and should not be serialized if not
* required. Used for built-in admin functionality accessed by trusted users only.
* NOT intended to be comprehensive or the last word on security!
* True if the given parameter name is likely sensitive and should not be serialized.
* To customize, set `hoist.sensitiveParamTerms` within your app's `application.groovy` to a
* list of terms that should trigger this behavior.
*
* See {@link io.xh.hoist.configuration.ApplicationConfig} for the default list.
*/
static boolean isSensitiveParamName(String name) {
def pattern = ~/(?i)(password|pwd|secret)$/
pattern.matcher(name).find()
sensitiveParams.any { name.containsIgnoreCase(it) }
}

/** String parsing for a boolean. */
Expand Down Expand Up @@ -220,6 +225,7 @@ class Utils {
c.call()
}


static testSerialization(Object obj, Class clazz, LogSupport logSupport, Map opts) {
Kryo kryo = new Kryo()
kryo.registrationRequired = false
Expand Down Expand Up @@ -251,4 +257,13 @@ class Utils {
"${endTime - startTime}ms"
)
}

private static terms = null
private static List<String> getSensitiveParams() {
if (terms == null) {
terms = grailsConfig.getProperty('hoist.sensitiveParamTerms', ArrayList.class) as List<String>
}
return terms
}

}
Loading