Deprecate the "hw" configuration options, make "padlockeng" disablable

The "hw" and "hw-.*" style options are historical artifacts, sprung from the time when ENGINE was first designed, with hardware crypto accelerators and HSMs in mind. Today, these options have largely lost their value, replaced by options such as "no-{foo}eng" and "no-engine". This completes the transition by making "hw" and "hw-.*" deprecated, but automatically translated into more modern variants of the same. In the process, we get rid of the last regular expression in Configure's @disablables, a feature that was ill supported anyway. Also, padlock now gets treated just as every other engine. Reviewed-by: Paul Dale <[email protected]> (Merged from openssl#8380)
shawnl · Mar 5, 2019 · 469ce8f · 469ce8f
1 parent e3b35d2
commit 469ce8f
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 76 deletions.
diff --git a/Configure b/Configure
@@ -374,7 +374,6 @@ my @disablables = (
     "fuzz-afl",
     "gost",
     "heartbeats",
-    "hw(-.+)?",
     "idea",
     "makedepend",
     "md2",
@@ -386,6 +385,7 @@ my @disablables = (
     "pinshared",
     "ocb",
     "ocsp",
+    "padlockeng",
     "pic",
     "poly1305",
     "posix-io",
@@ -434,6 +434,8 @@ foreach my $proto ((@tls, @dtls))
 my %deprecated_disablables = (
     "ssl2" => undef,
     "buf-freelists" => undef,
+    "hw" => "hw",               # causes cascade, but no macro
+    "hw-padlock" => "padlockeng",
     "ripemd" => "rmd160",
     "ui" => "ui-console",
     );
@@ -495,7 +497,9 @@ my @disable_cascades = (
     # Without position independent code, there can be no shared libraries or DSOs
     "pic"               => [ "shared" ],
     "shared"            => [ "dynamic-engine" ],
-    "engine"            => [ "afalgeng", "devcryptoeng" ],
+
+    "engine"            => [ grep /eng$/, @disablables ],
+    "hw"                => [ "padlockeng" ],
 
     # no-autoalginit is only useful when building non-shared
     "autoalginit"       => [ "shared", "apps" ],
@@ -674,8 +678,9 @@ while (@argvcopy)
         if (/^(no|disable|enable)-(.+)$/)
                 {
                 my $word = $2;
-                if (!exists $deprecated_disablables{$word}
-                        && !grep { $word =~ /^${_}$/ } @disablables)
+                if ($word !~ m|hw(?:-.+)| # special treatment for hw regexp opt
+                        && !exists $deprecated_disablables{$word}
+                        && !grep { $word eq $_ } @disablables)
                         {
                         $unsupported_options{$_} = 1;
                         next;
@@ -729,6 +734,10 @@ while (@argvcopy)
                                 $disabled{$deprecated_disablables{$1}} = "option";
                                 }
                         }
+                elsif ($1 =~ m|hw(?:-.+)|) # deprecate hw options in regexp form
+                        {
+                        $deprecated_options{$_} = 1;
+                        }
                 else
                         {
                         $disabled{$1} = "option";
@@ -1193,6 +1202,10 @@ $config{build_file} = env('BUILDFILE') || $target{build_file} || "Makefile";
 my %skipdir = ();
 my %disabled_info = ();         # For configdata.pm
 foreach my $what (sort keys %disabled) {
+    # There are deprecated disablables that translate to themselves.
+    # They cause disabling cascades, but should otherwise not regiter.
+    next if $deprecated_disablables{$what};
+
     $config{options} .= " no-$what";
 
     if (!grep { $what eq $_ } ( 'dso', 'threads', 'shared', 'pic',

diff --git a/INSTALL b/INSTALL
@@ -396,9 +396,6 @@
                    available if the GOST algorithms are also available through
                    loading an externally supplied engine.
 
-  no-hw-padlock
-                   Don't build the padlock engine.
-
   no-makedepend
                    Don't generate dependencies.
 
@@ -413,6 +410,11 @@
   no-ocsp
                    Don't build support for OCSP.
 
+  no-padlockeng
+  no-hw-padlock
+                   Don't build the padlock engine.
+                   ('no-hw-padlock' is deprecated and should not be used)
+
   no-pic
                    Don't build with support for Position Independent Code.
 

diff --git a/crypto/init.c b/crypto/init.c
@@ -389,7 +389,7 @@ DEFINE_RUN_ONCE_STATIC(ossl_init_engine_devcrypto)
     return 1;
 }
 #  endif
-#  if !defined(OPENSSL_NO_HW) && !defined(OPENSSL_NO_HW_PADLOCK)
+#  if !defined(OPENSSL_NO_PADLOCKENG)
 static CRYPTO_ONCE engine_padlock = CRYPTO_ONCE_STATIC_INIT;
 DEFINE_RUN_ONCE_STATIC(ossl_init_engine_padlock)
 {
@@ -761,7 +761,7 @@ int OPENSSL_init_crypto(uint64_t opts, const OPENSSL_INIT_SETTINGS *settings)
             && !RUN_ONCE(&engine_devcrypto, ossl_init_engine_devcrypto))
         return 0;
 #  endif
-#  if !defined(OPENSSL_NO_HW) && !defined(OPENSSL_NO_HW_PADLOCK)
+#  if !defined(OPENSSL_NO_PADLOCKENG)
     if ((opts & OPENSSL_INIT_ENGINE_PADLOCK)
             && !RUN_ONCE(&engine_padlock, ossl_init_engine_padlock))
         return 0;

diff --git a/engines/build.info b/engines/build.info
@@ -1,9 +1,10 @@
 IF[{- !$disabled{"engine"} -}]
-
   IF[{- $disabled{"dynamic-engine"} -}]
     LIBS=../libcrypto
-    SOURCE[../libcrypto]=\
-            e_padlock.c {- $target{padlock_asm_src} -}
+    IF[{- !$disabled{padlockeng} -}]
+      SOURCE[../libcrypto]=\
+              e_padlock.c {- $target{padlock_asm_src} -}
+    ENDIF
     IF[{- !$disabled{capieng} -}]
       SOURCE[../libcrypto]=e_capi.c
     ENDIF
@@ -14,13 +15,18 @@ IF[{- !$disabled{"engine"} -}]
       SOURCE[../libcrypto]=e_devcrypto.c
     ENDIF
   ELSE
-    MODULES{engine}=padlock
-    SOURCE[padlock]=e_padlock.c {- $target{padlock_asm_src} -}
-    DEPEND[padlock]=../libcrypto
-    INCLUDE[padlock]=../include
-    IF[{- defined $target{shared_defflag} -}]
-      SHARED_SOURCE[padlock]=padlock.ld
-      GENERATE[padlock.ld]=../util/engines.num
+    IF[{- !$disabled{padlockeng} -}]
+      MODULES{engine}=padlock
+      SOURCE[padlock]=e_padlock.c {- $target{padlock_asm_src} -}
+      DEPEND[padlock]=../libcrypto
+      INCLUDE[padlock]=../include
+      GENERATE[e_padlock-x86.s]=asm/e_padlock-x86.pl \
+        $(PERLASM_SCHEME) $(LIB_CFLAGS) $(LIB_CPPFLAGS) $(PROCESSOR)
+      GENERATE[e_padlock-x86_64.s]=asm/e_padlock-x86_64.pl $(PERLASM_SCHEME)
+      IF[{- defined $target{shared_defflag} -}]
+        SHARED_SOURCE[padlock]=padlock.ld
+        GENERATE[padlock.ld]=../util/engines.num
+      ENDIF
     ENDIF
     IF[{- !$disabled{capieng} -}]
       MODULES{engine}=capi
@@ -69,8 +75,4 @@ IF[{- !$disabled{"engine"} -}]
       GENERATE[ossltest.ld]=../util/engines.num
     ENDIF
   ENDIF
-
-  GENERATE[e_padlock-x86.s]=asm/e_padlock-x86.pl \
-        $(PERLASM_SCHEME) $(LIB_CFLAGS) $(LIB_CPPFLAGS) $(PROCESSOR)
-  GENERATE[e_padlock-x86_64.s]=asm/e_padlock-x86_64.pl $(PERLASM_SCHEME)
 ENDIF
diff --git a/engines/e_padlock.c b/engines/e_padlock.c
@@ -19,40 +19,39 @@
 #include <openssl/err.h>
 #include <openssl/modes.h>
 
-#ifndef OPENSSL_NO_HW
-# ifndef OPENSSL_NO_HW_PADLOCK
+#ifndef OPENSSL_NO_PADLOCKENG
 
 /*
  * VIA PadLock AES is available *ONLY* on some x86 CPUs. Not only that it
  * doesn't exist elsewhere, but it even can't be compiled on other platforms!
  */
 
-#  undef COMPILE_HW_PADLOCK
-#  if defined(PADLOCK_ASM)
-#   define COMPILE_HW_PADLOCK
-#   ifdef OPENSSL_NO_DYNAMIC_ENGINE
+# undef COMPILE_PADLOCKENG
+# if defined(PADLOCK_ASM)
+#  define COMPILE_PADLOCKENG
+#  ifdef OPENSSL_NO_DYNAMIC_ENGINE
 static ENGINE *ENGINE_padlock(void);
-#   endif
 #  endif
+# endif
 
-#  ifdef OPENSSL_NO_DYNAMIC_ENGINE
+# ifdef OPENSSL_NO_DYNAMIC_ENGINE
 void engine_load_padlock_int(void);
 void engine_load_padlock_int(void)
 {
 /* On non-x86 CPUs it just returns. */
-#   ifdef COMPILE_HW_PADLOCK
+#  ifdef COMPILE_PADLOCKENG
     ENGINE *toadd = ENGINE_padlock();
     if (!toadd)
         return;
     ENGINE_add(toadd);
     ENGINE_free(toadd);
     ERR_clear_error();
-#   endif
+#  endif
 }
 
-#  endif
+# endif
 
-#  ifdef COMPILE_HW_PADLOCK
+# ifdef COMPILE_PADLOCKENG
 
 /* Function for ENGINE detection and control */
 static int padlock_available(void);
@@ -106,7 +105,7 @@ static int padlock_bind_helper(ENGINE *e)
     return 1;
 }
 
-#   ifdef OPENSSL_NO_DYNAMIC_ENGINE
+#  ifdef OPENSSL_NO_DYNAMIC_ENGINE
 /* Constructor */
 static ENGINE *ENGINE_padlock(void)
 {
@@ -123,7 +122,7 @@ static ENGINE *ENGINE_padlock(void)
 
     return eng;
 }
-#   endif
+#  endif
 
 /* Check availability of the engine */
 static int padlock_init(ENGINE *e)
@@ -135,7 +134,7 @@ static int padlock_init(ENGINE *e)
  * This stuff is needed if this ENGINE is being compiled into a
  * self-contained shared-library.
  */
-#   ifndef OPENSSL_NO_DYNAMIC_ENGINE
+#  ifndef OPENSSL_NO_DYNAMIC_ENGINE
 static int padlock_bind_fn(ENGINE *e, const char *id)
 {
     if (id && (strcmp(id, padlock_id) != 0)) {
@@ -151,14 +150,14 @@ static int padlock_bind_fn(ENGINE *e, const char *id)
 
 IMPLEMENT_DYNAMIC_CHECK_FN()
 IMPLEMENT_DYNAMIC_BIND_FN(padlock_bind_fn)
-#   endif                       /* !OPENSSL_NO_DYNAMIC_ENGINE */
+#  endif                       /* !OPENSSL_NO_DYNAMIC_ENGINE */
 /* ===== Here comes the "real" engine ===== */
 
 /* Some AES-related constants */
-#   define AES_BLOCK_SIZE          16
-#   define AES_KEY_SIZE_128        16
-#   define AES_KEY_SIZE_192        24
-#   define AES_KEY_SIZE_256        32
+#  define AES_BLOCK_SIZE          16
+#  define AES_KEY_SIZE_128        16
+#  define AES_KEY_SIZE_192        24
+#  define AES_KEY_SIZE_256        32
     /*
      * Here we store the status information relevant to the current context.
      */
@@ -224,29 +223,29 @@ static int padlock_available(void)
 
 /* ===== AES encryption/decryption ===== */
 
-#   if defined(NID_aes_128_cfb128) && ! defined (NID_aes_128_cfb)
-#    define NID_aes_128_cfb NID_aes_128_cfb128
-#   endif
+#  if defined(NID_aes_128_cfb128) && ! defined (NID_aes_128_cfb)
+#   define NID_aes_128_cfb NID_aes_128_cfb128
+#  endif
 
-#   if defined(NID_aes_128_ofb128) && ! defined (NID_aes_128_ofb)
-#    define NID_aes_128_ofb NID_aes_128_ofb128
-#   endif
+#  if defined(NID_aes_128_ofb128) && ! defined (NID_aes_128_ofb)
+#   define NID_aes_128_ofb NID_aes_128_ofb128
+#  endif
 
-#   if defined(NID_aes_192_cfb128) && ! defined (NID_aes_192_cfb)
-#    define NID_aes_192_cfb NID_aes_192_cfb128
-#   endif
+#  if defined(NID_aes_192_cfb128) && ! defined (NID_aes_192_cfb)
+#   define NID_aes_192_cfb NID_aes_192_cfb128
+#  endif
 
-#   if defined(NID_aes_192_ofb128) && ! defined (NID_aes_192_ofb)
-#    define NID_aes_192_ofb NID_aes_192_ofb128
-#   endif
+#  if defined(NID_aes_192_ofb128) && ! defined (NID_aes_192_ofb)
+#   define NID_aes_192_ofb NID_aes_192_ofb128
+#  endif
 
-#   if defined(NID_aes_256_cfb128) && ! defined (NID_aes_256_cfb)
-#    define NID_aes_256_cfb NID_aes_256_cfb128
-#   endif
+#  if defined(NID_aes_256_cfb128) && ! defined (NID_aes_256_cfb)
+#   define NID_aes_256_cfb NID_aes_256_cfb128
+#  endif
 
-#   if defined(NID_aes_256_ofb128) && ! defined (NID_aes_256_ofb)
-#    define NID_aes_256_ofb NID_aes_256_ofb128
-#   endif
+#  if defined(NID_aes_256_ofb128) && ! defined (NID_aes_256_ofb)
+#   define NID_aes_256_ofb NID_aes_256_ofb128
+#  endif
 
 /* List of supported ciphers. */
 static const int padlock_cipher_nids[] = {
@@ -276,9 +275,9 @@ static int padlock_cipher_nids_num = (sizeof(padlock_cipher_nids) /
 static int padlock_aes_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key,
                                 const unsigned char *iv, int enc);
 
-#   define NEAREST_ALIGNED(ptr) ( (unsigned char *)(ptr) +         \
+#  define NEAREST_ALIGNED(ptr) ( (unsigned char *)(ptr) +         \
         ( (0x10 - ((size_t)(ptr) & 0x0F)) & 0x0F )      )
-#   define ALIGNED_CIPHER_DATA(ctx) ((struct padlock_cipher_data *)\
+#  define ALIGNED_CIPHER_DATA(ctx) ((struct padlock_cipher_data *)\
         NEAREST_ALIGNED(EVP_CIPHER_CTX_get_cipher_data(ctx)))
 
 static int
@@ -453,17 +452,17 @@ padlock_ctr_cipher(EVP_CIPHER_CTX *ctx, unsigned char *out_arg,
     return 1;
 }
 
-#   define EVP_CIPHER_block_size_ECB       AES_BLOCK_SIZE
-#   define EVP_CIPHER_block_size_CBC       AES_BLOCK_SIZE
-#   define EVP_CIPHER_block_size_OFB       1
-#   define EVP_CIPHER_block_size_CFB       1
-#   define EVP_CIPHER_block_size_CTR       1
+#  define EVP_CIPHER_block_size_ECB       AES_BLOCK_SIZE
+#  define EVP_CIPHER_block_size_CBC       AES_BLOCK_SIZE
+#  define EVP_CIPHER_block_size_OFB       1
+#  define EVP_CIPHER_block_size_CFB       1
+#  define EVP_CIPHER_block_size_CTR       1
 
 /*
  * Declaring so many ciphers by hand would be a pain. Instead introduce a bit
  * of preprocessor magic :-)
  */
-#   define DECLARE_AES_EVP(ksize,lmode,umode)      \
+#  define DECLARE_AES_EVP(ksize,lmode,umode)      \
 static EVP_CIPHER *_hidden_aes_##ksize##_##lmode = NULL; \
 static const EVP_CIPHER *padlock_aes_##ksize##_##lmode(void) \
 {                                                                       \
@@ -626,12 +625,12 @@ padlock_aes_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key,
             AES_set_decrypt_key(key, key_len, &cdata->ks);
         else
             AES_set_encrypt_key(key, key_len, &cdata->ks);
-#   ifndef AES_ASM
+#  ifndef AES_ASM
         /*
          * OpenSSL C functions use byte-swapped extended key.
          */
         padlock_key_bswap(&cdata->ks);
-#   endif
+#  endif
         cdata->cword.b.keygen = 1;
         break;
 
@@ -714,12 +713,10 @@ static RAND_METHOD padlock_rand = {
     padlock_rand_status,        /* rand status */
 };
 
-#  endif                        /* COMPILE_HW_PADLOCK */
-# endif                         /* !OPENSSL_NO_HW_PADLOCK */
-#endif                          /* !OPENSSL_NO_HW */
+# endif                        /* COMPILE_PADLOCKENG */
+#endif                         /* !OPENSSL_NO_PADLOCKENG */
 
-#if defined(OPENSSL_NO_HW) || defined(OPENSSL_NO_HW_PADLOCK) \
-        || !defined(COMPILE_HW_PADLOCK)
+#if defined(OPENSSL_NO_PADLOCKENG) || !defined(COMPILE_PADLOCKENG)
 # ifndef OPENSSL_NO_DYNAMIC_ENGINE
 OPENSSL_EXPORT
     int bind_engine(ENGINE *e, const char *id, const dynamic_fns *fns);