[Openvpn-devel,M] Change in openvpn[master]: misc: make get_auth_challenge static

Message ID 76c50ee7fb8a7f7f54543ce7b5abe5be887f126b-HTML@gerrit.openvpn.net
State Accepted
Headers show
Series [Openvpn-devel,M] Change in openvpn[master]: misc: make get_auth_challenge static | expand

Commit Message

flichtenheld (Code Review) Dec. 8, 2023, 4:29 p.m. UTC
Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/476?usp=email

to review the following change.


Change subject: misc: make get_auth_challenge static
......................................................................

misc: make get_auth_challenge static

Not used outside of misc.c.

Rename to parse_auth_challenge since it really just parses
the string that you put in into the struct.

Add doxygen documentation.

Change-Id: I0abeec9f862aea1f6a8fdf350fa0008cf2e5d613
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
---
M src/openvpn/misc.c
M src/openvpn/misc.h
M src/openvpn/ssl.h
3 files changed, 103 insertions(+), 85 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/76/476/1

Patch

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index ce6e4fd..9352843 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -124,6 +124,88 @@ 
     }
     return true;
 }
+
+/**
+ * Parses an authentication challenge string and returns an auth_challenge_info structure.
+ * The authentication challenge string should follow the dynamic challenge/response protocol.
+ *
+ * See doc/management-notes.txt for more info on the the dynamic challenge/response protocol
+ * implemented here.
+ *
+ * @param auth_challenge The authentication challenge string to parse.
+ * @param gc             The gc_arena structure for memory allocation.
+ *
+ * @return               A pointer to the parsed auth_challenge_info structure, or NULL if parsing fails.
+ */
+static struct auth_challenge_info *
+parse_auth_challenge(const char *auth_challenge, struct gc_arena *gc)
+{
+    if (auth_challenge)
+    {
+        struct auth_challenge_info *ac;
+        const int len = strlen(auth_challenge);
+        char *work = (char *) gc_malloc(len+1, false, gc);
+        char *cp;
+
+        struct buffer b;
+        buf_set_read(&b, (const uint8_t *)auth_challenge, len);
+
+        ALLOC_OBJ_CLEAR_GC(ac, struct auth_challenge_info, gc);
+
+        /* parse prefix */
+        if (!buf_parse(&b, ':', work, len))
+        {
+            return NULL;
+        }
+        if (strcmp(work, "CRV1"))
+        {
+            return NULL;
+        }
+
+        /* parse flags */
+        if (!buf_parse(&b, ':', work, len))
+        {
+            return NULL;
+        }
+        for (cp = work; *cp != '\0'; ++cp)
+        {
+            const char c = *cp;
+            if (c == 'E')
+            {
+                ac->flags |= CR_ECHO;
+            }
+            else if (c == 'R')
+            {
+                ac->flags |= CR_RESPONSE;
+            }
+        }
+
+        /* parse state ID */
+        if (!buf_parse(&b, ':', work, len))
+        {
+            return NULL;
+        }
+        ac->state_id = string_alloc(work, gc);
+
+        /* parse user name */
+        if (!buf_parse(&b, ':', work, len))
+        {
+            return NULL;
+        }
+        ac->user = (char *) gc_malloc(strlen(work)+1, true, gc);
+        openvpn_base64_decode(work, (void *)ac->user, -1);
+
+        /* parse challenge text */
+        ac->challenge_text = string_alloc(BSTR(&b), gc);
+
+        return ac;
+    }
+    else
+    {
+        return NULL;
+    }
+}
+
 #endif /* ifdef ENABLE_MANAGEMENT */
 
 /*
@@ -287,7 +369,7 @@ 
 #ifdef ENABLE_MANAGEMENT
             if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE) && response_from_stdin)
             {
-                struct auth_challenge_info *ac = get_auth_challenge(auth_challenge, &gc);
+                struct auth_challenge_info *ac = parse_auth_challenge(auth_challenge, &gc);
                 if (ac)
                 {
                     char *response = (char *) gc_malloc(USER_PASS_LEN, false, &gc);
@@ -392,83 +474,6 @@ 
     return true;
 }
 
-#ifdef ENABLE_MANAGEMENT
-
-/*
- * See management/management-notes.txt for more info on the
- * the dynamic challenge/response protocol implemented here.
- */
-struct auth_challenge_info *
-get_auth_challenge(const char *auth_challenge, struct gc_arena *gc)
-{
-    if (auth_challenge)
-    {
-        struct auth_challenge_info *ac;
-        const int len = strlen(auth_challenge);
-        char *work = (char *) gc_malloc(len+1, false, gc);
-        char *cp;
-
-        struct buffer b;
-        buf_set_read(&b, (const uint8_t *)auth_challenge, len);
-
-        ALLOC_OBJ_CLEAR_GC(ac, struct auth_challenge_info, gc);
-
-        /* parse prefix */
-        if (!buf_parse(&b, ':', work, len))
-        {
-            return NULL;
-        }
-        if (strcmp(work, "CRV1"))
-        {
-            return NULL;
-        }
-
-        /* parse flags */
-        if (!buf_parse(&b, ':', work, len))
-        {
-            return NULL;
-        }
-        for (cp = work; *cp != '\0'; ++cp)
-        {
-            const char c = *cp;
-            if (c == 'E')
-            {
-                ac->flags |= CR_ECHO;
-            }
-            else if (c == 'R')
-            {
-                ac->flags |= CR_RESPONSE;
-            }
-        }
-
-        /* parse state ID */
-        if (!buf_parse(&b, ':', work, len))
-        {
-            return NULL;
-        }
-        ac->state_id = string_alloc(work, gc);
-
-        /* parse user name */
-        if (!buf_parse(&b, ':', work, len))
-        {
-            return NULL;
-        }
-        ac->user = (char *) gc_malloc(strlen(work)+1, true, gc);
-        openvpn_base64_decode(work, (void *)ac->user, -1);
-
-        /* parse challenge text */
-        ac->challenge_text = string_alloc(BSTR(&b), gc);
-
-        return ac;
-    }
-    else
-    {
-        return NULL;
-    }
-}
-
-#endif /* ifdef ENABLE_MANAGEMENT */
-
 void
 purge_user_pass(struct user_pass *up, const bool force)
 {
diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h
index b000b72..98de90a 100644
--- a/src/openvpn/misc.h
+++ b/src/openvpn/misc.h
@@ -86,8 +86,6 @@ 
     const char *challenge_text;
 };
 
-struct auth_challenge_info *get_auth_challenge(const char *auth_challenge, struct gc_arena *gc);
-
 /*
  * Challenge response info on client as pushed by server.
  */
@@ -120,12 +118,31 @@ 
 
 #define GET_USER_PASS_INLINE_CREDS (1<<10)  /* indicates that auth_file is actually inline creds */
 
+/**
+ * Retrieves the user credentials from various sources depending on the flags.
+ *
+ * @param up The user_pass structure to store the retrieved credentials.
+ * @param auth_file The path to the authentication file. Might be NULL.
+ * @param prefix The prefix to prepend to user prompts.
+ * @param flags Additional flags to control the behavior of the function.
+ * @param auth_challenge The authentication challenge string.
+ * @return true if the user credentials were successfully retrieved, false otherwise.
+ */
 bool get_user_pass_cr(struct user_pass *up,
                       const char *auth_file,
                       const char *prefix,
                       const unsigned int flags,
                       const char *auth_challenge);
 
+/**
+ * Retrieves the user credentials from various sources depending on the flags.
+ *
+ * @param up The user_pass structure to store the retrieved credentials.
+ * @param auth_file The path to the authentication file. Might be NULL.
+ * @param prefix The prefix to prepend to user prompts.
+ * @param flags Additional flags to control the behavior of the function.
+ * @return true if the user credentials were successfully retrieved, false otherwise.
+ */
 static inline bool
 get_user_pass(struct user_pass *up,
               const char *auth_file,
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 6ba6ff8..71b99db 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -410,11 +410,7 @@ 
 bool ssl_clean_auth_token(void);
 
 #ifdef ENABLE_MANAGEMENT
-/*
- * ssl_get_auth_challenge will parse the server-pushed auth-failed
- * reason string and return a dynamically allocated
- * auth_challenge_info struct.
- */
+
 void ssl_purge_auth_challenge(void);
 
 void ssl_put_auth_challenge(const char *cr_str);