feat: various security hardening

Author: ymarcon

src/dist/conf/application.yml

@@ -43,4 +43,9 @@ security:
   # RAppArmor profile to apply to user role, see setup instructions at <https://github.com/jeroen/RAppArmor>
   apparmor:
     enabled: false
-    profile: testprofile
+    profile: testprofile
+    # Security headers
+    headers:
+      content-security-policy: "default-src 'self'; script-src 'self' 'unsafe-inline'; object-src 'none'; frame-ancestors 'none';"
+      x-content-type-options: "nosniff"
+      x-frame-options: "DENY"

src/main/java/org/obiba/rock/domain/ExceptionErrorMessage.java

@@ -19,14 +19,16 @@ public class ExceptionErrorMessage extends ErrorMessage {
   public ExceptionErrorMessage(String status, Exception exception, String... args) {
     setStatus(status);
     setKey(exception.getClass().getSimpleName());
-    setMessage(exception.getMessage());
+    // Use generic message to prevent information disclosure
+    setMessage("An error occurred");
     setArgs(Lists.newArrayList(args));
   }
 
   public ExceptionErrorMessage(HttpStatus status, Exception exception, String... args) {
     setStatus(status.value() + "");
     setKey(exception.getClass().getSimpleName());
-    setMessage(exception.getMessage());
+    // Use generic message to prevent information disclosure
+    setMessage("An error occurred");
     setArgs(Lists.newArrayList(args));
   }
 }

src/main/java/org/obiba/rock/r/FileReadROperation.java

@@ -21,6 +21,10 @@ public class FileReadROperation extends AbstractROperation {
   private final File destination;
 
   public FileReadROperation(String fileName, File destination) {
+    // Validate file path to prevent directory traversal
+    if (fileName == null || fileName.contains("..") || fileName.contains("/") || fileName.contains("\\")) {
+      throw new IllegalArgumentException("Invalid file path: contains illegal characters");
+    }
     this.fileName = fileName;
     this.destination = destination;
   }

src/main/java/org/obiba/rock/r/FileWriteROperation.java

@@ -21,6 +21,10 @@ public class FileWriteROperation extends AbstractROperation {
   private final File source;
 
   public FileWriteROperation(String fileName, File source) {
+    // Validate file path to prevent directory traversal
+    if (fileName == null || fileName.contains("..") || fileName.contains("/") || fileName.contains("\\")) {
+      throw new IllegalArgumentException("Invalid file path: contains illegal characters");
+    }
     this.fileName = fileName;
     this.source = source;
   }

src/main/java/org/obiba/rock/rest/RSessionsController.java

@@ -40,6 +40,11 @@ public class RSessionsController {
    */
   @GetMapping
   List<RSession> getRSessions(@AuthenticationPrincipal User user, @RequestParam(name = "subject", required = false) String subject) {
+    // Validate subject parameter
+    if (subject != null && (subject.contains("..") || subject.contains("/"))) {
+      throw new IllegalArgumentException("Invalid subject parameter: contains illegal characters");
+    }
+    
     if (Roles.isAdmin(user) || Roles.isManager(user)) {
       // get all/filtered sessions
       return Strings.isNullOrEmpty(subject) ? rSessionService.getRSessions()

src/main/java/org/obiba/rock/security/CustomBasicAuthenticationEntryPoint.java

@@ -26,12 +26,12 @@ public void commence(HttpServletRequest request, HttpServletResponse response, A
     response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
     response.setContentType("application/json");
     PrintWriter writer = response.getWriter();
-    writer.println("{\"status\": \"401\", \"key\": \"UnAuthorized\", \"message\": \"" + authEx.getMessage() + "\"}");
+    writer.println("{\"status\": \"401\", \"key\": \"UnAuthorized\", \"message\": \"Authentication failed\"}");
   }
 
   @Override
   public void afterPropertiesSet() {
-    setRealmName("RockRealm");
+    setRealmName("Rock");
     super.afterPropertiesSet();
   }
 }

src/main/java/org/obiba/rock/security/SecurityConfiguration.java

@@ -59,24 +59,24 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
   private void configure(HttpSecurity http) throws Exception {
     if (!securityProperties.isEnabled())
       http
-          .csrf(AbstractHttpConfigurer::disable)
-          .authorizeHttpRequests((configurer) -> configurer.requestMatchers("/**").permitAll());
+        .csrf(AbstractHttpConfigurer::disable)
+        .authorizeHttpRequests((configurer) -> configurer.requestMatchers("/**").permitAll());
     else
       http
-          .csrf(AbstractHttpConfigurer::disable)
-          .formLogin(AbstractHttpConfigurer::disable)
-          .authorizeHttpRequests((configurer) -> configurer
-              .requestMatchers("/rserver/**").hasAnyRole(Roles.ROCK_ADMIN, Roles.ROCK_MANAGER)
-              .requestMatchers("/r/sessions/**").hasAnyRole(Roles.ROCK_ADMIN, Roles.ROCK_MANAGER, Roles.ROCK_USER)
-              .requestMatchers("/r/session/**").hasAnyRole(Roles.ROCK_ADMIN, Roles.ROCK_USER)
-              .requestMatchers("/").permitAll()
-              .requestMatchers("/_check").permitAll()
-              .requestMatchers("/_info").permitAll()
-              .anyRequest().denyAll())
-          .httpBasic((configurer) -> configurer
-              .realmName("RockRealm")
-              .authenticationEntryPoint(getBasicAuthenticationEntryPoint()))
-          .sessionManagement((configurer) -> configurer.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
+        .csrf(AbstractHttpConfigurer::disable)
+        .formLogin(AbstractHttpConfigurer::disable)
+        .authorizeHttpRequests((configurer) -> configurer
+            .requestMatchers("/rserver/**").hasAnyRole(Roles.ROCK_ADMIN, Roles.ROCK_MANAGER)
+            .requestMatchers("/r/sessions/**").hasAnyRole(Roles.ROCK_ADMIN, Roles.ROCK_MANAGER, Roles.ROCK_USER)
+            .requestMatchers("/r/session/**").hasAnyRole(Roles.ROCK_ADMIN, Roles.ROCK_USER)
+            .requestMatchers("/").permitAll()
+            .requestMatchers("/_check").permitAll()
+            .requestMatchers("/_info").permitAll()
+            .anyRequest().denyAll())
+        .httpBasic((configurer) -> configurer
+            .realmName("Rock")
+            .authenticationEntryPoint(getBasicAuthenticationEntryPoint()))
+        .sessionManagement((configurer) -> configurer.sessionCreationPolicy(SessionCreationPolicy.STATELESS));
   }
 
   private BasicAuthenticationEntryPoint getBasicAuthenticationEntryPoint() {
@@ -85,11 +85,10 @@ private BasicAuthenticationEntryPoint getBasicAuthenticationEntryPoint() {
 
   private PasswordEncoder newPasswordEncoder() {
     Map<String, PasswordEncoder> encoders = Maps.newHashMap();
-    encoders.put("noop", new NoOpPasswordEncoder());
     encoders.put("bcrypt", new BCryptPasswordEncoder(-1, new SecureRandom()));
     encoders.put("pbkdf2", Pbkdf2PasswordEncoder.defaultsForSpringSecurity_v5_8());
     encoders.put("scrypt", SCryptPasswordEncoder.defaultsForSpringSecurity_v5_8());
-    return new DelegatingPasswordEncoder("noop", encoders);
+    return new DelegatingPasswordEncoder("bcrypt", encoders);
   }
 
   @Bean
@@ -109,8 +108,13 @@ public UserDetailsService userDetailsService() {
       log.debug(u.getId() + ":" + u.getSecret() + ":" + Joiner.on(";").join(u.getRoles()));
       String[] roles = new String[u.getRoles().size()];
       roles = u.getRoles().stream().map(String::toUpperCase).toList().toArray(roles);
+      String encodedPassword = u.getSecret().startsWith("{") ? u.getSecret() : passwordEncoder.encode(u.getSecret());
+      // Validate password is not empty and follows expected format
+      if (encodedPassword == null || encodedPassword.isEmpty() || encodedPassword.length() < 8) {
+        throw new IllegalArgumentException("Password must be at least 8 characters long");
+      }
       manager.createUser(User.withUsername(u.getId())
-          .password(u.getSecret().startsWith("{") ? u.getSecret() : passwordEncoder.encode(u.getSecret()))
+          .password(encodedPassword)
           .roles(roles)
           .build());
     });