[xmlsec] xmlsec-mscrypto code review

Aleksey Sanin aleksey at aleksey.com
Sat Sep 20 00:57:23 PDT 2003


I finally got to a Windows box and started xmlsec-mscrypto code review.
The first pass includes three files:
    src/mscrypto/app.c
    src/mscrypto/bignum.c
    src/mscrypto/certkeys.c
and it is checked in the CVS. I also have attached the resulting patch  
to this
email in case you wonder what I have changed :) This patch mostly touches
mscrypto specific code and probably absolutely not interesting for someone
who is not using xmlsec-mscrypto. I am getting the same errors from xmlsec
test suites (hmac, some x509 and some aes tests) thus I feel that I did 
not regress :)

One more time I would like to say that I really appreciate all the work 
Wouter is
doing. After looking at the code I have to say that it's really 
impressive how
hard is it to do something right with mscrypto api.

Aleksey


My comments on the xmlsec-mscrypto code and my patch:
----------------------------------------------------------------------------------------------------------------------------
0) It's my first close expirience with MS Crypto API  and I could not 
resist to
express my opinion about it. While MS Crypto API is OK for writing
applications (i.e. same as OpenSSL, NSS, BSAFE, etc.) but it's *absolutely*
not OK for writing secure applications. I was just suprised to see how 
*difficult* is
to ensure that everything is OK (also see item 1) bellow). I knew it's a 
badly
designed API but I never thought that it is soo bad :(

1) You should trust nobody! Anyone can fool you: user or another application
might provide you incorrect data; call to xmlsec or system function 
might fail with
an error code; worse, the same call might fail but the return code is 
"success" and
so on. The patch fixes a lot of places where the original code failed to 
check
input data or function return values. One of my favorite examples is 
that original
code *silently* assumed that base64 decoded value of a RSA public exponent
obtained from XML fits in a DWORD. And did memcpy to copy from 
xmlSecBuffer to
a DWORD. The trivial DoS attack (at least DoS!!!) is to put very long 
base64 string
in XML file and enjoy the server crash :)
One of the strongest sides of xmlsec library is that there are very few 
known ways to
crash it (and all of them are related to running the application in an 
environment with
a very limited memory to force a malloc failure). I would like to keep 
it in xmlsec-mscrypto too.
Be a little paranoid is good in this context :)

2) Memory leaks. I fixed a lot of them (especially in error cases). 
Wouter, I wonder
if you can run some memory checking tool (purify?) on the 
xmlsec-mscrypto code.
I feel that there are too many places to spot everything on code review.

3) Formatting. Just for clarification, the formating is:

tab size=8;indentation=4;insert spaces=yes

Does anybody know how to specify it in the source file so VI, Emacs, 
etc. would get
it automaticaly?

4) malloc/free vs. xmlMalloc/xmlFree
xmlsec library use libxml2 memory management functions. This provides an 
easy way to
replace default memory management functions with custom ones. And this 
might be very
usefull in some cases.

5) xmlSecBufferReadFile
I have introduced a new xmlSecBufferReadFile() function that reads 
binary file content
into a buffer. There were 3 places with the same code to do this thus it 
was probably a right time :)

6) XMLSEC_ERRORS_R_XMLSEC_FAILED vs. XMLSEC_ERRORS_R_CRYPTO_FAILED
The correct usage rule is:
    if the failed function starts with "xmlSec" then use
          XMLSEC_ERRORS_R_XMLSEC_FAILED
    else if it is xmlMalloc/xmlFree/xmlStrdup/etc then use
          XMLSEC_ERRORS_R_MALLOC_FAILED
    else if the function starts with "xml" or "xslt" (i.e. i comes from 
libxml or libxslt) then use
          XMLSEC_ERRORS_R_XML_FAILED
    else if it is related to IO (fopen, fread, fwrite, etc.) then use
          XMLSEC_ERRORS_R_IO_FAILED
    else if the function could be used only from xmlsec-crypto (i.e. it 
is crypto engine related) then use
          XMLSEC_ERRORS_R_IO_FAILED
    else
          it is something new and should be discussed
    fi

Correct error reason is very important. For example,  some applications 
ignore all the
XMLSEC_ERRORS_R_XMLSEC_FAILED errors to get to the bottom of the errors 
stack and report
the actual problem.

7) "size=%d;error=%d" instead of "size %d, error: %d":
It would be great if xmlsec-mscrypto can follow the error message 
standard adopted in the other files
of xmlsec library: <name1>=<value1>;<name2>=<value2>;...
This greatly helps when one needs to write a logs parser. For example, 
to find the reason of memory
allocation failures.

8) xmlSecMSCryptoNodeGetBigNumValue(), 
xmlSecMSCryptoNodeSetBigNumValue() and reading/writing
RSA/DSA keys
I rewrote these functions to avoid un-necessary xmlSecBuffer allocations 
and simplify the code. I feel that
even after that there is a room for improvement but it was enough for a 
first pass. I fixed a lot of problems
in this code including crashers, memory leaks, etc. (see item 0) ). It's 
just one of examples of how bad is
mscrypto api.

9) xmlSecMSCryptoKeyDataDuplicate() : there was a comment that it's not 
clear what exactly should be done.
I made a change and would appreciate if someone (Wouter?) would look at it.

10) Before (and after) the change I am getting an error "The oridinal 
1810 could not be located in the dynamic
link library LIBEAY32.dll" in a message dialog when executing one of 
x509 tests. I guess it's something
with my environment but if anyone can save me time for investigation I 
would appreciate this.










-------------- next part --------------
Index: include/xmlsec/buffer.h
===================================================================
RCS file: /cvs/gnome/xmlsec/include/xmlsec/buffer.h,v
retrieving revision 1.10.2.1
diff -u -r1.10.2.1 buffer.h
--- include/xmlsec/buffer.h	13 Sep 2003 04:46:20 -0000	1.10.2.1
+++ include/xmlsec/buffer.h	20 Sep 2003 07:04:52 -0000
@@ -87,6 +87,10 @@
 								 xmlSecSize size);
 XMLSEC_EXPORT int		xmlSecBufferRemoveTail		(xmlSecBufferPtr buf,
 								 xmlSecSize size);
+
+XMLSEC_EXPORT int		xmlSecBufferReadFile		(xmlSecBufferPtr buf,
+								 const char* filename);
+
 XMLSEC_EXPORT int		xmlSecBufferBase64NodeContentRead(xmlSecBufferPtr buf,
 								 xmlNodePtr node);
 XMLSEC_EXPORT int		xmlSecBufferBase64NodeContentWrite(xmlSecBufferPtr buf,
Index: include/xmlsec/mscrypto/app.h
===================================================================
RCS file: /cvs/gnome/xmlsec/include/xmlsec/mscrypto/Attic/app.h,v
retrieving revision 1.1.2.2
diff -u -r1.1.2.2 app.h
--- include/xmlsec/mscrypto/app.h	9 Sep 2003 19:41:45 -0000	1.1.2.2
+++ include/xmlsec/mscrypto/app.h	20 Sep 2003 07:04:52 -0000
@@ -21,18 +21,18 @@
 /**
  * Init/shutdown
  */
-XMLSEC_CRYPTO_EXPORT int	xmlSecMSCryptoAppInit			(const char* config);
-XMLSEC_CRYPTO_EXPORT int	xmlSecMSCryptoAppShutdown		(void);
+XMLSEC_CRYPTO_EXPORT int	xmlSecMSCryptoAppInit				(const char* config);
+XMLSEC_CRYPTO_EXPORT int	xmlSecMSCryptoAppShutdown			(void);
 
 /** 
  * Keys Manager
  */
-XMLSEC_CRYPTO_EXPORT int	xmlSecMSCryptoAppDefaultKeysMngrInit	(xmlSecKeysMngrPtr mngr);
-XMLSEC_CRYPTO_EXPORT int 	xmlSecMSCryptoAppDefaultKeysMngrAdoptKey	(xmlSecKeysMngrPtr mngr,
-									 xmlSecKeyPtr key);
-XMLSEC_CRYPTO_EXPORT int 	xmlSecMSCryptoAppDefaultKeysMngrLoad	(xmlSecKeysMngrPtr mngr,
+XMLSEC_CRYPTO_EXPORT int	xmlSecMSCryptoAppDefaultKeysMngrInit(xmlSecKeysMngrPtr mngr);
+XMLSEC_CRYPTO_EXPORT int 	xmlSecMSCryptoAppDefaultKeysMngrAdoptKey(xmlSecKeysMngrPtr mngr,
+																xmlSecKeyPtr key);
+XMLSEC_CRYPTO_EXPORT int 	xmlSecMSCryptoAppDefaultKeysMngrLoad(xmlSecKeysMngrPtr mngr,
 									 const char* uri);
-XMLSEC_CRYPTO_EXPORT int 	xmlSecMSCryptoAppDefaultKeysMngrSave	(xmlSecKeysMngrPtr mngr,
+XMLSEC_CRYPTO_EXPORT int 	xmlSecMSCryptoAppDefaultKeysMngrSave(xmlSecKeysMngrPtr mngr,
 									 const char* filename,
 									 xmlSecKeyDataType type);
 #ifndef XMLSEC_NO_X509
Index: include/xmlsec/mscrypto/bignum.h
===================================================================
RCS file: /cvs/gnome/xmlsec/include/xmlsec/mscrypto/Attic/bignum.h,v
retrieving revision 1.1.2.1
diff -u -r1.1.2.1 bignum.h
--- include/xmlsec/mscrypto/bignum.h	12 Sep 2003 01:00:54 -0000	1.1.2.1
+++ include/xmlsec/mscrypto/bignum.h	20 Sep 2003 07:04:52 -0000
@@ -19,8 +19,8 @@
 
 #include <xmlsec/xmlsec.h>
 
-XMLSEC_CRYPTO_EXPORT int	xmlSecMSCryptoNodeGetBigNumValue(const xmlNodePtr cur, xmlSecBufferPtr retval);
-XMLSEC_CRYPTO_EXPORT int 	xmlSecMSCryptoNodeSetBigNumValue(xmlNodePtr cur, const xmlSecBufferPtr a, int addLineBreaks);
+XMLSEC_CRYPTO_EXPORT int	xmlSecMSCryptoNodeGetBigNumValue(xmlNodePtr cur, xmlSecBufferPtr retval);
+XMLSEC_CRYPTO_EXPORT int 	xmlSecMSCryptoNodeSetBigNumValue(xmlNodePtr cur, xmlSecByte* buf, xmlSecSize bufLen, int addLineBreaks);
 
 #ifdef __cplusplus
 }
Index: src/buffer.c
===================================================================
RCS file: /cvs/gnome/xmlsec/src/buffer.c,v
retrieving revision 1.10.2.1
diff -u -r1.10.2.1 buffer.c
--- src/buffer.c	13 Sep 2003 04:46:21 -0000	1.10.2.1
+++ src/buffer.c	20 Sep 2003 07:04:52 -0000
@@ -449,6 +449,69 @@
 }
 
 /**
+ * xmlSecBufferReadFile:
+ * @buf:		the pointer to buffer object.
+ * @filename:		the filename.
+ *
+ * Reads the content of the file @filename in the buffer.
+ *
+ * Returns 0 on success or a negative value if an error occurs.
+ */
+int 
+xmlSecBufferReadFile(xmlSecBufferPtr buf, const char* filename) {
+    xmlSecByte buffer[1024];
+    FILE* f;
+    int ret, len;
+
+    xmlSecAssert2(buf != NULL, -1);
+    xmlSecAssert2(filename != NULL, -1);
+
+    f = fopen(filename, "rb");
+    if(f == NULL) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    NULL,
+		    "fopen",
+		    XMLSEC_ERRORS_R_IO_FAILED,
+		    "filename=%s;errno=%d", 
+		    xmlSecErrorsSafeString(filename),
+		    errno);
+	return(-1);
+    }
+
+    while(1) {
+        len = fread(buffer, 1, sizeof(buffer), f);
+	if(len == 0) {
+            break;
+        }else if(len < 0) {
+            xmlSecError(XMLSEC_ERRORS_HERE,
+                        NULL,
+                        "fread",
+                        XMLSEC_ERRORS_R_IO_FAILED,
+                        "filename=%s;errno=%d", 
+                        xmlSecErrorsSafeString(filename),
+			errno);
+            fclose(f);
+            return(-1);
+        }
+
+	ret = xmlSecBufferAppend(buf, buffer, len);
+	if(ret < 0) {
+            xmlSecError(XMLSEC_ERRORS_HERE,
+                        NULL,
+                        "xmlSecBufferAppend",
+                        XMLSEC_ERRORS_R_XMLSEC_FAILED,
+                        "size=%d", 
+                        len);
+            fclose(f);
+            return(-1);
+        }     
+    }
+
+    fclose(f);
+    return(0);
+}
+
+/**
  * xmlSecBufferBase64NodeContentRead:
  * @buf:		the pointer to buffer object.
  * @node:		the pointer to node.
Index: src/keys.c
===================================================================
RCS file: /cvs/gnome/xmlsec/src/keys.c,v
retrieving revision 1.53
diff -u -r1.53 keys.c
--- src/keys.c	8 Aug 2003 16:15:39 -0000	1.53
+++ src/keys.c	20 Sep 2003 07:04:53 -0000
@@ -833,8 +833,6 @@
 xmlSecKeyReadBinaryFile(xmlSecKeyDataId dataId, const char* filename) {
     xmlSecKeyPtr key;
     xmlSecBuffer buffer;
-    xmlSecByte buf[1024];
-    FILE *f;
     int ret;
     
     xmlSecAssert2(dataId != xmlSecKeyDataIdUnknown, NULL);
@@ -851,37 +849,17 @@
 	return(NULL);	
     }
 
-    f = fopen(filename, "rb");
-    if(f == NULL) {
+    ret = xmlSecBufferReadFile(&buffer, filename);
+    if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(dataId)),
-		    "fopen",
-		    XMLSEC_ERRORS_R_IO_FAILED,
+		    "xmlSecBufferReadFile",
+		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "filename=%s", 
 		    xmlSecErrorsSafeString(filename));
 	xmlSecBufferFinalize(&buffer);
 	return(NULL);
     }
-
-    while(1) {
-        ret = fread(buf, 1, sizeof(buf), f);
-	if(ret > 0) {
-	    xmlSecBufferAppend(&buffer, buf, ret);
-	} else if(ret == 0) {
-	    break;
-	} else {
-	    xmlSecError(XMLSEC_ERRORS_HERE,
-			xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(dataId)),
-			"fread",
-			XMLSEC_ERRORS_R_IO_FAILED,
-			"filename=%s", 
-			xmlSecErrorsSafeString(filename));
-	    fclose(f);
-	    xmlSecBufferFinalize(&buffer);
-	    return(NULL);
-	}
-    }
-    fclose(f);    
 
     key = xmlSecKeyReadBuffer(dataId, &buffer);
     if(key == NULL) {
Index: src/mscrypto/app.c
===================================================================
RCS file: /cvs/gnome/xmlsec/src/mscrypto/Attic/app.c,v
retrieving revision 1.1.2.4
diff -u -r1.1.2.4 app.c
--- src/mscrypto/app.c	11 Sep 2003 22:13:10 -0000	1.1.2.4
+++ src/mscrypto/app.c	20 Sep 2003 07:04:53 -0000
@@ -69,17 +69,14 @@
  */
 xmlSecKeyPtr
 xmlSecMSCryptoAppKeyLoad(const char *filename, xmlSecKeyDataFormat format,
-			const char *pwd, 
-			void* pwdCallback, 
-			void* pwdCallbackCtx) {
+			 const char *pwd, void* pwdCallback, void* pwdCallbackCtx) {
     
     
     xmlSecAssert2(filename != NULL, NULL);
     xmlSecAssert2(format != xmlSecKeyDataFormatUnknown, NULL);
     
-    if (format == xmlSecKeyDataFormatPkcs12) {
-	return (xmlSecMSCryptoAppPkcs12Load(filename, pwd, pwdCallback,
-					    pwdCallbackCtx));
+    if(format == xmlSecKeyDataFormatPkcs12) {
+	return(xmlSecMSCryptoAppPkcs12Load(filename, pwd, pwdCallback, pwdCallbackCtx));
     }
 
     /* Any other format like PEM keys is currently not supported */
@@ -107,7 +104,7 @@
  */
 int		
 xmlSecMSCryptoAppKeyCertLoad(xmlSecKeyPtr key, const char* filename, 
-			  xmlSecKeyDataFormat format) {
+			     xmlSecKeyDataFormat format) {
     xmlSecKeyDataPtr data;
     xmlSecKeyDataFormat certFormat;
     PCCERT_CONTEXT pCert;
@@ -145,7 +142,7 @@
     }
 
     pCert = xmlSecMSCryptoAppCertLoad(filename, certFormat);
-    if (pCert == NULL) {
+    if(pCert == NULL) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
 		    "xmlSecMSCryptoAppCertLoad", 
@@ -185,22 +182,19 @@
  */
 xmlSecKeyPtr	
 xmlSecMSCryptoAppPkcs12Load(const char *filename, 
-			  const char *pwd,
-		          void* pwdCallback ATTRIBUTE_UNUSED, 
-			  void* pwdCallbackCtx ATTRIBUTE_UNUSED) {
-    FILE *f = NULL;
+			    const char *pwd,
+			    void* pwdCallback ATTRIBUTE_UNUSED, 
+			    void* pwdCallbackCtx ATTRIBUTE_UNUSED) {
     xmlSecBuffer buffer;
-    xmlSecByte buf[1024];
     int ret, len;
     CRYPT_DATA_BLOB pfx;
-    HCERTSTORE hCertStore;
-    PCCERT_CONTEXT tmpcert;
+    HCERTSTORE hCertStore = NULL;
+    PCCERT_CONTEXT tmpcert = NULL;
     PCCERT_CONTEXT pCert = NULL;
-    WCHAR * wcPwd;
+    WCHAR * wcPwd = NULL;
     xmlSecKeyDataPtr x509Data = NULL;
     xmlSecKeyDataPtr data = NULL;
     xmlSecKeyPtr key = NULL;
-    DWORD dwData, dwDataLen;
     BOOL bres;
 
     xmlSecAssert2(filename != NULL, NULL);
@@ -216,90 +210,77 @@
 	return(NULL);	
     }
 
-    f = fopen(filename, "rb");
-    if (f == NULL) {
+    ret = xmlSecBufferReadFile(&buffer, filename);
+    if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
-		    "fopen",
-		    XMLSEC_ERRORS_R_IO_FAILED,
+		    "xmlSecBufferReadFile",
+		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "filename=%s", 
 		    xmlSecErrorsSafeString(filename));
-	xmlSecBufferFinalize(&buffer);
-	return(NULL);
-    }
-
-    while(1) {
-        ret = fread(buf, 1, sizeof(buf), f);
-        if (ret > 0) {
-            xmlSecBufferAppend(&buffer, buf, ret);
-        } else if(ret == 0) {
-            break;
-        } else {
-            xmlSecError(XMLSEC_ERRORS_HERE,
-                        NULL,
-                        "fread",
-                        XMLSEC_ERRORS_R_IO_FAILED,
-                        "filename=%s", 
-                        xmlSecErrorsSafeString(filename));
-            fclose(f);
-            xmlSecBufferFinalize(&buffer);
-            return(NULL);
-        }
+	goto done;
     }
-    fclose(f);
 
+    memset(&pfx, 0, sizeof(pfx));
     pfx.pbData = xmlSecBufferGetData(&buffer);
     pfx.cbData = xmlSecBufferGetSize(&buffer);
-
-    if (FALSE == PFXIsPFXBlob(&pfx)) {
+    if(FALSE == PFXIsPFXBlob(&pfx)) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
 		    "PFXIsPFXBlob",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    "Invalid data");
-
-	return(NULL);
+		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+		    "size=%d",
+		    pfx.cbData);
+	goto done;
     }
 
     len = MultiByteToWideChar(CP_ACP, MB_PRECOMPOSED, pwd, -1, NULL, 0);
-    if (len <= 0) {
+    if(len <= 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
 		    "MultiByteToWideChar",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    "Error no: %d", GetLastError());
-	return(NULL);
+		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+		    "GetLastError=%d", GetLastError());
+	goto done;
     }
-    wcPwd = (WCHAR *)malloc((len + 1) * sizeof(WCHAR));
+
+    wcPwd = (WCHAR *)xmlMalloc((len + 1) * sizeof(WCHAR));
+    if(wcPwd == NULL) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    NULL,
+		    NULL,
+		    XMLSEC_ERRORS_R_MALLOC_FAILED,
+		    "len=%d", len);
+	goto done;
+    }
+
     ret = MultiByteToWideChar(CP_ACP, MB_PRECOMPOSED, pwd, -1, wcPwd, len);
     if (ret <= 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
 		    "MultiByteToWideChar",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    "Error no: %d", GetLastError());
-	return(NULL);
+		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+		    "GetLastError=%d", GetLastError());
+	goto done;
     }
 
     if (FALSE == PFXVerifyPassword(&pfx, wcPwd, 0)) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
 		    "PFXVerifyPassword",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    "Invalid password");
-	free(wcPwd);
-	return(NULL);
+		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+		    XMLSEC_ERRORS_NO_MESSAGE);
+	goto done;
     }
 
     hCertStore = PFXImportCertStore(&pfx, wcPwd, CRYPT_EXPORTABLE);
-    free(wcPwd);
     if (NULL == hCertStore) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
 		    "PFXImportCertStore",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    "Invalid data, Error nr: %d", GetLastError());
-	return(NULL);
+		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+		    "GetLastError=%d", GetLastError());
+	goto done;
     }
     
     x509Data = xmlSecKeyDataCreate(xmlSecMSCryptoKeyDataX509Id);
@@ -315,11 +296,10 @@
 
     while (pCert = CertEnumCertificatesInStore(hCertStore, pCert)) {
 	/* Find the certificate that has the private key */
-	dwDataLen = sizeof(DWORD);
-	dwData = 0;
-	if (TRUE == CertGetCertificateContextProperty(pCert, CERT_KEY_SPEC_PROP_ID, &dwData, &dwDataLen) &&
-	    dwData) {
-	/*if ((TRUE == CryptFindCertificateKeyProvInfo(pCert, 0, NULL)) && (NULL == data)) {*/
+	DWORD dwData = 0;
+        DWORD dwDataLen = sizeof(DWORD);
+
+	if((TRUE == CertGetCertificateContextProperty(pCert, CERT_KEY_SPEC_PROP_ID, &dwData, &dwDataLen)) && (dwData > 0)) {
 	    data = xmlSecMSCryptoCertAdopt(pCert, xmlSecKeyDataTypePrivate | xmlSecKeyDataTypePublic);
 	    if(data == NULL) {
 		xmlSecError(XMLSEC_ERRORS_HERE,
@@ -329,14 +309,16 @@
 			    XMLSEC_ERRORS_NO_MESSAGE);
 		goto done;
 	    }
+	
 	    tmpcert = CertDuplicateCertificateContext(pCert);
 	    if(tmpcert == NULL) {
 		xmlSecError(XMLSEC_ERRORS_HERE,
 			    NULL,
 			    "CertDuplicateCertificateContext",
 			    XMLSEC_ERRORS_R_CRYPTO_FAILED,
-			    "data=%s, error code=%d",
-			    xmlSecErrorsSafeString(xmlSecKeyDataGetName(x509Data)), GetLastError());
+			    "data=%s;GetLastError=%d",
+			    xmlSecErrorsSafeString(xmlSecKeyDataGetName(x509Data)), 
+			    GetLastError());
 		goto done;
 	    }
 
@@ -351,7 +333,6 @@
 		CertFreeCertificateContext(tmpcert);
 		goto done;
 	    }
-
 	}
 
 	tmpcert = CertDuplicateCertificateContext(pCert);
@@ -360,10 +341,12 @@
 			NULL,
 			"CertDuplicateCertificateContext",
 			XMLSEC_ERRORS_R_CRYPTO_FAILED,
-			"data=%s, error code=%d",
-			xmlSecErrorsSafeString(xmlSecKeyDataGetName(x509Data)), GetLastError());
+			"data=%s;GetLastError=%d",
+			xmlSecErrorsSafeString(xmlSecKeyDataGetName(x509Data)), 
+			GetLastError());
 	    goto done;	
 	}
+
 	ret = xmlSecMSCryptoKeyDataX509AdoptCert(x509Data, tmpcert);
 	if(ret < 0) {
 	    xmlSecError(XMLSEC_ERRORS_HERE,
@@ -378,9 +361,9 @@
     }
 
     if (data == NULL) {
-    xmlSecError(XMLSEC_ERRORS_HERE,
-		NULL,
-		"xmlSecMSCryptoAppPkcs12Load",
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    NULL,
+		    "xmlSecMSCryptoAppPkcs12Load",
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "private key not found in PKCS12 file");
 	goto done;
@@ -392,7 +375,7 @@
 		    NULL,
 		    "xmlSecKeyCreate",
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		XMLSEC_ERRORS_NO_MESSAGE);
+		    XMLSEC_ERRORS_NO_MESSAGE);
 	goto done;
     }    
     
@@ -424,16 +407,20 @@
     }
     x509Data = NULL;
 
-
 done:
-    CertCloseStore(hCertStore, 0);
-
+    if(hCertStore != NULL) {
+        CertCloseStore(hCertStore, 0);
+    }
+    if(wcPwd != NULL) {
+        xmlFree(wcPwd);
+    }
     if(x509Data != NULL) {
 	xmlSecKeyDataDestroy(x509Data);
     }
     if(data != NULL) {
         xmlSecKeyDataDestroy(data);
     }
+    xmlSecBufferFinalize(&buffer);
 
     return(key); 
 }
@@ -456,7 +443,7 @@
 				xmlSecKeyDataFormat format, 
 				xmlSecKeyDataType type ATTRIBUTE_UNUSED) {
     xmlSecKeyDataStorePtr x509Store;
-    PCCERT_CONTEXT pCert;
+    PCCERT_CONTEXT pCert = NULL;
     int ret;
 
     xmlSecAssert2(mngr != NULL, -1);
@@ -465,9 +452,9 @@
 
     x509Store = xmlSecKeysMngrGetDataStore(mngr, xmlSecMSCryptoX509StoreId);
     if(x509Store == NULL) {
-    xmlSecError(XMLSEC_ERRORS_HERE,
-		NULL,
-                    "xmlSecKeysMngrGetDataStore",
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    NULL,
+		    "xmlSecKeysMngrGetDataStore",
                     XMLSEC_ERRORS_R_XMLSEC_FAILED,
                     "xmlSecMSCryptoX509StoreId");
         return(-1);
@@ -490,10 +477,10 @@
                     NULL,
                     "xmlSecMSCryptoX509StoreAdoptCert",
                     XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		XMLSEC_ERRORS_NO_MESSAGE);
+		    XMLSEC_ERRORS_NO_MESSAGE);
 	CertFreeCertificateContext(pCert);
-    return(-1);
-}
+        return(-1);
+    }
 
     return(0);
 }
@@ -501,9 +488,7 @@
 static PCCERT_CONTEXT	
 xmlSecMSCryptoAppCertLoad(const char* filename, xmlSecKeyDataFormat format) {
     PCCERT_CONTEXT pCert = NULL;
-    FILE *f = NULL;
     xmlSecBuffer buffer;
-    xmlSecByte buf[1024];
     int ret;
     
     xmlSecAssert2(filename != NULL, NULL);
@@ -519,61 +504,44 @@
 	return(NULL);	
     }
 
-    f = fopen(filename, "rb");
-    if (f == NULL) {
+    ret = xmlSecBufferReadFile(&buffer, filename);
+    if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
-		    "fopen",
-		    XMLSEC_ERRORS_R_IO_FAILED,
+		    "xmlSecBufferReadFile",
+		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "filename=%s", 
 		    xmlSecErrorsSafeString(filename));
 	xmlSecBufferFinalize(&buffer);
 	return(NULL);
     }
 
-    while(1) {
-        ret = fread(buf, 1, sizeof(buf), f);
-        if (ret > 0) {
-            xmlSecBufferAppend(&buffer, buf, ret);
-        } else if(ret == 0) {
-            break;
-        } else {
-            xmlSecError(XMLSEC_ERRORS_HERE,
-                        NULL,
-                        "fread",
-                        XMLSEC_ERRORS_R_IO_FAILED,
-                        "filename=%s", 
-                        xmlSecErrorsSafeString(filename));
-            fclose(f);
-            xmlSecBufferFinalize(&buffer);
-            return(NULL);
-        }
-    }
-    fclose(f);    
-
     switch (format) {
-    case xmlSecKeyDataFormatDer:
-	pCert = CertCreateCertificateContext(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
-			                     xmlSecBufferGetData(&buffer),
-			                     xmlSecBufferGetSize(&buffer));
-	xmlSecBufferFinalize(&buffer);
-	if (NULL == pCert) {
+	case xmlSecKeyDataFormatDer:
+	    pCert = CertCreateCertificateContext(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
+						 xmlSecBufferGetData(&buffer),
+						 xmlSecBufferGetSize(&buffer));
+	    if (NULL == pCert) {
+		xmlSecError(XMLSEC_ERRORS_HERE,
+			    NULL,
+			    "CertCreateCertificateContext",
+			    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+	    		    "error code=%d", GetLastError());
+		xmlSecBufferFinalize(&buffer);
+		return (NULL);
+	    }
+	    break;
+	default:
 	    xmlSecError(XMLSEC_ERRORS_HERE,
 			NULL,
-			"CertCreateCertificateContext",
-			XMLSEC_ERRORS_R_IO_FAILED,
-	    		"error code=%d", GetLastError());
-	    return (NULL);
-	}
-	break;
-    default:
-	xmlSecError(XMLSEC_ERRORS_HERE,
-		    NULL,
-		    NULL,
-		    XMLSEC_ERRORS_R_INVALID_FORMAT,
-		    "format=%d", format); 
+			NULL,
+			XMLSEC_ERRORS_R_INVALID_FORMAT,
+			"format=%d", format); 
+	    xmlSecBufferFinalize(&buffer);
+	    return(NULL);
     }
         	
+    xmlSecBufferFinalize(&buffer);
     return(pCert);
 }
 
Index: src/mscrypto/bignum.c
===================================================================
RCS file: /cvs/gnome/xmlsec/src/mscrypto/Attic/bignum.c,v
retrieving revision 1.1.2.1
diff -u -r1.1.2.1 bignum.c
--- src/mscrypto/bignum.c	11 Sep 2003 22:13:10 -0000	1.1.2.1
+++ src/mscrypto/bignum.c	20 Sep 2003 07:04:53 -0000
@@ -33,59 +33,48 @@
  * (http://www.w3.org/TR/xmldsig-core/#sec-CryptoBinary) 
  * to a BYTE array.
  *
- * Returns a pointer to SECItem produced from CryptoBinary string
- * or NULL if an error occurs.
+ * Returns 0 on success and a negative values if an error occurs.
  */
 int
-xmlSecMSCryptoNodeGetBigNumValue(const xmlNodePtr cur, xmlSecBufferPtr retval) {
-    xmlSecBuffer buf;
-    BYTE *j, *k;
+xmlSecMSCryptoNodeGetBigNumValue(xmlNodePtr cur, xmlSecBufferPtr buf) {
+    xmlSecSize size;
+    xmlSecByte* start;
+    xmlSecByte* end;
+    xmlSecByte tmp;
     int ret;
-    int len;
-    unsigned int i;
 
     xmlSecAssert2(cur != NULL, -1);
-
-    ret = xmlSecBufferInitialize(&buf, 1024);
-    if(ret < 0) {
-	xmlSecError(XMLSEC_ERRORS_HERE,
-		    NULL,
-		    "xmlSecBufferInitialize",
-	    	    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    XMLSEC_ERRORS_NO_MESSAGE);
-	return(-1);
-    }
+    xmlSecAssert2(buf != NULL, -1);
+    xmlSecAssert2(xmlSecBufferGetSize(buf) == 0, -1);
     
-    ret = xmlSecBufferBase64NodeContentRead(&buf, cur);
+    ret = xmlSecBufferBase64NodeContentRead(buf, cur);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
 		    "xmlSecBufferBase64NodeContentRead",
 	    	    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    XMLSEC_ERRORS_NO_MESSAGE);
-	xmlSecBufferFinalize(&buf);
 	return(-1);
     }
     
-    len = xmlSecBufferGetSize(&buf);
-    ret = xmlSecBufferInitialize(retval, len);
-    if(ret < 0) {
-	xmlSecError(XMLSEC_ERRORS_HERE,
-		    NULL,
-		    "xmlSecBufferSetSize",
-	    	    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    XMLSEC_ERRORS_NO_MESSAGE);
-	xmlSecBufferFinalize(&buf);
-	return(-1);
+    /* invert the buffer */
+    start = xmlSecBufferGetData(buf);
+    size = xmlSecBufferGetSize(buf);
+    if((start == NULL) || (size < 1)) {
+	/* we are done */
+	return(0);
+    }
+
+    end = start + size - 1;
+    while(start < end) {
+	tmp	 = (*start);
+	(*start) = (*end);
+	(*end)	 = tmp;
+	
+	++start;
+	--end;
     }
-    
-    j = xmlSecBufferGetData(&buf);
-    k = xmlSecBufferGetData(retval) + len - 1;
-    for (i = 0; i < len ; ++i) {
-	*k-- = *j++;
-    }
-    xmlSecBufferSetSize(retval, len);
-    xmlSecBufferFinalize(&buf);
+
     return(0);
 }
 
@@ -106,57 +95,60 @@
  * Returns 0 on success or -1 otherwise.
  */
 int
-xmlSecMSCryptoNodeSetBigNumValue(xmlNodePtr cur, const xmlSecBufferPtr a, int addLineBreaks) {
-    xmlSecBuffer buf;
-    BYTE *j, *k;
-    unsigned int alen, i;
+xmlSecMSCryptoNodeSetBigNumValue(xmlNodePtr cur, xmlSecByte* buf, xmlSecSize bufLen,  int addLineBreaks) {
+    xmlSecBuffer buffer;
+    xmlSecByte* src;
+    xmlSecByte* dst;
+    xmlSecSize i;
     int ret;
     
-    xmlSecAssert2(a != NULL, -1);
+    xmlSecAssert2(buf != NULL, -1);
+    xmlSecAssert2(bufLen > 0, -1);
     xmlSecAssert2(cur != NULL, -1);
 
-    alen = xmlSecBufferGetSize(a);
-    ret = xmlSecBufferInitialize(&buf, alen + 1);
+    ret = xmlSecBufferInitialize(&buffer, bufLen);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
 		    "xmlSecBufferInitialize",
 	    	    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    "size=%d", alen + 1);
+		    "size=%d", bufLen + 1);
 	return(-1);
     }    
 
-    j = xmlSecBufferGetData(a);
-    k = xmlSecBufferGetData(&buf) + alen - 1;
-
-    for (i = 0; i < alen ; ++i)
-	*k-- = *j++;
-    
-    ret = xmlSecBufferSetSize(&buf, alen);
+    ret = xmlSecBufferSetSize(&buffer, bufLen);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
 		    "xmlSecBufferSetSize",
 	    	    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    "size=%d", alen);
-	xmlSecBufferFinalize(&buf);
+		    "size=%d", bufLen);
 	return(-1);
-    }
+    }    
+
+    
+    src = buf + bufLen - 1;
+    dst = xmlSecBufferGetData(&buffer);
+    xmlSecAssert2(dst != NULL, -1);
 
+    for (i = 0; i < bufLen ; ++i) {
+	*(dst++) = *(src--);
+    }
+    
     if(addLineBreaks) {
 	xmlNodeSetContent(cur, xmlSecStringCR);
     } else {
 	xmlNodeSetContent(cur, xmlSecStringEmpty);
     }
 
-    ret = xmlSecBufferBase64NodeContentWrite(&buf, cur, XMLSEC_BASE64_LINESIZE);
+    ret = xmlSecBufferBase64NodeContentWrite(&buffer, cur, XMLSEC_BASE64_LINESIZE);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    NULL,
 		    "xmlSecBufferBase64NodeContentWrite",
 	    	    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    XMLSEC_ERRORS_NO_MESSAGE);
-	xmlSecBufferFinalize(&buf);
+	xmlSecBufferFinalize(&buffer);
 	return(-1);
     }
 
@@ -164,7 +156,6 @@
 	xmlNodeAddContent(cur, xmlSecStringCR);
     }
 
-    xmlSecBufferFinalize(&buf);
+    xmlSecBufferFinalize(&buffer);
     return(0);
 }
-
Index: src/mscrypto/certkeys.c
===================================================================
RCS file: /cvs/gnome/xmlsec/src/mscrypto/Attic/certkeys.c,v
retrieving revision 1.1.2.4
diff -u -r1.1.2.4 certkeys.c
--- src/mscrypto/certkeys.c	14 Sep 2003 21:36:39 -0000	1.1.2.4
+++ src/mscrypto/certkeys.c	20 Sep 2003 07:04:54 -0000
@@ -41,14 +41,14 @@
  * now is however directed to certificates.  Wouter
  */
 struct _xmlSecMSCryptoKeyDataCtx {
-    HCRYPTPROV hProv;
-    LPCTSTR providerName;
-    DWORD providerType;
-    BOOL fCallerFreeProv;
-    PCCERT_CONTEXT pCert;
-    DWORD dwKeySpec;
-    HCRYPTKEY hKey;
-    xmlSecKeyDataType type;
+    HCRYPTPROV		hProv;
+    BOOL		fCallerFreeProv;
+    LPCTSTR		providerName;
+    DWORD		providerType;
+    PCCERT_CONTEXT	pCert;
+    DWORD		dwKeySpec;
+    HCRYPTKEY		hKey;
+    xmlSecKeyDataType	type;
 };	    
 
 /******************************************************************************
@@ -81,7 +81,8 @@
     xmlSecAssert2(xmlSecKeyDataIsValid(data), -1);
     xmlSecAssert2(xmlSecKeyDataCheckSize(data, xmlSecMSCryptoKeyDataSize), -1);
     xmlSecAssert2(pCert != NULL, -1);
-    xmlSecAssert2(type & (xmlSecKeyDataTypePublic | xmlSecKeyDataTypePrivate), -1);
+    xmlSecAssert2(pCert->pCertInfo != NULL, -1);
+    xmlSecAssert2((type & (xmlSecKeyDataTypePublic | xmlSecKeyDataTypePrivate)) != 0, -1);
 
     ctx = xmlSecMSCryptoKeyDataGetCtx(data);
     xmlSecAssert2(ctx != NULL, -1);
@@ -93,21 +94,25 @@
 
     if(ctx->pCert != NULL) {
 	CertFreeCertificateContext(ctx->pCert);
+	ctx->pCert = NULL;
     }
 
     if ((ctx->hProv != 0) && (ctx->fCallerFreeProv)) {
 	CryptReleaseContext(ctx->hProv, 0);
 	ctx->hProv = 0;
+	ctx->fCallerFreeProv = FALSE;
+    } else {
+	ctx->hProv = 0;
+	ctx->fCallerFreeProv = FALSE;
     }
 
-    ctx->pCert = pCert;
     ctx->type = type;
 
     /* Now we acquire a context for this key(pair). The context is needed
      * for the real crypto stuff in MS Crypto.
      */
-    if (type & xmlSecKeyDataTypePrivate) {
-        if (!CryptAcquireCertificatePrivateKey(ctx->pCert, 
+    if((type & xmlSecKeyDataTypePrivate) != 0){
+        if (!CryptAcquireCertificatePrivateKey(pCert, 
 					       CRYPT_ACQUIRE_USE_PROV_INFO_FLAG, 
 					       NULL, 
 					       &(ctx->hProv), 
@@ -117,10 +122,10 @@
 			NULL,
 			"CryptAcquireCertificatePrivateKey",
 			XMLSEC_ERRORS_R_CRYPTO_FAILED,
-			"error code=%d", GetLastError());
+			"GetLastError=%d", GetLastError());
 	    return(-1);
 	}
-    } else if (type & xmlSecKeyDataTypePublic) {
+    } else if((type & xmlSecKeyDataTypePublic) != 0){
 	if (!CryptAcquireContext(&(ctx->hProv), 
 				 NULL, 
 				 ctx->providerName, 
@@ -130,7 +135,7 @@
 			NULL,
 			"CryptAcquireContext",
 			XMLSEC_ERRORS_R_CRYPTO_FAILED,
-			"error code=%d", GetLastError());
+			"GetLastError=%d", GetLastError());
 	    return(-1);
 	}
 	ctx->dwKeySpec = 0;
@@ -141,7 +146,7 @@
 		    NULL,
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "Unsupported keytype");
-	return(0);
+	return(-1);
     }
 
     /* CryptImportPublicKeyInfo is only needed when a real key handle
@@ -151,19 +156,18 @@
      * so no unnessecary calls to CryptImportPublicKeyInfo are being
      * made. WK
      */
-    if (!CryptImportPublicKeyInfo(ctx->hProv, 
+    if(!CryptImportPublicKeyInfo(ctx->hProv, 
 	X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, 
-	&(ctx->pCert->pCertInfo->SubjectPublicKeyInfo), 
+	&(pCert->pCertInfo->SubjectPublicKeyInfo), 
 	&(ctx->hKey))) {
 	    xmlSecError(XMLSEC_ERRORS_HERE,
 			NULL,
 			"CryptImportPublicKeyInfo",
 			XMLSEC_ERRORS_R_CRYPTO_FAILED,
-			"error code=%d", GetLastError());
-	    CryptReleaseContext(ctx->hProv, 0);
-	    ctx->hProv = 0;
+			"GetLastError=%d", GetLastError());
 	    return(-1);
     }
+    ctx->pCert = pCert;
 
     return(0);
 }
@@ -187,20 +191,28 @@
 
     if(ctx->hKey != 0) {
 	CryptDestroyKey(ctx->hKey);
+	ctx->hKey = 0;
     }
+
     if(ctx->pCert != NULL) {
 	CertFreeCertificateContext(ctx->pCert);
 	ctx->pCert = NULL;
     }
-    if (ctx->hProv != 0 && ctx->fCallerFreeProv) {
+    
+    if((ctx->hProv != 0) && ctx->fCallerFreeProv) {
 	CryptReleaseContext(ctx->hProv, 0);
+	ctx->hProv = 0;
+	ctx->fCallerFreeProv = FALSE;
+    } else {
+	ctx->hProv = 0;
+	ctx->fCallerFreeProv = FALSE;
     }
 
-    ctx->hProv = hProv;
+    ctx->hProv		 = hProv;
     ctx->fCallerFreeProv = fCallerFreeProv;
-    ctx->dwKeySpec = dwKeySpec;
-    ctx->hKey = hKey;
-    ctx->type = type;
+    ctx->dwKeySpec	 = dwKeySpec;
+    ctx->hKey		 = hKey;
+    ctx->type		 = type;
 
     return(0);
 }
@@ -279,22 +291,24 @@
 	    xmlSecError(XMLSEC_ERRORS_HERE,
 			xmlSecErrorsSafeString(xmlSecKeyDataGetName(dst)),
 			"CryptDuplicateKey",
-			XMLSEC_ERRORS_R_XMLSEC_FAILED,
+			XMLSEC_ERRORS_R_CRYPTO_FAILED,
 			XMLSEC_ERRORS_NO_MESSAGE);
 	    return(-1);
 	}
     }
-    if (ctxSrc->hProv != 0) {
-	/* What to do when fCallerFreeProv == FALSE ???, add reference or not? */
+    if(ctxSrc->hProv != 0) {
 	CryptContextAddRef(ctxSrc->hProv, NULL, 0);
+	ctxDst->hProv		= ctxSrc->hProv;
+	ctxDst->fCallerFreeProv = TRUE;
+    } else {
+        ctxDst->hProv		= 0;
+	ctxDst->fCallerFreeProv = FALSE;
     }
-
-    ctxDst->hProv = ctxSrc->hProv;
-    ctxDst->fCallerFreeProv = ctxSrc->fCallerFreeProv;
-    ctxDst->dwKeySpec = ctxSrc->dwKeySpec;
-    ctxDst->providerName = ctxSrc->providerName;
-    ctxDst->providerType = ctxSrc->providerType;
-    ctxDst->type = ctxSrc->type;
+    
+    ctxDst->dwKeySpec	    = ctxSrc->dwKeySpec;
+    ctxDst->providerName    = ctxSrc->providerName;
+    ctxDst->providerType    = ctxSrc->providerType;
+    ctxDst->type	    = ctxSrc->type;
 
     return(0);
 }
@@ -312,10 +326,12 @@
     if (ctx->hKey != 0) {
 	CryptDestroyKey(ctx->hKey);
     }
+
     if(ctx->pCert != NULL) {
 	CertFreeCertificateContext(ctx->pCert);
     }
-    if (ctx->hProv != 0 && ctx->fCallerFreeProv) {
+
+    if ((ctx->hProv != 0) && ctx->fCallerFreeProv) {
 	CryptReleaseContext(ctx->hProv, 0);
     }
 
@@ -325,8 +341,6 @@
 static int 
 xmlSecMSCryptoKeyDataGetSize(xmlSecKeyDataPtr data) {
     xmlSecMSCryptoKeyDataCtxPtr ctx;
-    DWORD length = 0;
-    DWORD lenlen = sizeof(DWORD);
 
     xmlSecAssert2(xmlSecKeyDataIsValid(data), 0);
     xmlSecAssert2(xmlSecKeyDataCheckSize(data, xmlSecMSCryptoKeyDataSize), 0);
@@ -335,15 +349,19 @@
     xmlSecAssert2(ctx != NULL, 0);
 
     if(ctx->pCert != NULL) {
+        xmlSecAssert2(ctx->pCert->pCertInfo != NULL, 0);
 	return (CertGetPublicKeyLength(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING, 
 		&(ctx->pCert->pCertInfo->SubjectPublicKeyInfo)));
     } else if (ctx->hKey != 0) {
+        DWORD length = 0;
+	DWORD lenlen = sizeof(DWORD);
+	
 	if (!CryptGetKeyParam(ctx->hKey, KP_KEYLEN, (BYTE *)&length, &lenlen, 0)) {
 	    xmlSecError(XMLSEC_ERRORS_HERE,
 			NULL,
 			"CertDuplicateCertificateContext",
 			XMLSEC_ERRORS_R_CRYPTO_FAILED,
-			"Error no: %d", GetLastError());
+			"GetLastError=%d", GetLastError());
 	    return(0);
 	}
 	return(length);
@@ -405,6 +423,8 @@
     int ret;
     
     xmlSecAssert2(pCert != NULL, NULL);
+    xmlSecAssert2(pCert->pCertInfo != NULL, NULL);
+    xmlSecAssert2(pCert->pCertInfo->SubjectPublicKeyInfo.Algorithm.pszObjId != NULL, NULL);
 
 #ifndef XMLSEC_NO_RSA
     if (!strcmp(pCert->pCertInfo->SubjectPublicKeyInfo.Algorithm.pszObjId, szOID_RSA_RSA)) {
@@ -421,37 +441,39 @@
 #endif /* XMLSEC_NO_RSA */	
 
 #ifndef XMLSEC_NO_DSA
-	if (!strcmp(pCert->pCertInfo->SubjectPublicKeyInfo.Algorithm.pszObjId, szOID_X957_DSA /*szOID_DSALG_SIGN*/)) {
-		data = xmlSecKeyDataCreate(xmlSecMSCryptoKeyDataDsaId);
-		if(data == NULL) {
-			xmlSecError(XMLSEC_ERRORS_HERE,
-				NULL,
-				"xmlSecKeyDataCreate",
-				XMLSEC_ERRORS_R_XMLSEC_FAILED,
-				"xmlSecMSCryptoKeyDataDsaId");
-			return(NULL);	    
-		}
+    if (!strcmp(pCert->pCertInfo->SubjectPublicKeyInfo.Algorithm.pszObjId, szOID_X957_DSA /*szOID_DSALG_SIGN*/)) {
+	data = xmlSecKeyDataCreate(xmlSecMSCryptoKeyDataDsaId);
+	if(data == NULL) {
+		xmlSecError(XMLSEC_ERRORS_HERE,
+	    		    NULL,
+			    "xmlSecKeyDataCreate",
+			    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+			    "xmlSecMSCryptoKeyDataDsaId");
+		return(NULL);	    
 	}
+    }
 #endif *//* XMLSEC_NO_DSA */	
-	if (NULL == data) {
-		xmlSecError(XMLSEC_ERRORS_HERE,
-				NULL,
-				NULL,
-				XMLSEC_ERRORS_R_INVALID_TYPE,
-				"PCCERT_CONTEXT key type %s not supported", pCert->pCertInfo->SubjectPublicKeyInfo.Algorithm.pszObjId);
-		return(NULL);
+
+    if (NULL == data) {
+    	xmlSecError(XMLSEC_ERRORS_HERE,
+		    NULL,
+		    NULL,
+		    XMLSEC_ERRORS_R_INVALID_TYPE,
+		    "PCCERT_CONTEXT key type %s not supported", pCert->pCertInfo->SubjectPublicKeyInfo.Algorithm.pszObjId);
+	return(NULL);
     }
 
     xmlSecAssert2(data != NULL, NULL);    
+
     ret = xmlSecMSCryptoKeyDataAdoptCert(data, pCert, type);
     if(ret < 0) {
-		xmlSecError(XMLSEC_ERRORS_HERE,
-				NULL,
-				"xmlSecMSCryptoPCCDataAdoptPCC",
-				XMLSEC_ERRORS_R_XMLSEC_FAILED,
-				XMLSEC_ERRORS_NO_MESSAGE);
-		xmlSecKeyDataDestroy(data);
-		return(NULL);	    
+	xmlSecError(XMLSEC_ERRORS_HERE,
+			NULL,
+			"xmlSecMSCryptoPCCDataAdoptPCC",
+			XMLSEC_ERRORS_R_XMLSEC_FAILED,
+			XMLSEC_ERRORS_NO_MESSAGE);
+	xmlSecKeyDataDestroy(data);
+	return(NULL);	    
     }
     return(data);
 }
@@ -606,39 +628,70 @@
 
 static int 
 xmlSecMSCryptoKeyDataRsaXmlRead(xmlSecKeyDataId id, xmlSecKeyPtr key,
-    				xmlNodePtr node, xmlSecKeyInfoCtxPtr keyInfoCtx) {
-    
-				    
+    				xmlNodePtr node, xmlSecKeyInfoCtxPtr keyInfoCtx) {				    
+    xmlSecBuffer modulus, exponent, blob;
+    unsigned int blobBufferLen;
+    BLOBHEADER* header = NULL;
+    RSAPUBKEY* pubkey = NULL;
+    xmlSecByte* modulusBlob = NULL;
     xmlSecKeyDataPtr data = NULL;
+    HCRYPTPROV hProv = 0;
+    HCRYPTKEY hKey = 0;
     xmlNodePtr cur;
-    xmlSecBuffer modulus, exponent;
-    xmlSecBufferPtr blob;
-    BLOBHEADER * header;
-    unsigned int blobBufferLen;
-    RSAPUBKEY * pubkey;
-    BYTE *i;
-    unsigned int j;
-    BYTE *exponentBuf;
-    HCRYPTPROV hProv;
-    HCRYPTKEY hKey;
+    int res = -1;
     int ret;
 
     xmlSecAssert2(id == xmlSecMSCryptoKeyDataRsaId, -1);
     xmlSecAssert2(key != NULL, -1);
     xmlSecAssert2(node != NULL, -1);
     xmlSecAssert2(keyInfoCtx != NULL, -1);
-
+    
     if(xmlSecKeyGetValue(key) != NULL) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
 		    NULL,		    
 		    XMLSEC_ERRORS_R_INVALID_KEY_DATA,
 		    "key already has a value");
-	ret = -1;
-	goto done;
+	return(-1);
     }
 
+    /* initialize buffers */
+    ret = xmlSecBufferInitialize(&modulus, 0);
+    if(ret < 0) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    "xmlSecBufferInitialize",
+		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    "modulus");
+	return(-1);
+    }
+
+    ret = xmlSecBufferInitialize(&exponent, 0);
+    if(ret < 0) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    "xmlSecBufferInitialize",
+		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    "exponent");
+	xmlSecBufferFinalize(&modulus);
+	return(-1);
+    }
+
+    ret = xmlSecBufferInitialize(&blob, 0);
+    if(ret < 0) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    "xmlSecBufferInitialize",
+		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    "blob");
+	xmlSecBufferFinalize(&modulus);
+	xmlSecBufferFinalize(&exponent);
+	return(-1);
+    }
+
+    /* read xml */
     cur = xmlSecGetNextElementNode(node->children);
+
     /* first is Modulus node. It is REQUIRED because we do not support Seed and PgenCounter*/
     if((cur == NULL) || (!xmlSecCheckNodeName(cur,  xmlSecNodeRSAModulus, xmlSecDSigNs))) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
@@ -647,21 +700,21 @@
 		    XMLSEC_ERRORS_R_INVALID_NODE,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeRSAModulus));
-	ret = -1;
 	goto done;
     }
-    if(xmlSecMSCryptoNodeGetBigNumValue(cur, &modulus) != 0) {
+
+    ret = xmlSecMSCryptoNodeGetBigNumValue(cur, &modulus);
+    if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
-		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    NULL,
 		    "xmlSecMSCryptoNodeGetBigNumValue",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+	    	    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeRSAModulus));
-	ret = -1;
 	goto done;
     }
     cur = xmlSecGetNextElementNode(cur->next);
-
+    
     /* next is Exponent node. It is REQUIRED because we do not support Seed and PgenCounter*/
     if((cur == NULL) || (!xmlSecCheckNodeName(cur, xmlSecNodeRSAExponent, xmlSecDSigNs))) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
@@ -670,21 +723,20 @@
 		    XMLSEC_ERRORS_R_INVALID_NODE,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeRSAExponent));
-	ret = -1;
 	goto done;
     }
-    if(xmlSecMSCryptoNodeGetBigNumValue(cur, &exponent) != 0) {
+    ret = xmlSecMSCryptoNodeGetBigNumValue(cur, &exponent);
+    if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
-		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    NULL,
 		    "xmlSecMSCryptoNodeGetBigNumValue",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+	    	    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeRSAExponent));
-	ret = -1;
 	goto done;
     }
     cur = xmlSecGetNextElementNode(cur->next);
-
+    
     if((cur != NULL) && (xmlSecCheckNodeName(cur, xmlSecNodeRSAPrivateExponent, xmlSecNs))) {
         /* next is X node. It is REQUIRED for private key but
 	 * MSCrypto does not support it. We just ignore it */ 
@@ -697,71 +749,75 @@
 		    xmlSecErrorsSafeString(xmlSecNodeGetName(cur)),
 		    XMLSEC_ERRORS_R_INVALID_NODE,
 		    "no nodes expected");
-	ret = -1;
 	goto done;
     }
 
     /* Now try to create the key */
     blobBufferLen = sizeof(BLOBHEADER) + sizeof(RSAPUBKEY) + xmlSecBufferGetSize(&modulus);
-    blob = xmlSecBufferCreate(blobBufferLen);
-    xmlSecBufferSetSize(blob, blobBufferLen);
+    ret = xmlSecBufferSetSize(&blob, blobBufferLen);
+    if(ret < 0) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    NULL,
+		    "xmlSecBufferSetSize",
+	    	    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    "size=%d", blobBufferLen);
+	goto done;
+    }
 
     /* Set the blob header */
-    header = (BLOBHEADER *)xmlSecBufferGetData(blob);
-    header->bType = PUBLICKEYBLOB;
-    header->bVersion = 0x02;
-    header->reserved = 0;
-    header->aiKeyAlg = CALG_RSA_KEYX | CALG_RSA_SIGN;
+    header = (BLOBHEADER *)xmlSecBufferGetData(&blob);
+    header->bType	= PUBLICKEYBLOB;
+    header->bVersion	= 0x02;
+    header->reserved	= 0;
+    header->aiKeyAlg	= CALG_RSA_KEYX | CALG_RSA_SIGN;
 
     /* Set the public key header */
-    pubkey = (RSAPUBKEY *) (xmlSecBufferGetData(blob) + sizeof(BLOBHEADER));
-
-    pubkey->magic = 0x31415352;	/* == RSA1 */
-    pubkey->bitlen = xmlSecBufferGetSize(&modulus) * 8;	/* Number of bits in prime modulus */
-    pubkey->pubexp = 0;
-    i = ((BYTE *) &(pubkey->pubexp));
-    exponentBuf = xmlSecBufferGetData(&exponent);
-    for (j = 0; j < xmlSecBufferGetSize(&exponent); ++j) {
-	*i++ = exponentBuf[j];
-    }
-
-    /* copy in the modulus */
-    i = (BYTE *) (pubkey);
-    i += sizeof(RSAPUBKEY);
+    pubkey = (RSAPUBKEY*) (xmlSecBufferGetData(&blob) + sizeof(BLOBHEADER));
+    pubkey->magic	= 0x31415352;	/* == RSA1 public */
+    pubkey->bitlen	= xmlSecBufferGetSize(&modulus) * 8;	/* Number of bits in prime modulus */
+    pubkey->pubexp	= 0;
+    if(sizeof(pubkey->pubexp) < xmlSecBufferGetSize(&exponent)) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    NULL,
+		    NULL,
+	    	    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+		    "exponent size=%d", 
+		    xmlSecBufferGetSize(&exponent));
+	goto done;
+    }
+    memcpy(&(pubkey->pubexp), xmlSecBufferGetData(&exponent), xmlSecBufferGetSize(&exponent));
 
-    memcpy(i, xmlSecBufferGetData(&modulus), xmlSecBufferGetSize(&modulus));
+    modulusBlob = (xmlSecByte*) (xmlSecBufferGetData(&blob) + sizeof(BLOBHEADER) + sizeof(RSAPUBKEY));
+    memcpy(modulusBlob, xmlSecBufferGetData(&modulus), xmlSecBufferGetSize(&modulus));
 
     /* Now that we have the blob, import */
     if (!CryptAcquireContext(&hProv, NULL, MS_ENHANCED_PROV, PROV_RSA_FULL, 0)) {
-	if (NTE_BAD_KEYSET == GetLastError()) {
-	    if (!CryptAcquireContext(&hProv, NULL, MS_ENHANCED_PROV, PROV_RSA_FULL, CRYPT_NEWKEYSET)) {
+	if(NTE_BAD_KEYSET == GetLastError()) {
+	    if(!CryptAcquireContext(&hProv, NULL, MS_ENHANCED_PROV, PROV_RSA_FULL, CRYPT_NEWKEYSET)) {
 		xmlSecError(XMLSEC_ERRORS_HERE,
-		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
-		    "CryptAcquireContext",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    "Error no: %d", GetLastError());
-		ret = -1;
+			    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+			    "CryptAcquireContext",
+			    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+			    "GetLastError=%d", GetLastError());
 		goto done;
 	    }
 	} else {
-	xmlSecError(XMLSEC_ERRORS_HERE,
-		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
-		    "CryptAcquireContext",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    XMLSEC_ERRORS_NO_MESSAGE);
-	ret = -1;
-	goto done;
+    	    xmlSecError(XMLSEC_ERRORS_HERE,
+			xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+			"CryptAcquireContext",
+		        XMLSEC_ERRORS_R_CRYPTO_FAILED,
+			XMLSEC_ERRORS_NO_MESSAGE);
+	    goto done;
 	}
     }
-    if (!CryptImportKey(hProv, xmlSecBufferGetData(blob), xmlSecBufferGetSize(blob), 0, 0, &hKey)) {
+
+    if (!CryptImportKey(hProv, xmlSecBufferGetData(&blob), xmlSecBufferGetSize(&blob), 0, 0, &hKey)) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
 		    "CryptImportKey",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
 		    XMLSEC_ERRORS_NO_MESSAGE);
-	ret = -1;
 	goto done;
-
     }
 
     data = xmlSecKeyDataCreate(id);
@@ -771,7 +827,6 @@
 		    "xmlSecKeyDataCreate",
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    XMLSEC_ERRORS_NO_MESSAGE);
-	ret = -1;
 	goto done;
     }
 
@@ -782,11 +837,11 @@
 		    "xmlSecMSCryptoKeyDataAdoptKey",
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    XMLSEC_ERRORS_NO_MESSAGE);
-	xmlSecKeyDataDestroy(data);
 	goto done;
     }
+    hProv = 0;
     hKey = 0;
-     
+
     ret = xmlSecKeySetValue(key, data);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
@@ -799,35 +854,37 @@
     }
     data = NULL;
 
-    ret = 0;
+    /* success */
+    res = 0;
 
 done:
-    if (ret != 0) {
-	if (hProv == 0) {
-	    CryptReleaseContext(hProv, 0);
-	}
-	if (hKey != 0) {
-            CryptDestroyKey(hKey);
-	}
-	if (data != 0) {
-            xmlSecKeyDataDestroy(data);
-	}
+    if (hProv == 0) {
+	CryptReleaseContext(hProv, 0);
     }
-    return(ret);
+    if (hKey != 0) {
+        CryptDestroyKey(hKey);
+    }
+    if (data != 0) {
+        xmlSecKeyDataDestroy(data);
+    }
+
+    xmlSecBufferFinalize(&modulus);
+    xmlSecBufferFinalize(&exponent);
+    xmlSecBufferFinalize(&blob);
+    return(res);
 }
 
 static int 
 xmlSecMSCryptoKeyDataRsaXmlWrite(xmlSecKeyDataId id, xmlSecKeyPtr key,
 				xmlNodePtr node, xmlSecKeyInfoCtxPtr keyInfoCtx) {
     xmlSecMSCryptoKeyDataCtxPtr ctx;
+    xmlSecBuffer buf;
     DWORD dwBlobLen;
-    BYTE *blobBuffer, *i, *exponentBuffer;
-    xmlSecBufferPtr blob, modulus, exponent;
-    RSAPUBKEY *pk;
-    DWORD keyLen;
+    xmlSecByte* blob;
+    RSAPUBKEY *pubkey;
+    xmlSecSize modulusLen, exponentLen;
     xmlNodePtr cur;
     int ret;
-    unsigned int exponentLen;
     
     xmlSecAssert2(id == xmlSecMSCryptoKeyDataRsaId, -1);
     xmlSecAssert2(key != NULL, -1);
@@ -843,49 +900,56 @@
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
 		    "CryptExportKey",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
 		    XMLSEC_ERRORS_NO_MESSAGE);
 	return(-1);
     }
-    blob = xmlSecBufferCreate(dwBlobLen);
-    xmlSecBufferSetSize(blob, dwBlobLen);
-    blobBuffer = xmlSecBufferGetData(blob);
-    if (!CryptExportKey(ctx->hKey, 0, PUBLICKEYBLOB, 0, blobBuffer, &dwBlobLen)) {
+    
+    ret = xmlSecBufferInitialize(&buf, dwBlobLen);
+    if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
-		    "CryptExportKey",
+		    "xmlSecBufferInitialize",
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    XMLSEC_ERRORS_NO_MESSAGE);
+		    "size=%d", dwBlobLen);
 	return(-1);
     }
-    if (dwBlobLen < 1) {
+
+    blob = xmlSecBufferGetData(&buf);
+    if (!CryptExportKey(ctx->hKey, 0, PUBLICKEYBLOB, 0, blob, &dwBlobLen)) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
 		    "CryptExportKey",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    "Blob length < 1");
+		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+		    XMLSEC_ERRORS_NO_MESSAGE);
+	xmlSecBufferFinalize(&buf);
 	return(-1);
     }
+    if (dwBlobLen < sizeof(BLOBHEADER)) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    NULL,
+		    XMLSEC_ERRORS_R_INVALID_DATA,
+		    "blobLen=%d", dwBlobLen);
+	xmlSecBufferFinalize(&buf);
+	return(-1);
+    }
+    /* todo: check BLOBHEADER */
 
-    pk = (RSAPUBKEY *)(blobBuffer + sizeof(BLOBHEADER));
-    keyLen = pk->bitlen / 8;
-    /* Copy the key */
-    i = (BYTE *)pk;
-    i += sizeof(RSAPUBKEY);
-    modulus = xmlSecBufferCreate(keyLen);
-    xmlSecBufferSetData(modulus, i, keyLen);
-
-    exponent = xmlSecBufferCreate(4);
-    xmlSecBufferSetData(exponent, (BYTE *)(&(pk->pubexp)), 4);
+    pubkey	    = (RSAPUBKEY *)(blob + sizeof(BLOBHEADER));
+    /* todo: check RSAPUBKEY */
+    modulusLen	    = pubkey->bitlen / 8;
 
-    /* Remove leading zero's (from least significant end) */
-    exponentBuffer = xmlSecBufferGetData(exponent);
-    exponentLen = 3;
-    while (exponentLen > 0 && exponentBuffer[exponentLen] == 0) {
-	exponentLen--;
+    if (dwBlobLen < sizeof(BLOBHEADER) + sizeof(RSAPUBKEY) + modulusLen) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    NULL,
+		    XMLSEC_ERRORS_R_INVALID_DATA,
+		    "blobLen=%d; modulusLen=%d", dwBlobLen, modulusLen);
+	xmlSecBufferFinalize(&buf);
+	return(-1);
     }
-    exponentLen++;
-    xmlSecBufferSetSize(exponent, exponentLen);
+    blob	    += sizeof(BLOBHEADER) + sizeof(RSAPUBKEY);
 
     /* first is Modulus node */
     cur = xmlSecAddChild(node, xmlSecNodeRSAModulus, xmlSecDSigNs);
@@ -896,16 +960,19 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeRSAModulus));
+	xmlSecBufferFinalize(&buf);
 	return(-1);	
     }
-    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, modulus, 1);
+
+    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, blob, modulusLen, 1);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
-		    "xmlSecNssNodeSetBigNumValue",
+		    "xmlSecMSCryptoNodeSetBigNumValue",
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeRSAModulus));
+	xmlSecBufferFinalize(&buf);
 	return(-1);
     }    
 
@@ -918,21 +985,32 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeRSAExponent));
+	xmlSecBufferFinalize(&buf);
 	return(-1);	
     }
-    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, exponent, 1);
+
+    /* Remove leading zero's (from least significant end) */
+    blob	= (xmlSecByte*)(&(pubkey->pubexp));
+    exponentLen = sizeof(pubkey->pubexp);
+    while (exponentLen > 0 && blob[exponentLen - 1] == 0) {
+	exponentLen--;
+    }
+
+    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, blob, exponentLen, 1);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
-		    "xmlSecNssNodeSetBigNumValue",
+		    "xmlSecMSCryptoNodeSetBigNumValue",
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
-		    xmlSecErrorsSafeString(xmlSecNodeRSAExponent));
-    return(-1);	
-}
+    		    xmlSecErrorsSafeString(xmlSecNodeRSAExponent));
+	xmlSecBufferFinalize(&buf);
+	return(-1);	
+    }
 
     /* next is PrivateExponent node: not supported in MSCrypto */
 
+    xmlSecBufferFinalize(&buf);
     return(0);
 }
 
@@ -940,11 +1018,12 @@
 xmlSecMSCryptoKeyDataRsaGenerate(xmlSecKeyDataPtr data, xmlSecSize sizeBits, 
 				xmlSecKeyDataType type ATTRIBUTE_UNUSED) {
     xmlSecMSCryptoKeyDataCtxPtr ctx;
-    HCRYPTPROV hProv;
+    HCRYPTPROV hProv = 0;
+    HCRYPTKEY hKey = 0;
     DWORD dwKeySpec;
-    HCRYPTKEY hKey;
     DWORD dwSize;
-    int ret = -1;
+    int res = -1;
+    int ret;
 
     xmlSecAssert2(xmlSecKeyDataIsValid(data), xmlSecKeyDataTypeUnknown);
     xmlSecAssert2(xmlSecKeyDataCheckSize(data, xmlSecMSCryptoKeyDataSize), xmlSecKeyDataTypeUnknown);
@@ -952,44 +1031,43 @@
     xmlSecAssert2(sizeBits > 0, -1);
 
     ctx = xmlSecMSCryptoKeyDataGetCtx(data);
+    xmlSecAssert2(ctx != NULL, -1);
 
     if (!CryptAcquireContext(&hProv, XMLSEC_CONTAINER_NAME, MS_STRONG_PROV, PROV_RSA_FULL, 0)) {
 	if (NTE_BAD_KEYSET == GetLastError()) {
 	    if(!CryptAcquireContext(&hProv, XMLSEC_CONTAINER_NAME, MS_STRONG_PROV, PROV_RSA_FULL, CRYPT_NEWKEYSET)) {
-	xmlSecError(XMLSEC_ERRORS_HERE,
-		    xmlSecErrorsSafeString(xmlSecKeyDataGetName(data)),
-		    "CryptAcquireContext",
-		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
-		    "error code=%d", GetLastError());
+		xmlSecError(XMLSEC_ERRORS_HERE,
+			    xmlSecErrorsSafeString(xmlSecKeyDataGetName(data)),
+			    "CryptAcquireContext",
+			    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+			    "GetLastError=%d", GetLastError());
         
-	return(-1);
-    }
+	    	return(-1);
+	    }
 	} else {
 	    xmlSecError(XMLSEC_ERRORS_HERE,
 			    xmlSecErrorsSafeString(xmlSecKeyDataGetName(data)),
 			    "CryptAcquireContext",
 			    XMLSEC_ERRORS_R_CRYPTO_FAILED,
-			    "error code=%d", GetLastError());
+			    "GetLastError=%d", GetLastError());
                 
 	    return(-1);
 	}
     }
 
     dwKeySpec = AT_KEYEXCHANGE | AT_SIGNATURE;
-
     dwSize = ((sizeBits << 16) | CRYPT_EXPORTABLE);
-
     if (!CryptGenKey(hProv, CALG_RSA_SIGN, dwSize, &hKey)) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataGetName(data)),
 		    "CryptGenKey",
 		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
-		    "error code=%d", GetLastError());
+		    "GetLastError=%d", GetLastError());
 	goto done;
     }
 
     ret = xmlSecMSCryptoKeyDataAdoptKey(data, hProv, TRUE, hKey, dwKeySpec, 
-	xmlSecKeyDataTypePublic | xmlSecKeyDataTypePrivate);
+					xmlSecKeyDataTypePublic | xmlSecKeyDataTypePrivate);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataGetName(data)),
@@ -998,23 +1076,22 @@
 		    XMLSEC_ERRORS_NO_MESSAGE);
 	goto done;
     }
+    hProv = 0;
+    hKey = 0;
 
-    ret = 0;
+    /* success */
+    res = 0;
 
 done:
-    if (ret == 0) {
-	return (0);
-    }
-
     if (hProv != 0) {
-	CryptReleaseContext(ctx->hProv, 0);
+	CryptReleaseContext(hProv, 0);
     }
 
     if (hKey != 0) {
 	CryptDestroyKey(hKey);
     }
 
-    return(-1);
+    return(res);
 }
 
 static xmlSecKeyDataType 
@@ -1236,14 +1313,16 @@
 			   xmlNodePtr node, xmlSecKeyInfoCtxPtr keyInfoCtx) {
     xmlSecKeyDataPtr data = NULL;
     xmlNodePtr cur;
-    xmlSecBuffer p, q, g, y;
-    xmlSecBufferPtr keyBlob;
-    BYTE *keyBlobBuf, *i;
-    unsigned int blobBufferLen, j;
-    BLOBHEADER *header;
-    DSSPUBKEY *pubkey;
+    xmlSecBuffer p, q, g, y, blob;
+    unsigned int blobBufferLen;
+    BLOBHEADER *header = NULL;
+    DSSPUBKEY *pubkey = NULL;
+    DSSSEED* seed = NULL;
+    BYTE *buf = NULL;
     HCRYPTPROV hProv = 0;
     HCRYPTKEY hKey = 0;
+    xmlSecSize i;
+    int res = -1;
     int ret;
 
     xmlSecAssert2(id == xmlSecMSCryptoKeyDataDsaId, -1);
@@ -1256,11 +1335,72 @@
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
 		    NULL,
 		    XMLSEC_ERRORS_R_INVALID_KEY_DATA,
-		    XMLSEC_ERRORS_NO_MESSAGE);
-	ret = -1;
-	goto done;
+		    "key already has a value");
+	return(-1);
     }
 
+    /* initialize buffers */
+    ret = xmlSecBufferInitialize(&p, 0);
+    if(ret < 0) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    "xmlSecBufferInitialize",
+		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    "p");
+	return(-1);
+    }
+
+    ret = xmlSecBufferInitialize(&q, 0);
+    if(ret < 0) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    "xmlSecBufferInitialize",
+		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    "q");
+	xmlSecBufferFinalize(&p);
+	return(-1);
+    }
+
+    ret = xmlSecBufferInitialize(&g, 0);
+    if(ret < 0) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    "xmlSecBufferInitialize",
+		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    "g");
+	xmlSecBufferFinalize(&p);
+	xmlSecBufferFinalize(&q);
+	return(-1);
+    }
+
+    ret = xmlSecBufferInitialize(&y, 0);
+    if(ret < 0) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    "xmlSecBufferInitialize",
+		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    "y");
+	xmlSecBufferFinalize(&p);
+	xmlSecBufferFinalize(&q);
+	xmlSecBufferFinalize(&g);
+	return(-1);
+    }
+
+    ret = xmlSecBufferInitialize(&blob, 0);
+    if(ret < 0) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    "xmlSecBufferInitialize",
+		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    "blob");
+	xmlSecBufferFinalize(&p);
+	xmlSecBufferFinalize(&q);
+	xmlSecBufferFinalize(&g);
+	xmlSecBufferFinalize(&y);
+	return(-1);
+    }
+
+    /* read xml */
     cur = xmlSecGetNextElementNode(node->children);
 
     /* first is P node. It is REQUIRED because we do not support Seed and PgenCounter*/
@@ -1271,7 +1411,6 @@
 		    XMLSEC_ERRORS_R_INVALID_NODE,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAP));
-	ret = -1;
 	goto done;
     }
     if(xmlSecMSCryptoNodeGetBigNumValue(cur, &p) != 0) {
@@ -1281,7 +1420,6 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAP));
-	ret = -1;
 	goto done;
     }
     cur = xmlSecGetNextElementNode(cur->next);
@@ -1294,7 +1432,6 @@
 		    XMLSEC_ERRORS_R_INVALID_NODE,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAQ));
-	ret = -1;
 	goto done;
     }
     if(xmlSecMSCryptoNodeGetBigNumValue(cur, &q) != 0) {
@@ -1304,7 +1441,6 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAQ));
-	ret = -1;
 	goto done;
     }
     cur = xmlSecGetNextElementNode(cur->next);
@@ -1317,7 +1453,6 @@
 		    XMLSEC_ERRORS_R_INVALID_NODE,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAG));
-	ret = -1;
 	goto done;
     }
     if(xmlSecMSCryptoNodeGetBigNumValue(cur, &g) != 0) {
@@ -1327,7 +1462,6 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAG));
-	ret = -1;
 	goto done;
     }
     cur = xmlSecGetNextElementNode(cur->next);
@@ -1347,7 +1481,6 @@
 		    XMLSEC_ERRORS_R_INVALID_NODE,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAY));
-	ret = -1;
 	goto done;
     }
     if(xmlSecMSCryptoNodeGetBigNumValue(cur, &y) != 0) {
@@ -1356,7 +1489,6 @@
 		    "xmlSecMSCryptoNodeGetBigNumValue",
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", xmlSecErrorsSafeString(xmlSecNodeDSAY));
-	ret = -1;
 	goto done;
     }
     cur = xmlSecGetNextElementNode(cur->next);
@@ -1377,58 +1509,97 @@
 		    xmlSecErrorsSafeString(xmlSecNodeGetName(cur)),
 		    XMLSEC_ERRORS_R_UNEXPECTED_NODE,
 		    XMLSEC_ERRORS_NO_MESSAGE);
-	ret = -1;
 	goto done;
     }
 
-    
-    blobBufferLen = sizeof(BLOBHEADER) + sizeof(DSSPUBKEY) + (3 * xmlSecBufferGetSize(&p)) + 0x14 + 0x18 /*sizeof(DSSSEED)*/;
-    keyBlob = xmlSecBufferCreate(blobBufferLen);
-    xmlSecBufferSetSize(keyBlob, blobBufferLen);
+    /* we assume that sizeof(q) < 0x14, sizeof(g) <= sizeof(p) and sizeof(y) <= sizeof(p) */
+    blobBufferLen = sizeof(BLOBHEADER) + sizeof(DSSPUBKEY) + 3 * xmlSecBufferGetSize(&p) + 0x14 + sizeof(DSSSEED);
+    ret = xmlSecBufferSetSize(&blob, blobBufferLen);
+    if(ret < 0) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    NULL,
+		    "xmlSecBufferSetSize",
+	    	    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    "size=%d", blobBufferLen);
+	goto done;
+    }
 
     /* Set blob header */
-    header = (BLOBHEADER *)xmlSecBufferGetData(keyBlob);
-    header->bType = PUBLICKEYBLOB;
-    header->bVersion = 0x02;
-    header->reserved = 0;
-    header->aiKeyAlg = CALG_DSS_SIGN;
+    header		= (BLOBHEADER *)xmlSecBufferGetData(&blob);
+    header->bType	= PUBLICKEYBLOB;
+    header->bVersion	= 0x02;
+    header->reserved	= 0;
+    header->aiKeyAlg	= CALG_DSS_SIGN;
 
     /* Set the public key header */
-    pubkey = (DSSPUBKEY *) (xmlSecBufferGetData(keyBlob) + sizeof(BLOBHEADER));
-    pubkey->magic = 0x31535344;	/* == DSS1 */
-    pubkey->bitlen = xmlSecBufferGetSize(&p) * 8; /* Number of bits in prime modulus */
-
-    /* copy the keys */
-    i = (BYTE *)(pubkey);
-    i += sizeof(DSSPUBKEY);
-
-    memcpy(i, xmlSecBufferGetData(&p), xmlSecBufferGetSize(&p));
-    i+= xmlSecBufferGetSize(&p);
-    memcpy(i, xmlSecBufferGetData(&q), xmlSecBufferGetSize(&q));
-    i+= xmlSecBufferGetSize(&q);
+    pubkey		= (DSSPUBKEY *) (xmlSecBufferGetData(&blob) + sizeof(BLOBHEADER));
+    pubkey->magic	= 0x31535344;	/* == DSS1 pub key */
+    pubkey->bitlen	= xmlSecBufferGetSize(&p) * 8; /* Number of bits in prime modulus */
+
+    /* copy the key data */
+    buf			= (BYTE*) (xmlSecBufferGetData(&blob) + sizeof(BLOBHEADER) + sizeof(DSSPUBKEY));
+    
+    /* set p */
+    memcpy(buf, xmlSecBufferGetData(&p), xmlSecBufferGetSize(&p));
+    buf += xmlSecBufferGetSize(&p);
+
+    /* set q */
+    if(xmlSecBufferGetSize(&q) > 0x14) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    NULL,
+		    "q",
+	    	    XMLSEC_ERRORS_R_INVALID_SIZE,
+		    "size=%d > 0x14", xmlSecBufferGetSize(&q));
+	goto done;
+    }
+    memcpy(buf, xmlSecBufferGetData(&q), xmlSecBufferGetSize(&q));
+    buf += xmlSecBufferGetSize(&q);
+
     /* Pad with zeros */
-    for (j = xmlSecBufferGetSize(&q); j < 20 ; ++j) {
-	*i++ = 0;
+    for(i = xmlSecBufferGetSize(&q); i < 0x14; ++i) {
+	*(buf++) = 0;    
     }
+
     /* set generator */
-    memcpy(i, xmlSecBufferGetData(&g), xmlSecBufferGetSize(&g));
-    i+= xmlSecBufferGetSize(&g);
-    /* Pad */
-    for (j = xmlSecBufferGetSize(&g); j < xmlSecBufferGetSize(&p) ; ++j) {
-	*i++ = 0;
+    if(xmlSecBufferGetSize(&g) > xmlSecBufferGetSize(&p)) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    NULL,
+		    "g",
+	    	    XMLSEC_ERRORS_R_INVALID_SIZE,
+		    "size=%d > %d", 
+		    xmlSecBufferGetSize(&g), 
+		    xmlSecBufferGetSize(&p));
+	goto done;
+    }
+    memcpy(buf, xmlSecBufferGetData(&g), xmlSecBufferGetSize(&g));
+    buf += xmlSecBufferGetSize(&g);
+    /* Pad with zeros */
+    for(i = xmlSecBufferGetSize(&g); i < xmlSecBufferGetSize(&p); ++i) {
+	*(buf++) = 0;    
     }
+
     /* Public key */
-    memcpy(i, xmlSecBufferGetData(&y), xmlSecBufferGetSize(&y));
-    i+= xmlSecBufferGetSize(&y);
-    /* Pad */
-    for (j = xmlSecBufferGetSize(&y); j < xmlSecBufferGetSize(&p) ; ++j)
-	*i++ = 0;
-
-    /* Set seed to 0 */
-    for (j = 0; j < 0x18; ++j) {
-	*i++ = 0xFF;	/* SEED Counter set to 0xFFFFFFFF will cause seed to be ignored */
+    if(xmlSecBufferGetSize(&y) > xmlSecBufferGetSize(&p)) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    NULL,
+		    "y",
+	    	    XMLSEC_ERRORS_R_INVALID_SIZE,
+		    "size=%d > %d", 
+		    xmlSecBufferGetSize(&y), 
+		    xmlSecBufferGetSize(&p));
+	goto done;
+    }
+    memcpy(buf, xmlSecBufferGetData(&y), xmlSecBufferGetSize(&y));
+    buf += xmlSecBufferGetSize(&y);
+    /* Pad with zeros */
+    for(i = xmlSecBufferGetSize(&y); i < xmlSecBufferGetSize(&p); ++i) {
+	*(buf++) = 0;    
     }
 
+    /* Set seed to 0xFFFFFFFFF */
+    seed = (DSSSEED*)buf;
+    memset(seed, 0, sizeof(*seed));
+    seed->counter = 0xFFFFFFFF; /* SEED Counter set to 0xFFFFFFFF will cause seed to be ignored */
 
     if (!CryptAcquireContext(&hProv, NULL, MS_DEF_DSS_PROV, PROV_DSS, 0)) {
 	if (NTE_BAD_KEYSET == GetLastError()) {
@@ -1436,30 +1607,27 @@
 		xmlSecError(XMLSEC_ERRORS_HERE,
 			    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
 			    "CryptAcquireContext",
-			    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-			    "Error no: %d", GetLastError());
-		ret = -1;
+			    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+			    "GetLastError=%d", GetLastError());
 		goto done;
 	    }
 	} else {
 	    xmlSecError(XMLSEC_ERRORS_HERE,
 			xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
 			"CryptAcquireContext",
-			XMLSEC_ERRORS_R_XMLSEC_FAILED,
-			"Error no: %d", GetLastError());
-	    ret = -1;
+			XMLSEC_ERRORS_R_CRYPTO_FAILED,
+			"GetLastError=%d", GetLastError());
 	    goto done;
 	}
     }
 
     /* import the key blob */
-    if (!CryptImportKey(hProv, xmlSecBufferGetData(keyBlob), xmlSecBufferGetSize(keyBlob), 0, 0, &hKey)) {
+    if (!CryptImportKey(hProv, xmlSecBufferGetData(&blob), xmlSecBufferGetSize(&blob), 0, 0, &hKey)) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
 		    "CryptImportKey",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
+		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
 		    XMLSEC_ERRORS_NO_MESSAGE);
-	ret = -1;
 	goto done;
     }
 
@@ -1470,7 +1638,6 @@
 		    "xmlSecKeyDataCreate",
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    XMLSEC_ERRORS_NO_MESSAGE);
-	ret = -1;
 	goto done;
     }
 
@@ -1483,6 +1650,8 @@
 		    XMLSEC_ERRORS_NO_MESSAGE);
 	goto done;
     }
+    hProv = 0;
+    hKey = 0;
 
     ret = xmlSecKeySetValue(key, data);
     if(ret < 0) {
@@ -1495,40 +1664,41 @@
     }
     data = NULL;
 
-    ret = 0;
+    /* success */
+    res = 0;
 
 done:
-    if (ret != 0) {
-	if (hKey != 0) {
-	    CryptDestroyKey(hKey);
-	}
-	if (hProv != 0) {
-	    CryptReleaseContext(hProv, 0);
-	}
-	if (data != NULL) {
-	    xmlSecKeyDataDestroy(data);
-	}
+    if (hKey != 0) {
+	CryptDestroyKey(hKey);
+    }
+    if (hProv != 0) {
+	CryptReleaseContext(hProv, 0);
+    }
+    if (data != NULL) {
+	xmlSecKeyDataDestroy(data);
     }
 
-    xmlSecBufferFinalize(keyBlob);
+    xmlSecBufferFinalize(&blob);
     xmlSecBufferFinalize(&p);
     xmlSecBufferFinalize(&q);
     xmlSecBufferFinalize(&g);
     xmlSecBufferFinalize(&y);
 
-    return(ret);
+    return(res);
 }
 
 static int 
 xmlSecMSCryptoKeyDataDsaXmlWrite(xmlSecKeyDataId id, xmlSecKeyPtr key,
 				xmlNodePtr node, xmlSecKeyInfoCtxPtr keyInfoCtx) {
     xmlSecMSCryptoKeyDataCtxPtr ctx;
-    DWORD dwBlobLen, keyLen;
-    BYTE *blobBuffer, *i;
-    xmlSecBufferPtr blob, p, q, g, y;
-    DSSPUBKEY *pk;
+    xmlSecBuffer buf;
+    DWORD dwBlobLen;
+    xmlSecByte* blob;
+    DSSPUBKEY *pubkey;
+    xmlSecSize keyLen, len;
     xmlNodePtr cur;
-    int ret, len;
+    int ret;
+
     
     xmlSecAssert2(id == xmlSecMSCryptoKeyDataDsaId, -1);
     xmlSecAssert2(key != NULL, -1);
@@ -1548,60 +1718,52 @@
 		    XMLSEC_ERRORS_NO_MESSAGE);
 	return(-1);
     }
-    blob = xmlSecBufferCreate(dwBlobLen);
-    xmlSecBufferSetSize(blob, dwBlobLen);
-    blobBuffer = xmlSecBufferGetData(blob);
-    if (!CryptExportKey(ctx->hKey, 0, PUBLICKEYBLOB, 0, blobBuffer, &dwBlobLen)) {
+
+    ret = xmlSecBufferInitialize(&buf, dwBlobLen);
+    if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
-		    "CryptExportKey",
+		    "xmlSecBufferInitialize",
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    XMLSEC_ERRORS_NO_MESSAGE);
+		    "size=%d", dwBlobLen);
 	return(-1);
     }
-    if (dwBlobLen < 1) {
+
+    blob = xmlSecBufferGetData(&buf);
+    if (!CryptExportKey(ctx->hKey, 0, PUBLICKEYBLOB, 0, blob, &dwBlobLen)) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
 		    "CryptExportKey",
-		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
-		    "Blob length < 1");
+		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
+		    XMLSEC_ERRORS_NO_MESSAGE);
+	xmlSecBufferFinalize(&buf);
 	return(-1);
     }
+    if (dwBlobLen < sizeof(BLOBHEADER)) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    NULL,
+		    XMLSEC_ERRORS_R_INVALID_DATA,
+		    "blobLen=%d", dwBlobLen);
+	xmlSecBufferFinalize(&buf);
+	return(-1);
+    }
+    /* todo: check BLOBHEADER */
 
-    /* Now first get all the needed buffers */
-    /* TODO: Adds loads of xmlSecBuffer error catching and handling */
-    pk = (DSSPUBKEY *)(blobBuffer + sizeof(BLOBHEADER));
-    keyLen = pk->bitlen / 8;
-	
-    i = (BYTE *)(pk);
-    i += sizeof(DSSPUBKEY);
+    pubkey	    = (DSSPUBKEY*)(blob + sizeof(BLOBHEADER));
+    keyLen	    = pubkey->bitlen / 8;
 
-    p = xmlSecBufferCreate(keyLen);
-    xmlSecBufferSetData(p, i, keyLen);
-    i+=keyLen;
-
-    len = 20;
-    while (i[len - 1] == 0 && len > 0) {
-	len--;
-    }
-    q = xmlSecBufferCreate(len);
-    xmlSecBufferSetData(q, i, len);
-    i+=20;
-
-    len = keyLen;
-    while (i[len - 1] == 0 && len > 0) {
-	len--;
-    }
-    g = xmlSecBufferCreate(len);
-    xmlSecBufferSetData(g, i, len);
-    i+=keyLen;
-
-    len = keyLen;
-    while (i[len] == 0 && len > 0) {
-	len--;
+    /* we assume that sizeof(q) < 0x14, sizeof(g) <= sizeof(p) and sizeof(y) <= sizeof(p) */
+    if (dwBlobLen < sizeof(BLOBHEADER) + sizeof(DSSPUBKEY) + 3 * keyLen + 0x14 + sizeof(DSSSEED)) {
+	xmlSecError(XMLSEC_ERRORS_HERE,
+		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
+		    NULL,
+		    XMLSEC_ERRORS_R_INVALID_DATA,
+		    "blobLen=%d; keyLen=%d", dwBlobLen, keyLen);
+	xmlSecBufferFinalize(&buf);
+	return(-1);
     }
-    y = xmlSecBufferCreate(len);
-    xmlSecBufferSetData(y, i, len);
+    blob	    += sizeof(BLOBHEADER) + sizeof(DSSPUBKEY);
 
     /* first is P node */
     cur = xmlSecAddChild(node, xmlSecNodeDSAP, xmlSecDSigNs);
@@ -1612,9 +1774,11 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAP));
+	xmlSecBufferFinalize(&buf);
 	return(-1);	
     }
-    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, p, 1);
+
+    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, blob, keyLen, 1);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
@@ -1622,8 +1786,10 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAP));
+	xmlSecBufferFinalize(&buf);
 	return(-1);
     }    
+    blob += keyLen;
 
     /* next is Q node. */
     cur = xmlSecAddChild(node, xmlSecNodeDSAQ, xmlSecDSigNs);
@@ -1634,9 +1800,14 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAQ));
+	xmlSecBufferFinalize(&buf);
 	return(-1);	
     }
-    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, q, 1);
+
+    /* we think that the size of q is 0x14, skip trailing zeros */
+    for(len = 0x14; len > 0 && blob[len - 1] == 0; --len);
+
+    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, blob, len, 1);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
@@ -1644,8 +1815,10 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAQ));
+	xmlSecBufferFinalize(&buf);
 	return(-1);
     }
+    blob += 0x14;
 
     /* next is G node. */
     cur = xmlSecAddChild(node, xmlSecNodeDSAG, xmlSecDSigNs);
@@ -1656,9 +1829,14 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAG));
+	xmlSecBufferFinalize(&buf);
 	return(-1);	
     }
-    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, g, 1);
+
+    /* skip trailing zeros */
+    for(len = keyLen; len > 0 && blob[len - 1] == 0; --len);
+
+    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, blob, len, 1);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
@@ -1666,8 +1844,10 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAG));
-    return(-1);
-}
+	xmlSecBufferFinalize(&buf);
+        return(-1);
+    }
+    blob += keyLen;
 
     /* next is X node: not supported in MSCrypto */
 
@@ -1680,9 +1860,14 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAY));
+	xmlSecBufferFinalize(&buf);
 	return(-1);	
     }
-    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, y, 1);
+
+    /* skip trailing zeros */
+    for(len = keyLen; len > 0 && blob[len - 1] == 0; --len);
+
+    ret = xmlSecMSCryptoNodeSetBigNumValue(cur, blob, len, 1);
     if(ret < 0) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)),
@@ -1690,20 +1875,24 @@
 		    XMLSEC_ERRORS_R_XMLSEC_FAILED,
 		    "node=%s", 
 		    xmlSecErrorsSafeString(xmlSecNodeDSAY));
+	xmlSecBufferFinalize(&buf);
 	return(-1);
     }
+    blob += keyLen;
 
+    xmlSecBufferFinalize(&buf);
     return(0);
 }
 
 static int
 xmlSecMSCryptoKeyDataDsaGenerate(xmlSecKeyDataPtr data, xmlSecSize sizeBits, xmlSecKeyDataType type ATTRIBUTE_UNUSED) {
     xmlSecMSCryptoKeyDataCtxPtr ctx;
-    HCRYPTPROV hProv; 
+    HCRYPTPROV hProv = 0; 
+    HCRYPTKEY hKey = 0;
     DWORD dwKeySpec;
-    HCRYPTKEY hKey;
     DWORD dwSize;
-    int ret = -1;
+    int res = -1;
+    int ret;
 
     xmlSecAssert2(xmlSecKeyDataIsValid(data), xmlSecKeyDataTypeUnknown);
     xmlSecAssert2(xmlSecKeyDataCheckSize(data, xmlSecMSCryptoKeyDataSize), xmlSecKeyDataTypeUnknown);
@@ -1719,7 +1908,7 @@
 			    xmlSecErrorsSafeString(xmlSecKeyDataGetName(data)),
 			    "CryptAcquireContext",
 			    XMLSEC_ERRORS_R_CRYPTO_FAILED,
-			    "error code=%d", GetLastError());
+			    "GetLastError=%d", GetLastError());
 		return(-1);
 	    }
 	} else {
@@ -1727,20 +1916,19 @@
 			xmlSecErrorsSafeString(xmlSecKeyDataGetName(data)),
 			"CryptAcquireContext",
 			XMLSEC_ERRORS_R_CRYPTO_FAILED,
-			"error code=%d", GetLastError());
+			"GetLastError=%d", GetLastError());
 	    return(-1);
 	}
     }
 
     dwKeySpec = AT_SIGNATURE;
     dwSize = ((sizeBits << 16) | CRYPT_EXPORTABLE);
-
     if (!CryptGenKey(hProv, CALG_DSS_SIGN, dwSize, &hKey)) {
 	xmlSecError(XMLSEC_ERRORS_HERE,
 		    xmlSecErrorsSafeString(xmlSecKeyDataGetName(data)),
 		    "CryptGenKey",
 		    XMLSEC_ERRORS_R_CRYPTO_FAILED,
-		    "error code=%d", GetLastError());
+		    "GetLastError=%d", GetLastError());
 	goto done;
     }
 
@@ -1754,14 +1942,13 @@
 		    XMLSEC_ERRORS_NO_MESSAGE);
 	goto done;
     }
+    hProv = 0;
+    hKey = 0;
 
-    ret = 0;
+    /* success */
+    res = 0;
 
 done:
-    if (ret == 0) {
-	return (0);
-    }
-
     if (hProv != 0) {
 	CryptReleaseContext(ctx->hProv, 0);
     }
@@ -1770,7 +1957,7 @@
 	CryptDestroyKey(hKey);
     }
 
-    return(-1);
+    return(res);
 }
 
 static xmlSecKeyDataType


More information about the xmlsec mailing list