Fix OPENSSL_ia32cap parsing. This environment variable can be used to tweak the CPUID values and is reportedly used by `rr` to disable RDRAND support. This broke in 85145fdc91, however, when two values were given. Change-Id: I5ac8ce863f18242bfd9f37872c628e7e487052d5 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/79407 Reviewed-by: David Benjamin <davidben@google.com> Auto-Submit: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>

diff --git a/crypto/cpu_intel.cc b/crypto/cpu_intel.cc
index 4116012..dc83c9b 100644
--- a/crypto/cpu_intel.cc
+++ b/crypto/cpu_intel.cc

@@ -17,7 +17,9 @@
 #if !defined(OPENSSL_NO_ASM) && \
     (defined(OPENSSL_X86) || defined(OPENSSL_X86_64))
 
+#include <errno.h>
 #include <inttypes.h>
+#include <limits.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -89,8 +91,10 @@
 }
 
 // handle_cpu_env applies the value from |in| to the CPUID values in |out[0]|
-// and |out[1]|. See the comment in |OPENSSL_cpuid_setup| about this.
-static void handle_cpu_env(uint32_t *out, const char *in) {
+// and |out[1]|. See the comment in |OPENSSL_cpuid_setup| about this. The
+// |is_last| argument specifies whether the value is at the end of the string.
+// Otherwise it may be followed by a colon.
+static void handle_cpu_env(uint32_t *out, const char *in, bool is_last) {
   const int invert_op = in[0] == '~';
   const int or_op = in[0] == '|';
   const int skip_first_byte = invert_op || or_op;
@@ -99,9 +103,13 @@
 
   const char *start = in + skip_first_byte;
   char *end;
+  errno = 0;
+  // We need to parse 64-bit values with `strtoull`.
+  static_assert(sizeof(unsigned long long) == sizeof(uint64_t));
   unsigned long long v = strtoull(start, &end, base);
 
-  if (end == start || *end != '\0') {
+  if (end == start || (*end != '\0' && (is_last || *end != ':')) ||
+      (v == ULLONG_MAX && errno == ERANGE)) {
     return;
   }
 
@@ -124,11 +132,11 @@
 
   uint32_t num_ids = eax;
 
-  int is_intel = ebx == 0x756e6547 /* Genu */ && //
-                 edx == 0x49656e69 /* ineI */ && //
+  int is_intel = ebx == 0x756e6547 /* Genu */ &&  //
+                 edx == 0x49656e69 /* ineI */ &&  //
                  ecx == 0x6c65746e /* ntel */;
-  int is_amd = ebx == 0x68747541 /* Auth */ && //
-               edx == 0x69746e65 /* enti */ && //
+  int is_amd = ebx == 0x68747541 /* Auth */ &&  //
+               edx == 0x69746e65 /* enti */ &&  //
                ecx == 0x444d4163 /* cAMD */;
 
   uint32_t extended_features[2] = {0};
@@ -267,10 +275,10 @@
   // The first value determines OPENSSL_ia32cap_P[0] and [1]. The second [2]
   // and [3].
 
-  handle_cpu_env(&OPENSSL_ia32cap_P[0], env1);
+  handle_cpu_env(&OPENSSL_ia32cap_P[0], env1, /*is_last=*/false);
   env2 = strchr(env1, ':');
   if (env2 != NULL) {
-    handle_cpu_env(&OPENSSL_ia32cap_P[2], env2 + 1);
+    handle_cpu_env(&OPENSSL_ia32cap_P[2], env2 + 1, /*is_last=*/true);
   }
 }
 

diff --git a/crypto/crypto_test.cc b/crypto/crypto_test.cc
index fe5a2f6..3fa9d36 100644
--- a/crypto/crypto_test.cc
+++ b/crypto/crypto_test.cc

@@ -17,16 +17,25 @@
 
 #include <string>
 
-#include <openssl/base.h>
 #include <openssl/aead.h>
-#include <openssl/crypto.h>
+#include <openssl/base.h>
 #include <openssl/cipher.h>
+#include <openssl/crypto.h>
 #include <openssl/mem.h>
 
 #include <gtest/gtest.h>
 
 #include "internal.h"
 
+#if (defined(OPENSSL_X86) || defined(OPENSSL_X86_64)) && \
+    defined(OPENSSL_LINUX) && !defined(BORINGSSL_SHARED_LIBRARY)
+#define TEST_CPUID_ENVVAR
+
+#include <unistd.h>
+#include <sys/wait.h>
+#include <errno.h>
+
+#endif
 
 // Test that OPENSSL_VERSION_NUMBER and OPENSSL_VERSION_TEXT are consistent.
 // Node.js parses the version out of OPENSSL_VERSION_TEXT instead of using
@@ -159,9 +168,7 @@
 }
 
 #if defined(BORINGSSL_FIPS) && !defined(OPENSSL_ASAN)
-TEST(Crypto, OnDemandIntegrityTest) {
-  BORINGSSL_integrity_test();
-}
+TEST(Crypto, OnDemandIntegrityTest) { BORINGSSL_integrity_test(); }
 #endif
 
 OPENSSL_DEPRECATED static void DeprecatedFunction() {}
@@ -172,3 +179,46 @@
   DeprecatedFunction();
 }
 OPENSSL_END_ALLOW_DEPRECATED
+
+#if defined(TEST_CPUID_ENVVAR)
+TEST(Crypto, CPUIDEnvVariable) {
+  constexpr auto is_rdrand_set = []() -> bool {
+    return OPENSSL_get_ia32cap(1) & 0x40000000;
+  };
+
+  // This test execs itself and sets `CPUIDEnvVariable` in the child's
+  // environment. So, if that's set, this is the child process.
+  constexpr uint8_t kSuccessExitStatus = 81;
+  if (getenv("CPUIDEnvVariable")) {
+    _exit(is_rdrand_set() ? 1 : kSuccessExitStatus);
+  }
+
+  // If RDRAND isn't actually supported on this system, then this test is moot.
+  if (!is_rdrand_set()) {
+    GTEST_SKIP();
+  }
+
+  // Fork off a child process and set the `OPENSSL_ia32cap` environment
+  // variable such that RDRAND should _not_ be supported in the child.
+  const pid_t pid = fork();
+  if (pid == 0) {
+    const char *kArgs[] = {"/proc/self/exe",
+                           "--gtest_filter=Crypto.CPUIDEnvVariable", nullptr};
+    const char *kEnv[] = {"OPENSSL_ia32cap=~0x4000000000000000:0",
+                          "CPUIDEnvVariable=1", nullptr};
+    execve(kArgs[0], const_cast<char **>(kArgs), const_cast<char **>(kEnv));
+    _exit(1);
+  }
+
+  ASSERT_GT(pid, 0);
+  pid_t waited;
+  int status;
+  do {
+    waited = waitpid(pid, &status, 0);
+  } while (waited == -1 && errno == EINTR);
+
+  EXPECT_EQ(waited, pid);
+  EXPECT_TRUE(WIFEXITED(status));
+  EXPECT_EQ(WEXITSTATUS(status), kSuccessExitStatus);
+}
+#endif