Fix encryption, following RFC8291

lib/webpush.rb

@@ -10,6 +10,7 @@
 require 'webpush/errors'
 require 'webpush/vapid_key'
 require 'webpush/encryption'
+require 'webpush/legacy/encryption'
 require 'webpush/request'
 require 'webpush/railtie' if defined?(Rails)
 

lib/webpush/encryption.rb

@@ -7,12 +7,14 @@ module Encryption
     # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
     def encrypt(message, p256dh, auth)
       assert_arguments(message, p256dh, auth)
+      # Following RFC8291, messages can't be longer than 3993 bytes long
+      # so encrypted message do not exceed 4096 bytes
+      raise ArgumentError, "message is too big" if message.bytesize > 3993
 
       group_name = 'prime256v1'
       salt = Random.new.bytes(16)
 
-      server = OpenSSL::PKey::EC.new(group_name)
-      server.generate_key
+      server = OpenSSL::PKey::EC.generate(group_name)
       server_public_key_bn = server.public_key.to_bn
 
       group = OpenSSL::PKey::EC::Group.new(group_name)
@@ -34,10 +36,15 @@ def encrypt(message, p256dh, auth)
       nonce = HKDF.new(prk, salt: salt, info: nonce_info).next_bytes(12)
 
       ciphertext = encrypt_payload(message, content_encryption_key, nonce)
+      # This should never happen if message length <= 3993
+      raise ArgumentError, "encrypted payload is too big" if ciphertext.bytesize > 4096
 
       serverkey16bn = convert16bit(server_public_key_bn)
-      rs = ciphertext.bytesize
-      raise ArgumentError, "encrypted payload is too big" if rs > 4096
+      # According to RFC8188, the final record can be smaller than the record size
+      # RFC8291 requires encrypted messages to be at most 4096. And the example set the
+      # RS to 4096. RS can be smaller to 4096 but hardcoding is also following the specs.
+      # We set RS=4096 to allow testing with the RFC example.
+      rs = 4096
 
       aes128gcmheader = "#{salt}" + [rs].pack('N*') + [serverkey16bn.bytesize].pack('C*') + serverkey16bn
 
@@ -48,13 +55,14 @@ def encrypt(message, p256dh, auth)
     private
 
     def encrypt_payload(plaintext, content_encryption_key, nonce)
+      # RFC8291 requires the padding delimiter to be 0x02
+      plaintext = plaintext + "\x02"
       cipher = OpenSSL::Cipher.new('aes-128-gcm')
       cipher.encrypt
       cipher.key = content_encryption_key
       cipher.iv = nonce
       text = cipher.update(plaintext)
-      padding = cipher.update("\2\0")
-      e_text = text + padding + cipher.final
+      e_text = text + cipher.final
       e_tag = cipher.auth_tag
 
       e_text + e_tag
@@ -74,4 +82,4 @@ def blank?(value)
       value.nil? || value.empty?
     end
   end
-end
+end

lib/webpush/legacy/encryption.rb

@@ -0,0 +1,86 @@
+# frozen_string_literal: true
+
+module Webpush
+  module Legacy
+    module Encryption
+      # This implements RFC8291 draft 4:
+      # https://datatracker.ietf.org/doc/html/draft-ietf-webpush-encryption-04
+
+      extend self
+
+      def encrypt(message, p256dh, auth)
+        assert_arguments(message, p256dh, auth)
+
+        group_name = 'prime256v1'
+        salt = Random.new.bytes(16)
+
+        server = OpenSSL::PKey::EC.generate(group_name)
+        server_public_key_bn = server.public_key.to_bn
+
+        group = OpenSSL::PKey::EC::Group.new(group_name)
+        client_public_key_bn = OpenSSL::BN.new(Webpush.decode64(p256dh), 2)
+        client_public_key = OpenSSL::PKey::EC::Point.new(group, client_public_key_bn)
+
+        shared_secret = server.dh_compute_key(client_public_key)
+
+        client_auth_token = Webpush.decode64(auth)
+
+        prk = HKDF.new(shared_secret, salt: client_auth_token, algorithm: 'SHA256', info: "Content-Encoding: auth\0").next_bytes(32)
+
+        context = create_context(client_public_key_bn, server_public_key_bn)
+
+        content_encryption_key_info = create_info('aesgcm', context)
+        content_encryption_key = HKDF.new(prk, salt: salt, info: content_encryption_key_info).next_bytes(16)
+
+        nonce_info = create_info('nonce', context)
+        nonce = HKDF.new(prk, salt: salt, info: nonce_info).next_bytes(12)
+
+        ciphertext = encrypt_payload(message, content_encryption_key, nonce)
+
+        {
+          ciphertext: ciphertext,
+          salt: salt,
+          server_public_key_bn: server_public_key_bn.to_s(2),
+          server_public_key: server_public_key_bn.to_s(2),
+          shared_secret: shared_secret
+        }
+      end
+
+      private
+
+      def assert_arguments(message, p256dh, auth)
+        raise ArgumentError, 'message cannot be blank' if blank?(message)
+        raise ArgumentError, 'p256dh cannot be blank' if blank?(p256dh)
+        raise ArgumentError, 'auth cannot be blank' if blank?(auth)
+      end
+
+      def blank?(value)
+        value.nil? || value.empty?
+      end
+
+      def create_context(client_public_key, server_public_key)
+        c = client_public_key.to_s(2)
+        s = server_public_key.to_s(2)
+        "\0#{[c.bytesize].pack('n*')}#{c}#{[s.bytesize].pack('n*')}#{s}"
+      end
+
+      def encrypt_payload(plaintext, content_encryption_key, nonce)
+        cipher = OpenSSL::Cipher.new('aes-128-gcm')
+        cipher.encrypt
+        cipher.key = content_encryption_key
+        cipher.iv = nonce
+        padding = cipher.update("\0\0")
+        text = cipher.update(plaintext)
+
+        e_text = padding + text + cipher.final
+        e_tag = cipher.auth_tag
+
+        e_text + e_tag
+      end
+
+      def create_info(type, context)
+        "Content-Encoding: #{type}\0P-256#{context}"
+      end
+    end
+  end
+end

lib/webpush/vapid_key.rb

@@ -10,8 +10,7 @@ class VapidKey
     # @return [Webpush::VapidKey] a VapidKey instance for the given public and private keys
     def self.from_keys(public_key, private_key)
       key = new
-      key.public_key = public_key
-      key.private_key = private_key
+      key.set_keys!(public_key, private_key)
 
       key
     end
@@ -20,19 +19,14 @@ def self.from_keys(public_key, private_key)
     #
     # @return [Webpush::VapidKey] a VapidKey instance for the given public and private keys
     def self.from_pem(pem)
-      key = new
-      src = OpenSSL::PKey.read pem
-      key.curve.public_key = src.public_key
-      key.curve.private_key = src.private_key
-
-      key
+      new(OpenSSL::PKey.read pem)
     end
 
     attr_reader :curve
 
-    def initialize
-      @curve = OpenSSL::PKey::EC.new('prime256v1')
-      @curve.generate_key
+    def initialize(pkey = nil)
+      @curve = pkey
+      @curve = OpenSSL::PKey::EC.generate('prime256v1') if @curve.nil?
     end
 
     # Retrieve the encoded elliptic curve public key for VAPID protocol
@@ -57,11 +51,11 @@ def private_key
     end
 
     def public_key=(key)
-      curve.public_key = OpenSSL::PKey::EC::Point.new(group, to_big_num(key))
+      set_keys!(key, nil)
     end
 
     def private_key=(key)
-      curve.private_key = to_big_num(key)
+      set_keys!(nil, key)
     end
 
     def curve_name
@@ -78,16 +72,39 @@ def to_h
     alias to_hash to_h
 
     def to_pem
-      public_key = OpenSSL::PKey::EC.new curve
-      public_key.private_key = nil
-
-      curve.to_pem + public_key.to_pem
+      curve.to_pem + curve.public_to_pem
     end
 
     def inspect
       "#<#{self.class}:#{object_id.to_s(16)} #{to_h.map { |k, v| ":#{k}=#{v}" }.join(' ')}>"
     end
 
+    def set_keys!(public_key = nil, private_key = nil)
+      if public_key.nil?
+        public_key = curve.public_key
+      else
+        public_key = OpenSSL::PKey::EC::Point.new(group, to_big_num(public_key))
+      end
+
+      if private_key.nil?
+        private_key = curve.private_key
+      else
+        private_key = to_big_num(private_key)
+      end
+
+      asn1 = OpenSSL::ASN1::Sequence([
+        OpenSSL::ASN1::Integer.new(1),
+        # Not properly padded but OpenSSL doesn't mind
+        OpenSSL::ASN1::OctetString(private_key.to_s(2)),
+        OpenSSL::ASN1::ObjectId('prime256v1', 0, :EXPLICIT),
+        OpenSSL::ASN1::BitString(public_key.to_octet_string(:uncompressed), 1, :EXPLICIT),
+      ])
+
+      der = asn1.to_der
+
+      @curve = OpenSSL::PKey::EC.new(der)
+    end
+
     private
 
     def to_big_num(key)

spec/webpush/encryption_spec.rb

@@ -4,9 +4,7 @@
   describe '#encrypt' do
     let(:curve) do
       group = 'prime256v1'
-      curve = OpenSSL::PKey::EC.new(group)
-      curve.generate_key
-      curve
+      OpenSSL::PKey::EC.generate(group)
     end
 
     let(:p256dh) do

spec/webpush/legacy/encryption_spec.rb

@@ -0,0 +1,101 @@
+require 'spec_helper'
+
+describe Webpush::Legacy::Encryption do
+  describe '#encrypt' do
+    let(:curve) do
+      group = 'prime256v1'
+      OpenSSL::PKey::EC.generate(group)
+    end
+
+    let(:p256dh) do
+      ecdh_key = curve.public_key.to_bn.to_s(2)
+      Base64.urlsafe_encode64(ecdh_key)
+    end
+
+    let(:auth) { Base64.urlsafe_encode64(Random.new.bytes(16)) }
+
+    it 'returns ECDH encrypted cipher text, salt, and server_public_key' do
+      payload = Webpush::Legacy::Encryption.encrypt('Hello World', p256dh, auth)
+      expect(decrypt(payload)).to eq('Hello World')
+    end
+
+    it 'returns error when message is blank' do
+      expect { Webpush::Legacy::Encryption.encrypt(nil, p256dh, auth) }.to raise_error(ArgumentError)
+      expect { Webpush::Legacy::Encryption.encrypt('', p256dh, auth) }.to raise_error(ArgumentError)
+    end
+
+    it 'returns error when p256dh is blank' do
+      expect { Webpush::Legacy::Encryption.encrypt('Hello world', nil, auth) }.to raise_error(ArgumentError)
+      expect { Webpush::Legacy::Encryption.encrypt('Hello world', '', auth) }.to raise_error(ArgumentError)
+    end
+
+    it 'returns error when auth is blank' do
+      expect { Webpush::Legacy::Encryption.encrypt('Hello world', p256dh, '') }.to raise_error(ArgumentError)
+      expect { Webpush::Legacy::Encryption.encrypt('Hello world', p256dh, nil) }.to raise_error(ArgumentError)
+    end
+
+    # Bug fix for https://github.com/zaru/webpush/issues/22
+    it 'handles unpadded base64 encoded subscription keys' do
+      unpadded_p256dh = p256dh.gsub(/=*\Z/, '')
+      unpadded_auth = auth.gsub(/=*\Z/, '')
+
+      payload = Webpush::Legacy::Encryption.encrypt('Hello World', unpadded_p256dh, unpadded_auth)
+      expect(decrypt(payload)).to eq('Hello World')
+    end
+
+    def decrypt(payload)
+      salt = payload.fetch(:salt)
+      serverkey16bn = payload.fetch(:server_public_key_bn)
+      ciphertext = payload.fetch(:ciphertext)
+
+      group_name = 'prime256v1'
+      group = OpenSSL::PKey::EC::Group.new(group_name)
+      server_public_key_bn = OpenSSL::BN.new(serverkey16bn.unpack('H*').first, 16)
+      server_public_key = OpenSSL::PKey::EC::Point.new(group, server_public_key_bn)
+      shared_secret = curve.dh_compute_key(server_public_key)
+
+      client_public_key_bn = curve.public_key.to_bn
+      client_auth_token = Webpush.decode64(auth)
+
+      info = "Content-Encoding: auth\0"
+      context = create_context(curve.public_key, server_public_key)
+      content_encryption_key_info = "Content-Encoding: aesgcm\0P-256#{context}"
+      nonce_info = "Content-Encoding: nonce\0P-256#{context}"
+
+      prk = HKDF.new(shared_secret, salt: client_auth_token, algorithm: 'SHA256', info: info).next_bytes(32)
+
+      content_encryption_key = HKDF.new(prk, salt: salt, info: content_encryption_key_info).next_bytes(16)
+      nonce = HKDF.new(prk, salt: salt, info: nonce_info).next_bytes(12)
+
+      decrypt_ciphertext(ciphertext, content_encryption_key, nonce)
+    end
+
+    def create_context(client_public_key, server_public_key)
+      c = client_public_key.to_bn.to_s(2)
+      s = server_public_key.to_bn.to_s(2)
+      context = "\0"
+      context += [c.bytesize].pack("n*")
+      context += c
+      context += [s.bytesize].pack("n*")
+      context += s
+      context
+    end
+
+    def decrypt_ciphertext(ciphertext, content_encryption_key, nonce)
+      secret_data = ciphertext.byteslice(0, ciphertext.bytesize-16)
+      auth = ciphertext.byteslice(ciphertext.bytesize-16, ciphertext.bytesize)
+      decipher = OpenSSL::Cipher.new('aes-128-gcm')
+      decipher.decrypt
+      decipher.key = content_encryption_key
+      decipher.iv = nonce
+      decipher.auth_tag = auth
+
+      decrypted = decipher.update(secret_data) + decipher.final
+
+      e = decrypted.byteslice(0, 2)
+      expect(e).to eq("\0\0")
+
+      decrypted.byteslice(2, decrypted.bytesize-2)
+    end
+  end
+end