refactor salmon stuff

* fix rubocop issues
* remove duplicate code
* use `describe` in specs for methods

lib/diaspora_federation.rb

@@ -1,5 +1,3 @@
-require "base64"
-
 require "diaspora_federation/logging"
 
 require "diaspora_federation/callbacks"

lib/diaspora_federation/salmon.rb

@@ -7,6 +7,8 @@ module DiasporaFederation
   end
 end
 
+require "base64"
+
 require "diaspora_federation/salmon/aes"
 require "diaspora_federation/salmon/exceptions"
 require "diaspora_federation/salmon/xml_payload"

lib/diaspora_federation/salmon/encrypted_slap.rb

@@ -76,29 +76,16 @@ module DiasporaFederation
       def self.from_xml(slap_xml, pkey)
         raise ArgumentError unless slap_xml.instance_of?(String) && pkey.instance_of?(OpenSSL::PKey::RSA)
         doc = Nokogiri::XML::Document.parse(slap_xml)
-        ns = {d: Salmon::XMLNS, me: MagicEnvelope::XMLNS}
-        header_xpath = "d:diaspora/d:encrypted_header"
-        magicenv_xpath = "d:diaspora/me:env"
 
-        if doc.namespaces.empty?
+        Slap.new.tap do |slap|
-          ns = nil
+          header_elem = doc.at_xpath("d:diaspora/d:encrypted_header", Slap::NS)
-          header_xpath = "diaspora/encrypted_header"
+          raise MissingHeader if header_elem.nil?
-          magicenv_xpath = "diaspora/env"
+          header = header_data(header_elem.content, pkey)
+          slap.author_id = header[:author_id]
+          slap.cipher_params = {key: Base64.decode64(header[:aes_key]), iv: Base64.decode64(header[:iv])}
+
+          slap.add_magic_env_from_doc(doc)
         end
-
-        slap = Slap.new
-
-        header_elem = doc.at_xpath(header_xpath, ns)
-        raise MissingHeader if header_elem.nil?
-        header = header_data(header_elem.content, pkey)
-        slap.author_id = header[:author_id]
-        slap.cipher_params = {key: header[:aes_key], iv: header[:iv]}
-
-        magic_env_elem = doc.at_xpath(magicenv_xpath, ns)
-        raise MissingMagicEnvelope if magic_env_elem.nil?
-        slap.magic_envelope = magic_env_elem
-
-        slap
       end
 
       # Creates an encrypted Salmon Slap and returns the XML string.
@@ -115,21 +102,13 @@ module DiasporaFederation
                                    entity.is_a?(Entity) &&
                                    pubkey.instance_of?(OpenSSL::PKey::RSA)
 
-        doc = Nokogiri::XML::Document.new
+        Slap.build_xml do |xml|
-        doc.encoding = "UTF-8"
+          magic_envelope = MagicEnvelope.new(pkey, entity)
+          envelope_key = magic_envelope.encrypt!
 
-        root = Nokogiri::XML::Element.new("diaspora", doc)
+          encrypted_header(author_id, envelope_key, pubkey, xml)
-        root.default_namespace = Salmon::XMLNS
+          magic_envelope.envelop(xml)
-        root.add_namespace("me", MagicEnvelope::XMLNS)
+        end
-        doc.root = root
-
-        magic_envelope = MagicEnvelope.new(pkey, entity, root)
-        envelope_key = magic_envelope.encrypt!
-
-        encrypted_header(author_id, envelope_key, pubkey, root)
-        magic_envelope.envelop
-
-        doc.to_xml
       end
 
       # decrypts and reads the data from the encrypted XML header
@@ -167,19 +146,17 @@ module DiasporaFederation
       # @param [Hash] envelope cipher params
       # @param [OpenSSL::PKey::RSA] recipient public_key
       # @param parent_node [Nokogiri::XML::Element] parent element for insering in XML document
-      def self.encrypted_header(author_id, envelope_key, pubkey, parent_node)
+      def self.encrypted_header(author_id, envelope_key, pubkey, xml)
-        data = header_xml(author_id, envelope_key)
+        data = header_xml(author_id, strict_base64_encode(envelope_key))
         key = AES.generate_key_and_iv
         ciphertext = AES.encrypt(data, key[:key], key[:iv])
 
-        json_key = JSON.generate(key: Base64.strict_encode64(key[:key]), iv: Base64.strict_encode64(key[:iv]))
+        json_key = JSON.generate(strict_base64_encode(key))
         encrypted_key = Base64.strict_encode64(pubkey.public_encrypt(json_key))
 
         json_header = JSON.generate(aes_key: encrypted_key, ciphertext: ciphertext)
 
-        header = Nokogiri::XML::Element.new("encrypted_header", parent_node.document)
+        xml.encrypted_header(Base64.strict_encode64(json_header))
-        header.content = Base64.strict_encode64(json_header)
-        parent_node << header
       end
       private_class_method :encrypted_header
 
@@ -198,6 +175,13 @@ module DiasporaFederation
         builder.to_xml.strip
       end
       private_class_method :header_xml
+
+      # @param [Hash] hash { key: "...", iv: "..." }
+      # @return [Hash] encoded hash: { key: "...", iv: "..." }
+      def self.strict_base64_encode(hash)
+        Hash[hash.map {|k, v| [k, Base64.strict_encode64(v)] }]
+      end
+      private_class_method :strict_base64_encode
     end
   end
 end

lib/diaspora_federation/salmon/magic_envelope.rb

@@ -43,20 +43,11 @@ module DiasporaFederation
       #
       # @param rsa_pkey [OpenSSL::PKey::RSA] private key used for signing
       # @param payload [Entity] Entity instance
-      # @param parent_node [Nokogiri::XML::Element] parent element for insering in XML document
       # @raise [ArgumentError] if either argument is not of the right type
-      def initialize(rsa_pkey, payload, parent_node=nil)
+      def initialize(rsa_pkey, payload)
         raise ArgumentError unless rsa_pkey.instance_of?(OpenSSL::PKey::RSA) &&
                                    payload.is_a?(Entity)
 
-        if parent_node.nil?
-          doc = Nokogiri::XML::Document.new
-          parent_node = Nokogiri::XML::Element.new("root", doc)
-          parent_node.add_namespace("me", XMLNS)
-          doc.root = parent_node
-        end
-
-        @parent_node = parent_node
         @rsa_pkey = rsa_pkey
         @payload = XmlPayload.pack(payload).to_xml.strip
       end
@@ -65,17 +56,13 @@ module DiasporaFederation
       # encoded data and signs the envelope using {DIGEST}.
       #
       # @return [Nokogiri::XML::Element] XML root node
-      def envelop
+      def envelop(xml)
-        builder = Nokogiri::XML::Builder.with(@parent_node) do |xml|
+        xml["me"].env {
-          xml["me"].env {
+          xml["me"].data(Base64.urlsafe_encode64(@payload), type: DATA_TYPE)
-            xml["me"].data(Base64.urlsafe_encode64(@payload), type: DATA_TYPE)
+          xml["me"].encoding(ENCODING)
-            xml["me"].encoding(ENCODING)
+          xml["me"].alg(ALGORITHM)
-            xml["me"].alg(ALGORITHM)
+          xml["me"].sig(Base64.urlsafe_encode64(signature))
-            xml["me"].sig(Base64.urlsafe_encode64(signature))
+        }
-          }
-        end
-
-        builder.doc.at_xpath("//me:env")
       end
 
       # Encrypts the payload with a new, random AES cipher and returns the cipher
@@ -88,9 +75,9 @@ module DiasporaFederation
       #
       # @return [Hash] AES key and iv. E.g.: { key: "...", iv: "..." }
       def encrypt!
-        key = AES.generate_key_and_iv
+        AES.generate_key_and_iv.tap do |key|
-        @payload = AES.encrypt(@payload, key[:key], key[:iv])
+          @payload = AES.encrypt(@payload, key[:key], key[:iv])
-        strict_base64_encode(key)
+        end
       end
 
       # Extracts the entity encoded in the magic envelope data, if the signature
@@ -120,16 +107,10 @@ module DiasporaFederation
         raise InvalidEnvelope unless envelope_valid?(magic_env)
         raise InvalidSignature unless signature_valid?(magic_env, rsa_pubkey)
 
-        enc = magic_env.at_xpath("me:encoding").content
+        raise InvalidEncoding unless encoding_valid?(magic_env)
-        alg = magic_env.at_xpath("me:alg").content
+        raise InvalidAlgorithm unless algorithm_valid?(magic_env)
 
-        raise InvalidEncoding unless enc == ENCODING
+        data = read_and_decrypt_data(magic_env, cipher_params)
-        raise InvalidAlgorithm unless alg == ALGORITHM
-
-        data = Base64.urlsafe_decode64(magic_env.at_xpath("me:data").content)
-        unless cipher_params.nil?
-          data = AES.decrypt(data, Base64.decode64(cipher_params[:key]), Base64.decode64(cipher_params[:iv]))
-        end
 
         XmlPayload.unpack(Nokogiri::XML::Document.parse(data).root)
       end
@@ -177,11 +158,22 @@ module DiasporaFederation
         data_arr.map {|i| Base64.urlsafe_encode64(i) }.join(".")
       end
 
-      # @param [Hash] hash { key: "...", iv: "..." }
+      def self.encoding_valid?(magic_env)
-      # @return [Hash] encoded hash: { key: "...", iv: "..." }
+        magic_env.at_xpath("me:encoding").content == ENCODING
-      def strict_base64_encode(hash)
-        Hash[hash.map {|k, v| [k, Base64.strict_encode64(v)] }]
       end
+      private_class_method :encoding_valid?
+
+      def self.algorithm_valid?(magic_env)
+        magic_env.at_xpath("me:alg").content == ALGORITHM
+      end
+      private_class_method :algorithm_valid?
+
+      def self.read_and_decrypt_data(magic_env, cipher_params)
+        data = Base64.urlsafe_decode64(magic_env.at_xpath("me:data").content)
+        data = AES.decrypt(data, cipher_params[:key], cipher_params[:iv]) unless cipher_params.nil?
+        data
+      end
+      private_class_method :read_and_decrypt_data
     end
   end
 end

lib/diaspora_federation/salmon/slap.rb

@@ -29,6 +29,9 @@ module DiasporaFederation
     class Slap
       attr_accessor :author_id, :magic_envelope, :cipher_params
 
+      # Namespaces
+      NS = {d: Salmon::XMLNS, me: MagicEnvelope::XMLNS}
+
       # Returns new instance of the Entity that is contained within the XML of
       # this Slap.
       #
@@ -59,27 +62,14 @@ module DiasporaFederation
       def self.from_xml(slap_xml)
         raise ArgumentError unless slap_xml.instance_of?(String)
         doc = Nokogiri::XML::Document.parse(slap_xml)
-        ns = {d: Salmon::XMLNS, me: MagicEnvelope::XMLNS}
-        author_xpath = "d:diaspora/d:header/d:author_id"
-        magicenv_xpath = "d:diaspora/me:env"
 
-        if doc.namespaces.empty?
+        Slap.new.tap do |slap|
-          ns = nil
+          author_elem = doc.at_xpath("d:diaspora/d:header/d:author_id", Slap::NS)
-          author_xpath = "diaspora/header/author_id"
+          raise MissingAuthor if author_elem.nil? || author_elem.content.empty?
-          magicenv_xpath = "diaspora/env"
+          slap.author_id = author_elem.content
+
+          slap.add_magic_env_from_doc(doc)
         end
-
-        slap = Slap.new
-
-        author_elem = doc.at_xpath(author_xpath, ns)
-        raise MissingAuthor if author_elem.nil? || author_elem.content.empty?
-        slap.author_id = author_elem.content
-
-        magic_env_elem = doc.at_xpath(magicenv_xpath, ns)
-        raise MissingMagicEnvelope if magic_env_elem.nil?
-        slap.magic_envelope = magic_env_elem
-
-        slap
       end
 
       # Creates an unencrypted Salmon Slap and returns the XML string.
@@ -94,25 +84,28 @@ module DiasporaFederation
                                    pkey.instance_of?(OpenSSL::PKey::RSA) &&
                                    entity.is_a?(Entity)
 
-        doc = Nokogiri::XML::Document.new
+        build_xml do |xml|
-        doc.encoding = "UTF-8"
+          xml.header {
+            xml.author_id(author_id)
+          }
 
-        root = Nokogiri::XML::Element.new("diaspora", doc)
+          MagicEnvelope.new(pkey, entity).envelop(xml)
-        root.default_namespace = Salmon::XMLNS
+        end
-        root.add_namespace("me", MagicEnvelope::XMLNS)
+      end
-        doc.root = root
 
-        header = Nokogiri::XML::Element.new("header", doc)
+      def self.build_xml
-        root << header
+        builder = Nokogiri::XML::Builder.new(encoding: "UTF-8") do |xml|
+          xml.diaspora("xmlns" => Salmon::XMLNS, "xmlns:me" => MagicEnvelope::XMLNS) {
+            yield xml
+          }
+        end
+        builder.to_xml
+      end
 
-        author = Nokogiri::XML::Element.new("author_id", doc)
+      def add_magic_env_from_doc(doc)
-        author.content = author_id
+        @magic_envelope = doc.at_xpath("d:diaspora/me:env", Slap::NS).tap do |env|
-        header << author
+          raise MissingMagicEnvelope if env.nil?
-
+        end
-        magic_envelope = MagicEnvelope.new(pkey, entity, root)
-        magic_envelope.envelop
-
-        doc.to_xml
       end
     end
   end

spec/lib/diaspora_federation/salmon/encrypted_slap_spec.rb

@@ -7,7 +7,7 @@ module DiasporaFederation
     let(:slap_xml) { Salmon::EncryptedSlap.generate_xml(author_id, pkey, entity, okey.public_key) }
     let(:ns) { {d: Salmon::XMLNS, me: Salmon::MagicEnvelope::XMLNS} }
 
-    context ".generate_xml" do
+    describe ".generate_xml" do
       context "sanity" do
         it "accepts correct params" do
           expect {
@@ -41,7 +41,7 @@ module DiasporaFederation
           JSON.parse(okey.private_decrypt(Base64.decode64(cipher_header["aes_key"])))
         }
 
-        it "encoded the header correctly" do
+        it "encodes the header correctly" do
           json_header = {}
           expect {
             json_header = JSON.parse(Base64.decode64(subject))
@@ -74,7 +74,7 @@ module DiasporaFederation
       end
     end
 
-    context ".from_xml" do
+    describe ".from_xml" do
       context "sanity" do
         it "accepts correct params" do
           expect {
@@ -92,7 +92,7 @@ module DiasporaFederation
 
         it "verifies the existence of 'encrypted_header'" do
           faulty_xml = <<XML
-<diaspora>
+<diaspora xmlns="https://joindiaspora.com/protocol" xmlns:me="http://salmon-protocol.org/ns/magic-env">
 </diaspora>
 XML
           expect {
@@ -102,7 +102,7 @@ XML
 
         it "verifies the existence of a magic envelope" do
           faulty_xml = <<XML
-<diaspora>
+<diaspora xmlns="https://joindiaspora.com/protocol" xmlns:me="http://salmon-protocol.org/ns/magic-env">
   <encrypted_header/>
 </diaspora>
 XML

spec/lib/diaspora_federation/salmon/magic_envelope_spec.rb

@@ -2,7 +2,16 @@ module DiasporaFederation
   describe Salmon::MagicEnvelope do
     let(:payload) { Entities::TestEntity.new(test: "asdf") }
     let(:pkey) { OpenSSL::PKey::RSA.generate(512) } # use small key for speedy specs
-    let(:envelope) { Salmon::MagicEnvelope.new(pkey, payload).envelop }
+    let(:envelope) { envelop_xml(Salmon::MagicEnvelope.new(pkey, payload)) }
+
+    def envelop_xml(magic_env)
+      builder = Nokogiri::XML::Builder.new(encoding: "UTF-8") do |xml|
+        xml.root("xmlns:me" => Salmon::MagicEnvelope::XMLNS) {
+          magic_env.envelop(xml)
+        }
+      end
+      builder.doc.at_xpath("//me:env")
+    end
 
     def sig_subj(env)
       data = Base64.urlsafe_decode64(env.at_xpath("me:data").content)
@@ -34,15 +43,15 @@ module DiasporaFederation
       end
     end
 
-    context "#envelop" do
+    describe "#envelop" do
       subject { Salmon::MagicEnvelope.new(pkey, payload) }
 
       it "should be an instance of Nokogiri::XML::Element" do
-        expect(subject.envelop).to be_an_instance_of Nokogiri::XML::Element
+        expect(envelop_xml(subject)).to be_an_instance_of Nokogiri::XML::Element
       end
 
       it "returns a magic envelope of correct structure" do
-        env = subject.envelop
+        env = envelop_xml(subject)
         expect(env.name).to eq("env")
 
         control = %w(data encoding alg sig)
@@ -55,7 +64,7 @@ module DiasporaFederation
       end
 
       it "signs the payload correctly" do
-        env = subject.envelop
+        env = envelop_xml(subject)
 
         subj = sig_subj(env)
         sig = Base64.urlsafe_decode64(env.at_xpath("me:sig").content)
@@ -64,14 +73,11 @@ module DiasporaFederation
       end
     end
 
-    context "#encrypt!" do
+    describe "#encrypt!" do
       subject { Salmon::MagicEnvelope.new(pkey, payload) }
 
       it "encrypts the payload, returning cipher params" do
-        params = {}
+        params = subject.encrypt!
-        expect {
-          params = subject.encrypt!
-        }.not_to raise_error
         expect(params).to include(:key, :iv)
       end
 
@@ -82,8 +88,8 @@ module DiasporaFederation
 
         cipher = OpenSSL::Cipher.new(Salmon::AES::CIPHER)
         cipher.encrypt
-        cipher.iv = Base64.decode64(params[:iv])
+        cipher.iv = params[:iv]
-        cipher.key = Base64.decode64(params[:key])
+        cipher.key = params[:key]
 
         ciphertext = cipher.update(plain_payload) + cipher.final
 
@@ -91,7 +97,7 @@ module DiasporaFederation
       end
     end
 
-    context ".unenvelop" do
+    describe ".unenvelop" do
       context "sanity" do
         it "works with sane input" do
           expect {
@@ -121,7 +127,7 @@ module DiasporaFederation
         end
 
         it "verifies the encoding" do
-          bad_env = Salmon::MagicEnvelope.new(pkey, payload).envelop
+          bad_env = envelop_xml(Salmon::MagicEnvelope.new(pkey, payload))
           elem = bad_env.at_xpath("me:encoding")
           elem.content = "invalid_enc"
           re_sign(bad_env, pkey)
@@ -131,7 +137,7 @@ module DiasporaFederation
         end
 
         it "verifies the algorithm" do
-          bad_env = Salmon::MagicEnvelope.new(pkey, payload).envelop
+          bad_env = envelop_xml(Salmon::MagicEnvelope.new(pkey, payload))
           elem = bad_env.at_xpath("me:alg")
           elem.content = "invalid_alg"
           re_sign(bad_env, pkey)
@@ -151,7 +157,7 @@ module DiasporaFederation
         env = Salmon::MagicEnvelope.new(pkey, payload)
         params = env.encrypt!
 
-        envelope = env.envelop
+        envelope = envelop_xml(env)
 
         entity = Salmon::MagicEnvelope.unenvelop(envelope, pkey.public_key, params)
         expect(entity).to be_an_instance_of Entities::TestEntity

spec/lib/diaspora_federation/salmon/slap_spec.rb

@@ -5,7 +5,7 @@ module DiasporaFederation
     let(:entity) { Entities::TestEntity.new(test: "qwertzuiop") }
     let(:slap) { Salmon::Slap.generate_xml(author_id, pkey, entity) }
 
-    context ".generate_xml" do
+    describe ".generate_xml" do
       context "sanity" do
         it "accepts correct params" do
           expect {
@@ -31,7 +31,7 @@ module DiasporaFederation
       end
     end
 
-    context ".from_xml" do
+    describe ".from_xml" do
       context "sanity" do
         it "accepts salmon xml as param" do
           expect {
@@ -49,7 +49,7 @@ module DiasporaFederation
 
         it "verifies the existence of an author_id" do
           faulty_xml = <<XML
-<diaspora>
+<diaspora xmlns="https://joindiaspora.com/protocol" xmlns:me="http://salmon-protocol.org/ns/magic-env">
   <header/>
 </diaspora>
 XML
@@ -60,7 +60,7 @@ XML
 
         it "verifies the existence of a magic envelope" do
           faulty_xml = <<-XML
-<diaspora>
+<diaspora xmlns="https://joindiaspora.com/protocol" xmlns:me="http://salmon-protocol.org/ns/magic-env">
   <header>
     <author_id>#{author_id}</author_id>
   </header>

spec/lib/diaspora_federation/salmon/xml_payload_spec.rb

@@ -3,7 +3,7 @@ module DiasporaFederation
     let(:entity) { Entities::TestEntity.new(test: "asdf") }
     let(:payload) { Salmon::XmlPayload.pack(entity) }
 
-    context ".pack" do
+    describe ".pack" do
       it "expects an Entity as param" do
         expect {
           Salmon::XmlPayload.pack(entity)
@@ -49,7 +49,7 @@ XML
       end
     end
 
-    context ".unpack" do
+    describe ".unpack" do
       context "sanity" do
         it "expects an Nokogiri::XML::Element as param" do
           expect {
@@ -124,7 +124,7 @@ XML
         end
       end
 
-      context ".entity_class_name" do
+      describe ".entity_class_name" do
         it "should parse a single word" do
           expect(Salmon::XmlPayload.send(:entity_class_name, "entity")).to eq("Entity")
         end