Merge pull request #1772 from greenbone/function_set_improvements

`function_set!` improvements

rust/crates/nasl-function-proc-macro/src/codegen.rs

@@ -33,6 +33,12 @@ impl<'a> ArgsStruct<'a> {
         })
     }
 
+    fn has_register_arg(&self) -> bool {
+        self.args
+            .iter()
+            .any(|arg| matches!(arg.kind, ArgKind::Register))
+    }
+
     fn get_args(&self) -> TokenStream {
         self
             .args.iter().map(|arg| {
@@ -146,6 +152,9 @@ impl<'a> ArgsStruct<'a> {
     }
 
     fn gen_checks(&self) -> TokenStream {
+        if self.has_register_arg() {
+            return quote! {};
+        }
         let named_array = self.make_array_of_names(ArgKind::get_named_arg_name);
         let maybe_named_array = self.make_array_of_names(ArgKind::get_maybe_named_arg_name);
         let num_allowed_positional_args = if self.has_positional_iterator_arg() {
@@ -160,6 +169,43 @@ impl<'a> ArgsStruct<'a> {
         }
     }
 
+    fn impl_add_to_set(
+        &self,
+        ident: &Ident,
+        fn_name: &Ident,
+        asyncness: Option<Async>,
+    ) -> TokenStream {
+        let nasl_function_expr = match (asyncness, &self.receiver_type) {
+            (Some(_), ReceiverType::None) => {
+                quote! { AsyncStateless(Box::new(#fn_name)) }
+            }
+            (Some(_), ReceiverType::RefSelf) => {
+                quote! { AsyncStateful(Box::new(Self::#fn_name)) }
+            }
+            (Some(_), ReceiverType::RefMutSelf) => {
+                quote! { AsyncStatefulMut(Box::new(Self::#fn_name)) }
+            }
+            (None, ReceiverType::None) => quote! { SyncStateless(#fn_name) },
+            (None, ReceiverType::RefSelf) => {
+                quote! { SyncStateful(Self::#fn_name) }
+            }
+            (None, ReceiverType::RefMutSelf) => {
+                quote! { SyncStatefulMut(Self::#fn_name) }
+            }
+        };
+
+        let (generics, state_type) = match &self.receiver_type {
+            ReceiverType::None => (quote! { < S > }, quote! { S }),
+            ReceiverType::RefSelf | ReceiverType::RefMutSelf => (quote! {}, quote! { Self }),
+        };
+
+        quote! {
+            fn #ident #generics (set: &mut crate::nasl::utils::StoredFunctionSet<#state_type>, name: &str) {
+                set.add_nasl_function(name, crate::nasl::utils::NaslFunction::#nasl_function_expr);
+            }
+        }
+    }
+
     pub fn impl_nasl_function_args(&self) -> TokenStream {
         let ItemFn {
             attrs,
@@ -193,21 +239,26 @@ impl<'a> ArgsStruct<'a> {
         };
         let asyncness = sig.asyncness;
         let checks = self.gen_checks();
-        let mangled_name = format!("_internal_{}", ident);
-        let mangled_ident = Ident::new(&mangled_name, ident.span());
-        let inner_call = self.get_inner_call_expr(&mangled_ident, asyncness);
+        let mangled_ident_original_fn = Ident::new(&format!("_internal_{}", ident), ident.span());
+        let mangled_ident_transformed_fn =
+            Ident::new(&(format!("_internal_convert_{}", ident)), ident.span());
+        let inner_call = self.get_inner_call_expr(&mangled_ident_original_fn, asyncness);
+        let add_to_set = self.impl_add_to_set(ident, &mangled_ident_transformed_fn, asyncness);
+
         quote! {
             #[allow(clippy::too_many_arguments)]
-            #asyncness fn #mangled_ident #generics ( #fn_args ) -> #output_ty {
+            #asyncness fn #mangled_ident_original_fn #generics ( #fn_args ) -> #output_ty {
                 #(#stmts)*
             }
 
-            #(#attrs)* #vis #asyncness #fn_token #ident #generics ( #inputs ) -> crate::nasl::NaslResult {
+            #(#attrs)* #vis #asyncness #fn_token #mangled_ident_transformed_fn #generics ( #inputs ) -> crate::nasl::NaslResult {
                 #checks
                 #get_args
                 let _result = #inner_call;
                 <#output_ty as crate::nasl::ToNaslResult>::to_nasl_result(_result)
             }
+
+            #add_to_set
         }
     }
 }

rust/crates/nasl-function-proc-macro/src/lib.rs

@@ -1,3 +1,72 @@
+//! This crate provides the `nasl_function` proc macro, which is
+//! designed to make implementing new NASL builtin functions as
+//! convenient as possible.
+//!
+//! Design: There are two main purposes that the `nasl_function` macro
+//! serves.
+//!
+//! Purpose 1: Unify argument handling.
+//!
+//! The `nasl_function!` macro provides a structured approach to argument handling
+//! within NASL builtin functions. The macro takes as input a function
+//! taking any number of arguments, along with instructions on whether
+//! those arguments are named, positional, optional, etc. It then
+//! produces a function that automatically handles conversion of the
+//! arguments into the correct types and produces consistent error
+//! messages if the function has been called with an invalid set of
+//! arguments.
+//!
+//! To do so, the macro transforms the annotated function into a function
+//! taking `&Context` and `&Register` as arguments (plus self arguments
+//! if needed) and then calls the original function from within the transformed
+//! function, deriving each argument from the `FromNaslValue` implementation
+//! of its type and handling optional and named arguments appropriately.
+//!
+//! The macro renames the inner function into a proper, first class
+//! function instead of a closure in order to provide support for
+//! async functions (without relying on the unstable async
+//! closures).
+//!
+//! Purpose 2: Provide a uniform way to add builtin functions to function sets.
+//!
+//! NASL builtin functions come in one of several types, depending on
+//! their asyncness and whether they are stateless or stateful (and
+//! whether they require mutable access to their state, if they
+//! are). The `NaslFunction` type defined in the executor code is a
+//! singular type which can represent all the various variants of
+//! builtin functions.  The executor also provides the
+//! `StoredFunctionSet`, which represents a set of `NaslFunction`s
+//! together with their state. This state struct is used both as the
+//! actual state that these functions require, as well as an
+//! identifying name.  Together, the `NaslFunction` and
+//! `StoredFunctionSet` types provide the ability to store NASL
+//! functions in a type-erased way, so that the interpreter can run
+//! them independently of their properties.
+//!
+//! In order to provide a unified interface for adding NASL functions
+//! to `StoredFunctionSet`s, there needs to be a way to convert any of
+//! the 6 variants which builtin functions come in (sync_stateless,
+//! async_stateless, sync_stateful, ... ) into their corresponding
+//! variant of `NaslFunction`. On the surface, this problem sounds
+//! simple: Simply implement `Into<NaslFunction>` for `Fn(&Context,
+//! &Register) -> NaslResult` as well as for `Fn(&Context, &Register)
+//! -> Future<NaslResult>`, as well as for the other 4 variants. Then
+//! provide a `add_function` method on `StoredFunctionSet` that takes
+//! any `impl Into<NaslFunction>` as argument. The problem with this
+//! approach is that the Rust compiler cannot determine that these 6
+//! implementations are coherent, i.e. it believes that there might be
+//! a type `T` that implements multiple of these `Fn` traits
+//! simultaneously, which would result in overlapping trait impls.
+//!
+//! In order to solve this problem, the `nasl_function!` macro
+//! transforms the annotated function into a special function that
+//! takes a `StoredFunctionSet` and adds the correct variant of
+//! `NaslFunction` to the set. This is a very indirect approach, but
+//! it works because the `nasl_function!` macro knows exactly what the
+//! signature of the annotated function is and can therefore derive
+//! which of the 6 variants of `NaslFunction` it should become,
+//! without requiring type-erasure via an intermediate trait.
+
 mod codegen;
 mod error;
 mod parse;

rust/src/nasl/builtin/array/mod.rs

@@ -84,7 +84,6 @@ pub struct Array;
 
 function_set! {
     Array,
-    sync_stateless,
     (
         make_array,
         make_list,

rust/src/nasl/builtin/cert/mod.rs

@@ -437,7 +437,6 @@ impl NaslCerts {
 
 function_set! {
     NaslCerts,
-    sync_stateful,
     (
         (NaslCerts::cert_open, "cert_open"),
         (NaslCerts::cert_close, "cert_close"),

rust/src/nasl/builtin/cryptographic/aes_cbc.rs

@@ -71,7 +71,8 @@ where
 ///   Currently the data is filled with zeroes. Therefore the length of the encrypted data must be
 ///   known for decryption. If no length is given, the last block is decrypted as a whole.
 /// - The iv must have a length of 16 bytes
-fn aes128_cbc_encrypt(register: &Register, _: &Context) -> Result<NaslValue, FnError> {
+#[nasl_function]
+fn aes128_cbc_encrypt(register: &Register) -> Result<NaslValue, FnError> {
     cbc::<Aes128>(register, Crypt::Encrypt)
 }
 
@@ -83,7 +84,8 @@ fn aes128_cbc_encrypt(register: &Register, _: &Context) -> Result<NaslValue, FnE
 ///   Currently the data is filled with zeroes. Therefore the length of the encrypted data must be
 ///   known for decryption. If no length is given, the last block is decrypted as a whole.
 /// - The iv must have a length of 16 bytes
-fn aes128_cbc_decrypt(register: &Register, _: &Context) -> Result<NaslValue, FnError> {
+#[nasl_function]
+fn aes128_cbc_decrypt(register: &Register) -> Result<NaslValue, FnError> {
     cbc::<Aes128>(register, Crypt::Decrypt)
 }
 
@@ -94,7 +96,8 @@ fn aes128_cbc_decrypt(register: &Register, _: &Context) -> Result<NaslValue, FnE
 ///   Currently the data is filled with zeroes. Therefore the length of the encrypted data must be
 ///   known for decryption. If no length is given, the last block is decrypted as a whole.
 /// - The iv must have a length of 16 bytes
-fn aes192_cbc_encrypt(register: &Register, _: &Context) -> Result<NaslValue, FnError> {
+#[nasl_function]
+fn aes192_cbc_encrypt(register: &Register) -> Result<NaslValue, FnError> {
     cbc::<Aes192>(register, Crypt::Encrypt)
 }
 
@@ -106,7 +109,8 @@ fn aes192_cbc_encrypt(register: &Register, _: &Context) -> Result<NaslValue, FnE
 ///   Currently the data is filled with zeroes. Therefore the length of the encrypted data must be
 ///   known for decryption. If no length is given, the last block is decrypted as a whole.
 /// - The iv must have a length of 16 bytes
-fn aes192_cbc_decrypt(register: &Register, _: &Context) -> Result<NaslValue, FnError> {
+#[nasl_function]
+fn aes192_cbc_decrypt(register: &Register) -> Result<NaslValue, FnError> {
     cbc::<Aes192>(register, Crypt::Decrypt)
 }
 
@@ -117,7 +121,8 @@ fn aes192_cbc_decrypt(register: &Register, _: &Context) -> Result<NaslValue, FnE
 ///   Currently the data is filled with zeroes. Therefore the length of the encrypted data must be
 ///   known for decryption. If no length is given, the last block is decrypted as a whole.
 /// - The iv must have a length of 16 bytes
-fn aes256_cbc_encrypt(register: &Register, _: &Context) -> Result<NaslValue, FnError> {
+#[nasl_function]
+fn aes256_cbc_encrypt(register: &Register) -> Result<NaslValue, FnError> {
     cbc::<Aes256>(register, Crypt::Encrypt)
 }
 
@@ -129,15 +134,15 @@ fn aes256_cbc_encrypt(register: &Register, _: &Context) -> Result<NaslValue, FnE
 ///   Currently the data is filled with zeroes. Therefore the length of the encrypted data must be
 ///   known for decryption. If no length is given, the last block is decrypted as a whole.
 /// - The iv must have a length of 16 bytes
-fn aes256_cbc_decrypt(register: &Register, _: &Context) -> Result<NaslValue, FnError> {
+#[nasl_function]
+fn aes256_cbc_decrypt(register: &Register) -> Result<NaslValue, FnError> {
     cbc::<Aes256>(register, Crypt::Decrypt)
 }
 
 pub struct AesCbc;
 
 function_set! {
     AesCbc,
-    sync_stateless,
     (
         aes128_cbc_encrypt,
         aes128_cbc_decrypt,