Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Derby Development" <derby-dev@db.apache.org>
Received-SPF: neutral (hermes.apache.org: local policy)
Message-ID: <41EF3DC3.6030407@Sourcery.Org>
Date: Wed, 19 Jan 2005 21:12:35 -0800
From: Kathey Marsden <kmarsden@Sourcery.Org>
User-Agent: Mozilla Thunderbird 0.7.3 (Windows/20040803)
MIME-Version: 1.0
To: Derby Development <derby-dev@db.apache.org>
Subject: [PATCH] Multiple EXCSAT fix
Content-Type: multipart/mixed;
 boundary="------------000805050307030902050304"

This is a multi-part message in MIME format.
--------------000805050307030902050304
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello All,

The attached patch is to address an issue  with the Network Server
regarding the handling of the EXCSAT codepoint.  Army told me that he
hit this with the C client tests that he is running.

There are three kinds of EXCSAT's we might get

1) Initial Exchange attributes.   For this we need to initialize the
apprequester. Session state is set to ATTEXC (attributes exchanged) then
the AR must follow up with ACCSEC and SECCHK to get the connection.

2) Send of EXCSAT as ping or manager level adjustment. For this we just
ignore the EXCSAT objects that are already set.  JCC doesn't use this,
but it is in DRDA.

3) Send of EXCSAT for connection reset. (see parseEXCSAT2()) This is
treated just like ping and will be followed up
by an ACCSEC request if in fact it is a connection reset.
If we have already exchanged attributes once just
process any new manager levels and return (case 2 and 3 above)
 	
Before the XA checkin, 1 and 2 worked.  After the XA checkin 1 and 3
worked and with this patch 1, 2, and 3 all work.

I had to rearrange the code a bit to do this because in DRDAConnThread,
processCommands() the EXCSAT case just called parseDRDAConnection which
expected the ACCSEC and SECCHK codepoints in the correct order.

As a solution I
1) Moved the handling of the ACCSEC and SECCHK codepoints from
parseDRDAConnection up to  processCommands().

2) Put the security state in the Session object.  The session has
   4 states.

two existed before:
INIT 	- initial state
ATTEXC  - attributes exchanged
	   (After first EXSAT - ACCSEC required)

now there are two new ones:

SECACC   - security accessed (After successful ACCSEC - need SECCHK)
CHKSEC   - checked security. After successful SECCHK and connection.
           no more required security codepoits.

The session.getRequiredSecurityCodepoint() method is used by
process_commands to determine if the next codepoint must be ACCSEC or
SECCHK.

Also the patch includes the fix Dan suggested for jdk 131 for
BrokeredPreparedStatent.getStatement(). But how to call
Connection.prepareStatement is still an issue.

	
Please let me know if you have any questions or issues.

Ran derbynetmats.  Changed some protocol tests that are still on the to
be contributed list.

I will check this in noon tomorrow if there are no objections if Army
gives the all clear with his tests.

Thanks

Kathey


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (MingW32)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFB7z3DG0h36bFmkocRAqcuAKCfyQhbXua3VMiqm3RIIAbFXyQyHQCfZp6M
YoDq/cAhR4sb9TcU9kYpU3g=
=3QZ5
-----END PGP SIGNATURE-----

--------------000805050307030902050304
Content-Type: text/plain;
 name="svndiff.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="svndiff.txt"

Index: java/engine/org/apache/derby/iapi/jdbc/BrokeredCallableStatement.java
===================================================================
--- java/engine/org/apache/derby/iapi/jdbc/BrokeredCallableStatement.java	(revision 125687)
+++ java/engine/org/apache/derby/iapi/jdbc/BrokeredCallableStatement.java	(working copy)
@@ -217,7 +217,7 @@
 	protected final CallableStatement getCallableStatement() throws SQLException {
 		return control.getRealCallableStatement();
 	}
-	protected final PreparedStatement getPreparedStatement() throws SQLException {
+	public final PreparedStatement getPreparedStatement() throws SQLException {
 		return getCallableStatement();
 	}
 	/**
Index: java/engine/org/apache/derby/iapi/jdbc/BrokeredPreparedStatement.java
===================================================================
--- java/engine/org/apache/derby/iapi/jdbc/BrokeredPreparedStatement.java	(revision 125687)
+++ java/engine/org/apache/derby/iapi/jdbc/BrokeredPreparedStatement.java	(working copy)
@@ -491,14 +491,14 @@
 	** Control methods.
 	*/
 
-	protected PreparedStatement getPreparedStatement() throws SQLException {
+	public PreparedStatement getPreparedStatement() throws SQLException {
 		return control.getRealPreparedStatement();
 	}
 
 	/**
 		Override the BrokeredStatement's getStatement() to always return a PreparedStatement.
 	*/
-	protected final Statement getStatement() throws SQLException {
+	public final Statement getStatement() throws SQLException {
 		return getPreparedStatement();
 	}
 
Index: java/engine/org/apache/derby/iapi/jdbc/BrokeredStatement.java
===================================================================
--- java/engine/org/apache/derby/iapi/jdbc/BrokeredStatement.java	(revision 125687)
+++ java/engine/org/apache/derby/iapi/jdbc/BrokeredStatement.java	(working copy)
@@ -510,7 +510,7 @@
 		newStatement.setQueryTimeout(oldStatement.getQueryTimeout());
 	}
 
-	protected Statement getStatement() throws SQLException {
+	public Statement getStatement() throws SQLException {
 		return control.getRealStatement();
 	}
 	protected final ResultSet wrapResultSet(ResultSet rs) {
Index: java/drda/org/apache/derby/impl/drda/DRDAStatement.java
===================================================================
--- java/drda/org/apache/derby/impl/drda/DRDAStatement.java	(revision 125687)
+++ java/drda/org/apache/derby/impl/drda/DRDAStatement.java	(working copy)
@@ -39,6 +39,7 @@
 import org.apache.derby.impl.jdbc.Util;
 import org.apache.derby.impl.jdbc.EmbedConnection;
 import  org.apache.derby.iapi.jdbc.BrokeredConnection;
+import  org.apache.derby.iapi.jdbc.BrokeredPreparedStatement;
 import org.apache.derby.impl.jdbc.EmbedResultSet;
 import org.apache.derby.impl.jdbc.EmbedParameterSetMetaData;
 import org.apache.derby.iapi.services.sanity.SanityManager;
@@ -276,7 +277,7 @@
 		try {
 			Method sh =
 				rsstmt.getClass().getMethod("getResultSetHoldability", getResultSetHoldabilityParam);
-			holdValue =  ((Integer) sh.invoke(ps,null)).intValue();
+			holdValue =  ((Integer) sh.invoke(rsstmt,null)).intValue();
 		}
 		catch (Exception e) {
 			handleReflectionException(e);
@@ -518,9 +519,13 @@
 	 *
 	 * @return prepared statement
 	 */
-	protected PreparedStatement getPreparedStatement() 
+	protected PreparedStatement getPreparedStatement() throws SQLException
 	{
-		return ps;
+		if (ps instanceof BrokeredPreparedStatement)
+			return (PreparedStatement)(
+						   ((BrokeredPreparedStatement) ps).getStatement());
+		else
+			return ps;
 	}
 
 
Index: java/drda/org/apache/derby/impl/drda/DRDAConnThread.java
===================================================================
--- java/drda/org/apache/derby/impl/drda/DRDAConnThread.java	(revision 125687)
+++ java/drda/org/apache/derby/impl/drda/DRDAConnThread.java	(working copy)
@@ -560,12 +560,10 @@
 		else
 		{
 			// exchange attributes with application requester
-			if (exchangeServerAttributes())
-				session.state = Session.ATTEXC;
-			else
-				closeSession();
+			exchangeServerAttributes();
 		}
 	}
+
 	/**
 	 * Process DRDA commands we can receive once server attributes have been
 	 * exchanged.
@@ -577,11 +575,16 @@
 		DRDAStatement stmt = null;
 		int updateCount = 0;
 		boolean PRPSQLSTTfailed = false;
+		boolean checkSecurityCodepoint = session.requiresSecurityCodepoint();
 		do
 		{
 			correlationID = reader.readDssHeader();
 			int codePoint = reader.readLengthAndCodePoint();
 			int writerMark = writer.markDSSClearPoint();
+			
+			if (checkSecurityCodepoint)
+				verifyInOrderACCSEC_SECCHK(codePoint,session.getRequiredSecurityCodepoint());
+
 			switch(codePoint)
 			{
 				case CodePoint.CNTQRY:
@@ -801,8 +804,20 @@
 					}
 					break;
 				case CodePoint.EXCSAT:
-					parseDRDAConnection();
+					parseEXCSAT();
+					writeEXCSATRD();
+					finalizeChain();
 					break;
+				case CodePoint.ACCSEC:
+					int securityCheckCode = parseACCSEC();
+					writeACCSECRD(securityCheckCode); 
+					checkSecurityCodepoint = true;
+					break;
+				case CodePoint.SECCHK:
+					if(parseDRDAConnection())
+						// security all checked and connection ok
+						checkSecurityCodepoint = false;
+					break;
 				/* since we don't support sqlj, we won't get bind commands from jcc, we
 				 * might get it from ccc; just skip them.
 				 */
@@ -916,11 +931,10 @@
 	/**
 	 * Exchange server attributes with application requester
 	 *
- 	 * @return true if the session was started successfully; false otherwise
-	 * @exception DRDAProtocolException, SQLException
+	 * @exception DRDAProtocolException
 	 */
-	private boolean exchangeServerAttributes()
-		throws  DRDAProtocolException, SQLException
+	private void exchangeServerAttributes()
+		throws  DRDAProtocolException
 	{
 		int codePoint;
 		correlationID = reader.readDssHeader();
@@ -944,10 +958,10 @@
 										  CodePoint.PRCCNVCD_EXCSAT_FIRST_AFTER_CONN);
 		}
 
-		// set up a new Application Requester to store information about the
-		// application requester for this session
-		
-		return parseDRDAConnection();
+		parseEXCSAT();
+		writeEXCSATRD();
+		finalizeChain();
+		session.setState(session.ATTEXC);
 	}
 	
 
@@ -956,49 +970,8 @@
 		int codePoint;
 		boolean sessionOK = true;
 
-		appRequester = new AppRequester();
-		parseEXCSAT();
-		writeEXCSATRD();
-		finalizeChain();
 
-		//we may have to do the access security more than once if we don't
-		//provide the requested security mechanism or we run into errors
-		//if we don't know the requested security mechanism
-		//we'll send our known security mechanisms and the requester will pick
-		//if he picks one that requires a security token then another ACCSEC 
-		//will flow
-		int securityCheckCode = 0;
-		boolean notdone = true;
-		while (notdone)
-		{
-			correlationID = reader.readDssHeader();
-			codePoint = reader.readLengthAndCodePoint();
-			verifyInOrderACCSEC_SECCHK(codePoint,CodePoint.ACCSEC);
-			securityCheckCode = parseACCSEC();
-			// need security token
-			if (securityCheckCode == 0  && 
-				database.securityMechanism == CodePoint.SECMEC_EUSRIDPWD &&
-				database.publicKeyIn == null)
-					securityCheckCode = CodePoint.SECCHKCD_SECTKNMISSING;
-
-			// shouldn't have security token
-			if (securityCheckCode == 0 &&
-				database.securityMechanism == CodePoint.SECMEC_USRIDPWD &&
-				database.publicKeyIn != null)
-					securityCheckCode = CodePoint.SECCHKCD_SECTKNMISSING;
-			if (SanityManager.DEBUG)
-				trace("** ACCSECRD securityCheckCode is: "+securityCheckCode);
-			writeACCSECRD(securityCheckCode);
-			// everything is O.K., we're done
-			if (securityCheckCode == 0) 
-			{
-				notdone = false;
-			}
-		}
-		correlationID = reader.readDssHeader();
-		codePoint = reader.readLengthAndCodePoint();
-		verifyInOrderACCSEC_SECCHK(codePoint,CodePoint.SECCHK);
-		securityCheckCode = parseSECCHK();
+		int securityCheckCode = parseSECCHK();
 		if (SanityManager.DEBUG)
 			trace("*** SECCHKRM securityCheckCode is: "+securityCheckCode);
 		writeSECCHKRM(securityCheckCode);
@@ -1231,7 +1204,32 @@
 		int codePoint;
 		String strVal;
 
+		// There are three kinds of EXCSAT's we might get.
+		// 1) Initial Exchange attributes.
+		//    For this we need to initialize the apprequester.
+		//    Session state is set to ATTEXC and then the AR must 
+		//    follow up with ACCSEC and SECCHK to get the connection.
+		//  2) Send of EXCSAT as ping or mangager level adjustment. 
+		//     (see parseEXCSAT2())
+		//     For this we just ignore the EXCSAT objects that
+		//     are already set.
+		//  3) Send of EXCSAT for connection reset. (see parseEXCSAT2())
+		//     This is treated just like ping and will be followed up 
+		//     by an ACCSEC request if in fact it is a connection reset.
 
+		// If we have already exchanged attributes once just 
+		// process any new manager levels and return (case 2 and 3 above)
+		if (appRequester != null)
+		{
+			parseEXCSAT2();
+			return;
+		}
+
+		// set up a new Application Requester to store information about the
+		// application requester for this session
+
+		appRequester = new AppRequester();
+
 		reader.markCollection();
 
 		codePoint = reader.getCodePoint();
@@ -1611,9 +1609,32 @@
 		database.securityMechanism = securityMechanism;
 		database.publicKeyIn = publicKeyIn;
 
+		// need security token
+		if (securityCheckCode == 0  && 
+			database.securityMechanism == CodePoint.SECMEC_EUSRIDPWD &&
+			database.publicKeyIn == null)
+			securityCheckCode = CodePoint.SECCHKCD_SECTKNMISSING;
+
+		// shouldn't have security token
+		if (securityCheckCode == 0 &&
+			database.securityMechanism == CodePoint.SECMEC_USRIDPWD &&
+			database.publicKeyIn != null)
+			securityCheckCode = CodePoint.SECCHKCD_SECTKNMISSING;
+		if (SanityManager.DEBUG)
+			trace("** ACCSECRD securityCheckCode is: "+securityCheckCode);
+		
+		// If the security check was successful set the session state to
+		// security accesseed.  Otherwise go back to attributes exchanged so we
+		// require another ACCSEC
+		if (securityCheckCode == 0)
+			session.setState(session.SECACC);
+		else
+			session.setState(session.ATTEXC);
+
 		return securityCheckCode;
 
 	}
+
 	/**
 	 * Parse OPNQRY
 	 * Instance Variables
@@ -2612,6 +2633,11 @@
 		{
 			securityCheckCode = verifyUserIdPassword();
 		}
+
+		// Security all checked 
+		if (securityCheckCode == 0)
+			session.setState(session.CHKSEC);
+		
 		return securityCheckCode;
 
 	}
Index: java/drda/org/apache/derby/impl/drda/Session.java
===================================================================
--- java/drda/org/apache/derby/impl/drda/Session.java	(revision 125687)
+++ java/drda/org/apache/derby/impl/drda/Session.java	(working copy)
@@ -36,10 +36,12 @@
 class Session
 {
 
-	// session states
+	// session states	   
 	protected static final int INIT = 1;	// before exchange of server attributes
-	protected static final int ATTEXC = 2;	// after exchange of server attributes
-	protected static final int CLOSED = 3;	// session has ended
+	protected static final int ATTEXC = 2;	// after first exchange of server attributes
+	protected static final int SECACC = 3;	// after ACCSEC (Security Manager Accessed)
+	protected static final int CHKSEC = 4;	// after SECCHK  (Checked Security)
+	protected static final int CLOSED = 5;	// session has ended
 
 	// session types
 	protected static final int DRDA_SESSION = 1;
@@ -196,8 +198,51 @@
 		return (Database)dbtable.get(dbName);
 	}
 
+	/**
+	 * Get requried security checkpoint.
+	 * Used to verify EXCSAT/ACCSEC/SECCHK order.
+	 *
+	 *  @return next required Security checkpoint or -1 if 
+	 *          neither ACCSEC or SECCHK are required at this time.
+	 *
+	 */
+	protected int getRequiredSecurityCodepoint()
+	{
+		switch (state)
+		{
+			case ATTEXC:
+				// On initial exchange of attributes we require ACCSEC 
+				// to access security manager
+				return CodePoint.ACCSEC;
+			case SECACC:
+				// After security manager has been accessed successfully we
+				// require SECCHK to check security
+				return CodePoint.SECCHK;
+			default:
+				return -1;
+		}	 
+	}
 
 	/**
+	 * Check if a security codepoint is required
+	 *
+	 * @return true if ACCSEC or SECCHK are required at this time.
+	 */
+	protected boolean requiresSecurityCodepoint()
+	{
+		return (getRequiredSecurityCodepoint() != -1);
+	}
+
+	/**
+	 * Set Session state
+	 * 
+	 */
+	protected void setState(int s)
+	{
+		state = s;
+	}
+	
+	/**
 	 * Get session into initial state
 	 *
 	 * @param traceDirectory	- directory for trace files


--------------000805050307030902050304--