FIX: Add OAEP padding option for user API key encryption (#36592)

ZogStriP · web-flow · commit 300ece3d4023 · 2025-12-11T10:41:13.000+01:00
Users/apps running OpenSSL in FIPS 140-3 mode cannot decrypt user API
key responses because FIPS forbids the legacy PKCS1_PADDING scheme that
Ruby/Discourse uses by default.

Add a `padding` parameter to the user API key endpoints that accepts
"oaep" to use PKCS1_OAEP_PADDING instead. The default remains PKCS1 for
backwards compatibility with existing clients.

Usage: POST /user-api-key.json?padding=oaep

Internal ref - t/170427
diff --git a/app/controllers/user_api_keys_controller.rb b/app/controllers/user_api_keys_controller.rb
@@ -11,6 +11,8 @@ class UserApiKeysController < ApplicationController
 
   AUTH_API_VERSION = 4
 
+  ALLOWED_PADDING_MODES = %w[pkcs1 oaep].freeze
+
   def new
     if request.head?
       head :ok, auth_api_version: AUTH_API_VERSION
@@ -90,10 +92,8 @@ def create
     public_key_str = (@client.public_key.presence || params[:public_key])
     public_key = OpenSSL::PKey::RSA.new(public_key_str)
 
-    # by default, Ruby uses `PKCS1_PADDING` here
-    # see https://docs.ruby-lang.org/en/3.2/OpenSSL/PKey/RSA.html#method-i-public_encrypt
-    # make sure that Node/OpenSSL can use the same padding in your implementation
-    @payload = Base64.encode64(public_key.public_encrypt(@payload))
+    validate_payload_size_for_oaep!(@payload, public_key)
+    @payload = Base64.encode64(rsa_encrypt(public_key, @payload))
 
     if scopes.include?("one_time_password")
       # encrypt one_time_password separately to bypass 128 chars encryption limit
@@ -218,6 +218,30 @@ def one_time_password(public_key, username)
     otp = SecureRandom.hex
     Discourse.redis.setex "otp_#{otp}", 10.minutes, username
 
-    Base64.encode64(public_key.public_encrypt(otp))
+    Base64.encode64(rsa_encrypt(public_key, otp))
+  end
+
+  def rsa_encrypt(public_key, data)
+    # OAEP padding is recommended for new applications and required for FIPS 140-3 compliance.
+    # PKCS1 padding is kept as default for backwards compatibility with existing clients.
+    padding_mode = params[:padding] == "oaep" ? "oaep" : "pkcs1"
+    public_key.encrypt(data, { "rsa_padding_mode" => padding_mode })
+  end
+
+  def validate_payload_size_for_oaep!(payload, public_key)
+    return unless params[:padding] == "oaep"
+
+    # RSA-OAEP max payload = key_size_bytes - 2*hash_size_bytes - 2
+    # OpenSSL uses SHA-1 (20 bytes) by default for OAEP
+    key_size_bytes = public_key.n.num_bytes
+    max_payload_size = key_size_bytes - 2 * 20 - 2
+
+    if payload.bytesize > max_payload_size
+      raise Discourse::InvalidParameters.new(
+              "Payload too large for OAEP encryption with this key size. " \
+                "Maximum: #{max_payload_size} bytes, got: #{payload.bytesize} bytes. " \
+                "Try using a shorter nonce or a larger RSA key (minimum 2048-bit recommended).",
+            )
+    end
   end
 end
diff --git a/spec/requests/user_api_keys_controller_spec.rb b/spec/requests/user_api_keys_controller_spec.rb
@@ -268,6 +268,44 @@
       expect(api_key.user_id).to eq(user.id)
     end
 
+    it "encrypts payload with OAEP padding when requested" do
+      # OAEP padding has larger overhead (42 bytes vs 11 for PKCS1), requiring a larger key
+      oaep_private_key = OpenSSL::PKey::RSA.new(2048)
+      oaep_public_key = oaep_private_key.public_key.to_pem
+
+      user = Fabricate(:user, trust_level: TrustLevel[0])
+      sign_in(user)
+
+      oaep_args = args.except(:auth_redirect).merge(padding: "oaep", public_key: oaep_public_key)
+
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
+      post "/user-api-key.json", params: oaep_args
+      expect(response.status).not_to eq(302)
+      payload = response.parsed_body["payload"]
+      encrypted = Base64.decode64(payload)
+      parsed =
+        JSON.parse(
+          oaep_private_key.private_decrypt(encrypted, OpenSSL::PKey::RSA::PKCS1_OAEP_PADDING),
+        )
+      api_key = UserApiKey.with_key(parsed["key"]).first
+      expect(api_key.user_id).to eq(user.id)
+    end
+
+    it "rejects OAEP requests when payload exceeds maximum size for the key" do
+      SiteSetting.user_api_key_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
+
+      sign_in Fabricate(:user, trust_level: TrustLevel[0])
+
+      public_key = OpenSSL::PKey::RSA.new(2048).public_key.to_pem
+      nonce = "x" * 150
+      params = args.except(:auth_redirect).merge(padding: "oaep", public_key:, nonce:)
+
+      post "/user-api-key.json", params: params
+
+      expect(response.status).to eq(400)
+      expect(response.parsed_body["errors"].first).to include("Payload too large for OAEP")
+    end
+
     it "will allow redirect to wildcard urls" do
       SiteSetting.allowed_user_api_auth_redirects = args[:auth_redirect] + "/*"
       args[:auth_redirect] = args[:auth_redirect] + "/bluebirds/fly"
@@ -409,5 +447,28 @@
 
       expect(Discourse.redis.get("otp_#{parsed}")).to eq(user.username)
     end
+
+    it "encrypts one-time-password with OAEP padding when requested" do
+      # OAEP padding has larger overhead, use a larger key for safety
+      oaep_private_key = OpenSSL::PKey::RSA.new(2048)
+      oaep_public_key = oaep_private_key.public_key.to_pem
+
+      SiteSetting.allowed_user_api_auth_redirects = otp_args[:auth_redirect]
+      user = Fabricate(:user, refresh_auto_groups: true)
+      sign_in(user)
+
+      post "/user-api-key/otp", params: otp_args.merge(padding: "oaep", public_key: oaep_public_key)
+      expect(response.status).to eq(302)
+
+      uri = URI.parse(response.redirect_url)
+
+      query = uri.query
+      payload = query.split("oneTimePassword=")[1]
+      encrypted = Base64.decode64(CGI.unescape(payload))
+
+      parsed = oaep_private_key.private_decrypt(encrypted, OpenSSL::PKey::RSA::PKCS1_OAEP_PADDING)
+
+      expect(Discourse.redis.get("otp_#{parsed}")).to eq(user.username)
+    end
   end
 end
