[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Problem with encrypted assertions



I've identified and addressed my specific issue with encryption and a
Shibboleth IdP.  The XML Encryption standard isn't guaranteed compliant with
RFC1423, but OpenSSL is expecting it to be compliant.

In this case, Shibboleth 2 (Java project utilizing OpenSAML libraries)
generates the expected final padding byte, but all other padding bytes are
unpredictable as allowed by XML Enc (
http://www.w3.org/TR/2002/REC-xmlenc-core-20021210).  So this is a false
positive error reported by OpenSSL.

A Google search revealed that Sampo also identified this back in 2005:
http://rt.openssl.org/Ticket/Display.html?user=guest&pass=guest&id=1067 .
 The recommended solution is for the caller to implement custom XML Enc
padding check.

So, below is the patch I'm using.  Essentially, it disables OpenSSL padding
check before final validation, and then validates the last padding byte and
trims the rest.  It's a simplified function from OpenSSL 0.9.8o.  This
should be fine for regular use with OpenSSL 0.9.7 and later, although the
function could benefit from more detailed exception reporting.

Similarly, the xmlsec1 utility performs its own custom padding validation,
which is why it didn't trigger decryption errors on Shibboleth2 assertions.

Regards,
Eric


diff -ru zxid-0.63/zxcrypto.c zxid-0.63.oa/zxcrypto.c
--- zxid-0.63/zxcrypto.c Tue Sep  7 23:04:19 2010
+++ zxid-0.63.new/zxcrypto.c Thu Sep 30 14:32:14 2010
@@ -115,9 +115,18 @@

   ASSERTOP(outlen + iv_len, <=, alloclen);

-  if(!EVP_CipherFinal_ex(&ctx, out->s + iv_len + outlen, &tmplen)) {  /*
Append final block */
-    where = "EVP_CipherFinal_ex()";
-    goto sslerr;
+  if (encflag) {
+    if(!EVP_CipherFinal_ex(&ctx, out->s + iv_len + outlen, &tmplen)) {  /*
Append final block */
+      where = "EVP_CipherFinal_ex()";
+      goto sslerr;
+    }
+  } else {
+    /* Perform our own padding check, as XML Enc is not guaranteed
compatible with OpenSSL & RFC 1423. */
+    EVP_CIPHER_CTX_set_padding(&ctx, 0);
+    if(!zx_EVP_DecryptFinal_ex(&ctx, out->s + iv_len + outlen, &tmplen)) {
 /* Append final block */
+      where = "zx_EVP_DecryptFinal_ex()";
+      goto sslerr;
+    }
   }
   EVP_CIPHER_CTX_cleanup(&ctx);

@@ -134,6 +143,53 @@
   return 0;
 }

+/*() zx_EVP_DecryptFinal_ex() is a drop-in replacement for OpenSSL
EVP_DecryptFinal_ex.
+ *  It performs XML Enc compatible padding check */
+
+int zx_EVP_DecryptFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int
*outl) {
+  int i,n;
+  unsigned int b;
+
+  *outl=0;
+  b=ctx->cipher->block_size;
+  if (b > 1)
+    {
+    if (ctx->buf_len || !ctx->final_used)
+      {
+      //EVPerr(EVP_F_EVP_DECRYPTFINAL_EX,EVP_R_WRONG_FINAL_BLOCK_LENGTH);
+      return(0);
+      }
+    ASSERTOP(b, <=, sizeof ctx->final);
+    n=ctx->final[b-1];
+    if (n == 0 || n > (int)b)
+      {
+      //EVPerr(EVP_F_EVP_DECRYPTFINAL_EX,EVP_R_BAD_DECRYPT);
+      return(0);
+      }
+
+    /* The padding used in XML Enc does not follow RFC 1423
+    * and is not supported by OpenSSL. The last padding byte
+    * is checked, but all other padding bytes are ignored
+    * and trimmed.
+    *
+    * [XMLENC] D. Eastlake, ed., XML Encryption Syntax and
+    * Processing, W3C Recommendation 10. Dec. 2002,
+    * http://www.w3.org/TR/2002/REC-xmlenc-core-20021210 */
+    if (ctx->final[b-1] != n)
+      {
+      //EVPerr(EVP_F_EVP_DECRYPTFINAL_EX,EVP_R_BAD_DECRYPT);
+      return(0);
+      }
+    n=ctx->cipher->block_size-n;
+    for (i=0; i<n; i++)
+      out[i]=ctx->final[i];
+    *outl=n;
+    }
+  else
+    *outl=0;
+  return(1);
+}
+
 /*() RSA public key encryption. See zx_get_rsa_pub_from_cert() for
  * a way to obtain public key data structure.
  * N.B. This function +only+ does the public key part. It does not


On Fri, Oct 1, 2010 at 1:23 PM, <kinzler@xxxxxxxxx> wrote:

> Christian Borup:
> >> I'm not having any luck authenticating via a IdP that returns encrypted
> >> assertions.
> >>
> >> Calling Net::SAML::simple_cf with the querry string the following is
> >> printed to stderr and exit seems to be called.
> >>
> >> t    zxsig.c:318 zx_report_openssl_error        zx E
> EVP_CipherFinal_ex():
> >> OpenSSL error(101077092) error:06065064:digital envelope
> >> routines:EVP_DecryptFinal:bad decrypt (evp_enc.c:445): ? 0
> >>
> >> Clues anyone?
>
> Eric Rybski:
> > I was able to decrypt the message with xmlsec1, built with the same
> > openssl versions and compiler.  So this leads me to believe that there
> may
> > be an issue inside zxid, with data being passed to
> > openssl EVP_CipherFinal_ex().
> > Did you ever identify a solution for this?
> |
> | I suspect the issue is probably a simple one (perhaps zxid is
> | mis-identifying the encryption algo, or loading an incomplete encrypted
> | string from the XML?).  Nothing obvious issues jumped out at me yet when
> | I was stepping through with gdb.
>
> I found the same OpenSSL error(101077092) with a SiteMinder-run IdP and
> my ZXID 0.64 SP.
>
> We turned off assertion encryption on both our sides to debug and
> found the SAMLResponse's failing on a digest mismatch in the signature
> validation of the (unencrypted) assertion.  We tracked down the cause of
> the mismatch to a whitespace problem.  The digest value was calculated
> over the assertion having newline-terminated lines, while the SAMLResponse
> was coming in with lines terminated with carriage returns and newlines.
> Apparently, ZXID isn't canonicalizing the line termination characters to
> newlines-only (XML standard), leading to the mismatch.  Other software,
> including xmlsec1, it seems, do canonicalize the CR-NL's to NL.
> Manually converting it to NL's and then feeding it to ZXID works fine.
>
> I don't know if this accounts for others' OpenSSL error(101077092)'s
> but it seems possible.
>
> Regards,                                        Steve Kinzler
>
> --
> from the brain of Steve Kinzler    /o)\    kinzler@xxxxxxxxxxxxxx
> an organ with a mind of its own    \(o/    www.cs.indiana.edu/~kinzler
> Bargeboard pecksniffian / achlorhydria proleg nascence / surra hysteroid.