You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fx-dev@ws.apache.org by Rami Jaamour <rj...@parasoft.com> on 2004/03/31 00:50:40 UTC
a small patch
Hello all,
I attached a small patch that checks for a NullPointerException in
WSSecurityEngine. It reproduces when you provide the wrong key store in
signature verification (there was a check in the past then it was
commented out so I added it back) -- might need a more proper fix here
but I wanted to at least bring it to your attention. I ran into it using
Parasoft's SOAPtest.
I made keystore in Merlin protected so I can extend it... I also made
some of it's methods throw specific Exceptions instead of just "Exception".
Best Regards,
--
Rami Jaamour
Software Engineer
SOAPtest <http://www.parasoft.com/jsp/products/home.jsp?product=SOAP>
Development
Parasoft Corporation <http://www.parasoft.com>
Re: a small patch
Posted by Werner Dittmann <We...@t-online.de>.
Rami,
that would mean that we have to spread the null checks in
all methods that need keystore - this was not the design of
Merlin (well, I didn't wrote it but AFAIK Merlin relies on keystore
not being null).
An idea would be that yor extension code overwrites the methods that need
keystore and check for null, and if not null call super(....), This way
there is no need to modify existing code and you extesnion takes care
of your specific needs.
Regards,
Werner
----- Original Message -----
From: Rami Jaamour
To: Werner Dittmann
Cc: fx-dev@ws.apache.org
Sent: Thursday, April 01, 2004 9:46 PM
Subject: Re: a small patch
Thank you for reviewing and integrating my patches. Yes, that is correct when you use Merlin directly, but I extend it and I have key stores passed directly to it instead of using load() for integration purposes, and in my project can be null when used in some signature verification scenarios (when no key stores are needed), so I needed the null check.
Rami Jaamour
Software Engineer
SOAPtest Development
Parasoft Corporation
Werner Dittmann wrote:
Ramis,
thanks for the fixes and the enhancements for Merlin.
Question: you inserted a check for keystore == null one
place. However, keystore can never be null, that is if the
initializaiton of keystore fails (see method load() in Merlin)
it always throws an exception. Thus if load() returns
sucessfully then keystore is ok, otherwise we have an
Exception thrown during instatiation of Merlin (load() is
called on the constructor).
Regards,
Werner
----- Original Message -----
From: Rami Jaamour
To: fx-dev@ws.apache.org
Sent: Wednesday, March 31, 2004 12:50 AM
Subject: a small patch
Hello all,
I attached a small patch that checks for a NullPointerException in WSSecurityEngine. It reproduces when you provide the wrong key store in signature verification (there was a check in the past then it was commented out so I added it back) -- might need a more proper fix here but I wanted to at least bring it to your attention. I ran into it using Parasoft's SOAPtest.
I made keystore in Merlin protected so I can extend it... I also made some of it's methods throw specific Exceptions instead of just "Exception".
Best Regards,
--
Rami Jaamour
Software Engineer
SOAPtest Development
Parasoft Corporation
--------------------------------------------------------------------------
Index: org/apache/ws/security/WSSecurityEngine.java
===================================================================
RCS file: /home/cvspublic/ws-fx/wss4j/src/org/apache/ws/security/WSSecurityEngine.java,v
retrieving revision 1.17
diff -u -r1.17 WSSecurityEngine.java
--- org/apache/ws/security/WSSecurityEngine.java 28 Mar 2004 17:48:39 -0000 1.17
+++ org/apache/ws/security/WSSecurityEngine.java 30 Mar 2004 22:24:10 -0000
@@ -486,7 +486,9 @@
if (tlog.isDebugEnabled()) {
t1 = System.currentTimeMillis();
}
- // if (certs != null && certs.length > 0 && certs[0] != null) {
+ if (certs == null || certs.length == 0 || certs[0] == null) {
+ throw new WSSecurityException(WSSecurityException.FAILED_CHECK);
+ }
try {
certs[0].checkValidity();
} catch (CertificateExpiredException e) {
@@ -518,8 +520,6 @@
} catch (XMLSignatureException e1) {
throw new WSSecurityException(WSSecurityException.FAILED_CHECK);
}
- // }
- // throw new WSSecurityException(WSSecurityException.FAILED_CHECK);
}
/**
Index: org/apache/ws/security/components/crypto/Merlin.java
===================================================================
RCS file: /home/cvspublic/ws-fx/wss4j/src/org/apache/ws/security/components/crypto/Merlin.java,v
retrieving revision 1.11
diff -u -r1.11 Merlin.java
--- org/apache/ws/security/components/crypto/Merlin.java 21 Mar 2004 14:40:40 -0000 1.11
+++ org/apache/ws/security/components/crypto/Merlin.java 30 Mar 2004 22:24:11 -0000
@@ -59,7 +59,7 @@
private static Log log = LogFactory.getLog(Merlin.class);
private static CertificateFactory certFact;
private Properties properties = null;
- private KeyStore keystore = null;
+ protected KeyStore keystore = null;
/**
* Constructor.
@@ -68,7 +68,7 @@
* @param properties
* @throws Exception
*/
- public Merlin(Properties properties) throws Exception {
+ public Merlin(Properties properties) throws CredentialException, IOException {
/*
* if no properties .. just return an instance, the rest will be
* done later or this instance is just used to handle certificate
@@ -284,6 +284,9 @@
Certificate cert = null;
try {
+ if (keystore == null) {
+ throw new WSSecurityException(WSSecurityException.FAILURE, "keystore");
+ }
for (Enumeration e = keystore.aliases(); e.hasMoreElements();) {
String alias = (String) e.nextElement();
Certificate[] certs = keystore.getCertificateChain(alias);
@@ -456,7 +459,7 @@
* @param input <code>InputStream</code> to read from
* @throws Exception
*/
- public void load(InputStream input) throws Exception {
+ public void load(InputStream input) throws CredentialException {
if (input == null) {
throw new IllegalArgumentException("input stream cannot be null");
}
Re: a small patch
Posted by Rami Jaamour <rj...@parasoft.com>.
Thank you for reviewing and integrating my patches. Yes, that is correct
when you use Merlin directly, but I extend it and I have key stores
passed directly to it instead of using load() for integration purposes,
and in my project can be null when used in some signature verification
scenarios (when no key stores are needed), so I needed the null check.
Rami Jaamour
Software Engineer
SOAPtest <http://www.parasoft.com/jsp/products/home.jsp?product=SOAP>
Development
Parasoft Corporation <http://www.parasoft.com>
Werner Dittmann wrote:
> Ramis,
>
> thanks for the fixes and the enhancements for Merlin.
> Question: you inserted a check for keystore == null one
> place. However, keystore can never be null, that is if the
> initializaiton of keystore fails (see method load() in Merlin)
> it always throws an exception. Thus if load() returns
> sucessfully then keystore is ok, otherwise we have an
> Exception thrown during instatiation of Merlin (load() is
> called on the constructor).
>
> Regards,
> Werner
>
>
> ----- Original Message -----
> *From:* Rami Jaamour <ma...@parasoft.com>
> *To:* fx-dev@ws.apache.org <ma...@ws.apache.org>
> *Sent:* Wednesday, March 31, 2004 12:50 AM
> *Subject:* a small patch
>
> Hello all,
>
> I attached a small patch that checks for a NullPointerException in
> WSSecurityEngine. It reproduces when you provide the wrong key
> store in signature verification (there was a check in the past
> then it was commented out so I added it back) -- might need a more
> proper fix here but I wanted to at least bring it to your
> attention. I ran into it using Parasoft's SOAPtest.
>
> I made keystore in Merlin protected so I can extend it... I also
> made some of it's methods throw specific Exceptions instead of
> just "Exception".
>
> Best Regards,
>
> --
> Rami Jaamour
> Software Engineer
> SOAPtest
> <http://www.parasoft.com/jsp/products/home.jsp?product=SOAP>
> Development
> Parasoft Corporation <http://www.parasoft.com>
>
> ------------------------------------------------------------------------
> Index: org/apache/ws/security/WSSecurityEngine.java
> ===================================================================
> RCS file:
> /home/cvspublic/ws-fx/wss4j/src/org/apache/ws/security/WSSecurityEngine.java,v
> retrieving revision 1.17
> diff -u -r1.17 WSSecurityEngine.java
> --- org/apache/ws/security/WSSecurityEngine.java 28 Mar 2004
> 17:48:39 -0000 1.17
> +++ org/apache/ws/security/WSSecurityEngine.java 30 Mar 2004
> 22:24:10 -0000
> @@ -486,7 +486,9 @@
> if (tlog.isDebugEnabled()) {
> t1 = System.currentTimeMillis();
> }
> - // if (certs != null && certs.length > 0 && certs[0] !=
> null) {
> + if (certs == null || certs.length == 0 || certs[0] == null) {
> + throw new
> WSSecurityException(WSSecurityException.FAILED_CHECK);
> + }
> try {
> certs[0].checkValidity();
> } catch (CertificateExpiredException e) {
> @@ -518,8 +520,6 @@
> } catch (XMLSignatureException e1) {
> throw new WSSecurityException(WSSecurityException.FAILED_CHECK);
> }
> - // }
> - // throw new WSSecurityException(WSSecurityException.FAILED_CHECK);
> }
>
> /**
> Index: org/apache/ws/security/components/crypto/Merlin.java
> ===================================================================
> RCS file:
> /home/cvspublic/ws-fx/wss4j/src/org/apache/ws/security/components/crypto/Merlin.java,v
> retrieving revision 1.11
> diff -u -r1.11 Merlin.java
> --- org/apache/ws/security/components/crypto/Merlin.java 21 Mar
> 2004 14:40:40 -0000 1.11
> +++ org/apache/ws/security/components/crypto/Merlin.java 30 Mar
> 2004 22:24:11 -0000
> @@ -59,7 +59,7 @@
> private static Log log = LogFactory.getLog(Merlin.class);
> private static CertificateFactory certFact;
> private Properties properties = null;
> - private KeyStore keystore = null;
> + protected KeyStore keystore = null;
>
> /**
> * Constructor.
> @@ -68,7 +68,7 @@
> * @param properties
> * @throws Exception
> */
> - public Merlin(Properties properties) throws Exception {
> + public Merlin(Properties properties) throws
> CredentialException, IOException {
> /*
> * if no properties .. just return an instance, the rest
> will be
> * done later or this instance is just used to handle
> certificate
> @@ -284,6 +284,9 @@
> Certificate cert = null;
>
> try {
> + if (keystore == null) {
> + throw new
> WSSecurityException(WSSecurityException.FAILURE, "keystore");
> + }
> for (Enumeration e = keystore.aliases(); e.hasMoreElements();) {
> String alias = (String) e.nextElement();
> Certificate[] certs = keystore.getCertificateChain(alias);
> @@ -456,7 +459,7 @@
> * @param input <code>InputStream</code> to read from
> * @throws Exception
> */
> - public void load(InputStream input) throws Exception {
> + public void load(InputStream input) throws CredentialException {
> if (input == null) {
> throw new IllegalArgumentException("input stream
> cannot be null");
> }
>
Re: a small patch
Posted by Werner Dittmann <We...@t-online.de>.
Ramis,
thanks for the fixes and the enhancements for Merlin.
Question: you inserted a check for keystore == null one
place. However, keystore can never be null, that is if the
initializaiton of keystore fails (see method load() in Merlin)
it always throws an exception. Thus if load() returns
sucessfully then keystore is ok, otherwise we have an
Exception thrown during instatiation of Merlin (load() is
called on the constructor).
Regards,
Werner
----- Original Message -----
From: Rami Jaamour
To: fx-dev@ws.apache.org
Sent: Wednesday, March 31, 2004 12:50 AM
Subject: a small patch
Hello all,
I attached a small patch that checks for a NullPointerException in WSSecurityEngine. It reproduces when you provide the wrong key store in signature verification (there was a check in the past then it was commented out so I added it back) -- might need a more proper fix here but I wanted to at least bring it to your attention. I ran into it using Parasoft's SOAPtest.
I made keystore in Merlin protected so I can extend it... I also made some of it's methods throw specific Exceptions instead of just "Exception".
Best Regards,
--
Rami Jaamour
Software Engineer
SOAPtest Development
Parasoft Corporation
------------------------------------------------------------------------------
Index: org/apache/ws/security/WSSecurityEngine.java
===================================================================
RCS file: /home/cvspublic/ws-fx/wss4j/src/org/apache/ws/security/WSSecurityEngine.java,v
retrieving revision 1.17
diff -u -r1.17 WSSecurityEngine.java
--- org/apache/ws/security/WSSecurityEngine.java 28 Mar 2004 17:48:39 -0000 1.17
+++ org/apache/ws/security/WSSecurityEngine.java 30 Mar 2004 22:24:10 -0000
@@ -486,7 +486,9 @@
if (tlog.isDebugEnabled()) {
t1 = System.currentTimeMillis();
}
- // if (certs != null && certs.length > 0 && certs[0] != null) {
+ if (certs == null || certs.length == 0 || certs[0] == null) {
+ throw new WSSecurityException(WSSecurityException.FAILED_CHECK);
+ }
try {
certs[0].checkValidity();
} catch (CertificateExpiredException e) {
@@ -518,8 +520,6 @@
} catch (XMLSignatureException e1) {
throw new WSSecurityException(WSSecurityException.FAILED_CHECK);
}
- // }
- // throw new WSSecurityException(WSSecurityException.FAILED_CHECK);
}
/**
Index: org/apache/ws/security/components/crypto/Merlin.java
===================================================================
RCS file: /home/cvspublic/ws-fx/wss4j/src/org/apache/ws/security/components/crypto/Merlin.java,v
retrieving revision 1.11
diff -u -r1.11 Merlin.java
--- org/apache/ws/security/components/crypto/Merlin.java 21 Mar 2004 14:40:40 -0000 1.11
+++ org/apache/ws/security/components/crypto/Merlin.java 30 Mar 2004 22:24:11 -0000
@@ -59,7 +59,7 @@
private static Log log = LogFactory.getLog(Merlin.class);
private static CertificateFactory certFact;
private Properties properties = null;
- private KeyStore keystore = null;
+ protected KeyStore keystore = null;
/**
* Constructor.
@@ -68,7 +68,7 @@
* @param properties
* @throws Exception
*/
- public Merlin(Properties properties) throws Exception {
+ public Merlin(Properties properties) throws CredentialException, IOException {
/*
* if no properties .. just return an instance, the rest will be
* done later or this instance is just used to handle certificate
@@ -284,6 +284,9 @@
Certificate cert = null;
try {
+ if (keystore == null) {
+ throw new WSSecurityException(WSSecurityException.FAILURE, "keystore");
+ }
for (Enumeration e = keystore.aliases(); e.hasMoreElements();) {
String alias = (String) e.nextElement();
Certificate[] certs = keystore.getCertificateChain(alias);
@@ -456,7 +459,7 @@
* @param input <code>InputStream</code> to read from
* @throws Exception
*/
- public void load(InputStream input) throws Exception {
+ public void load(InputStream input) throws CredentialException {
if (input == null) {
throw new IllegalArgumentException("input stream cannot be null");
}