RE: Draft-narayan-isms-sshsm-radius-01 review

Hi David,

Thanks for your comments. Please find my reply inline. 

-----Original Message-----
From: David Harrington [mailto:ietfdbhcomcast.net] 
Sent: Saturday, March 17, 2007 1:31 PM
To: ismsietf.org
Subject: [Isms] Draft-narayan-isms-sshsm-radius-01 review

Hi,

I have reviewed this document and have some general and some
specific
suggestions. Overall, I think the document is progressing,
but needs to
be more consistent with other isms documents, and could use
a
restructuring.

General:
I recommend restructuring the document based on what is to
be supported,
in an incremental manner:
1) "integration with legacy RADIUS" which would
only deal with
authenticating the user and authorizing an SSH subsystem,
and not deal
with integration with an ACM (or make it
implementation-dependent to
hang onto authz attributes received during a legacy authn).

2) "integration with Dynamic RADIUS" which would
support authorization
for a SSH subsystem PLUS authorize-only support in a
RADIUS-specific
ACM. 

I suggest that the RADIUS state attribute established during
authn be
passed as the securityName, since that is what the
authorize-only authz
is tied to. 

To maintain modularity, an SSH-RADIUS transport model should
pass the
RADIUS state attribute in the tmStateReference as the
proposed
securityName, and the Transport Security Model would
translate that into
the securityName. A RADIUS-specific ACM would then use the
securityName
(RADIUS state attr) to attempt an authorize-only
access-request, and use
the returned attributes to determine what access should be
allowed. If
the securityname != a known RADIUS-state-attr, then RADIUS
would return
an access-reject.

3) "integration with the proposed management policy
attributes". Used
with authorize-only, RADIUS could return the new
mgmt-policy-id
attribute to be used to create a mapping from securityname
(Radius-state-attr) to a "named" mgmt policy.

<Kaushik> 

The current draft suggests two models of integration for
access control

A. augment VACM to use securityName or other attributes
contained with
the RADIUS state attribute.
B. build new ACM for RADIUS integration that can use
securityName and
use RADIUS to acquire additional authorization information.


Are you suggesting that we donot pursue the first model.

</Kaushik>


---------Specific comments---------

Introduction
as noted previously, there is no SSH SM, only an SSH TM.
This should be
fixed throughout the document.

section2 s/(SSHSM) [sshsm]/[sshsm]/
	s/Security Model/Transport Model/
	"SSHSM requires ..." should be rewritten to
better reflect
current isms documents. I am willign to help rewrite this if
you want.
	s/explicitly//

<Kaushik> Sure, that would help. </Kaushik>


Section 3.1
I recommend moving the sentence starting "SSH
integration" before the
sentence starting "RADIUS will indicate"

Section 3.2
s/support multiple/supports multiple/
s/used SSH/used by SSH/
s/clients,/clients;/
The sentence starting "the 'password' method" is
not needed in this
document; it is irrelevant to SNMP integration.
The sentence starting "SSH server implementations
tyically" is not
needed.
s/rely of implementation/rely on implementation/

3.3
My mailer won't let me tell you that you have
"the" mispelled as "teh"

The paragraph starting "RADIUS Servers will
usually" could be condensed.
The sentence starting [radman] should start a new
paragragh.

3.3.1
s/vlaue/value/

<Kaushik> Will fix these. </Kaushik>

3.3.2

What happens if RADIUS has authorization attributes from
the
access-accept saved and passed in tmStateRef provided by the
transport
model, but the security model (USM/community/etc.) does not
use the
authorized user (securityName) from the tmStateRef?

<Kaushik>

I am not sure about this question. The assumption here is
that a
specific security model (such as sshsm) MUST use the
tmStateRef. Are you
asking what if a security model uses a different
securityName than the
one saved in the tmStateRef. As per RFC2865, the RADIUS
client will send
the RADIUS state attribute received in the Access Accept
unmodified in
the subsequent RADIUS request. The security model won't be
able to
change the state attribute which would be used by the RADIUS
server to
provide authorization information.

</Kaushik>


The sentence starting "The User-based security mdoel
integrates with
VACM" is incorrect; USM integrates with the RFC3411
architecture, which
integrates with VACM.

The descritpotrion of the use of a local database is not
consistent with
the RFC3411 descriotio of this usage.

This section talks about the proposed new mgmt-policy
attributes, but we
should try ot make th epoprosal not dependent on that
proposal. See my
general comments above.

s/comprises a policy or group name/comprises a policy, e.g.,
a group
name,/ s/mapping of the group name to/mapping of th epolicy
name to/
"unlikely to change" and "likely to be
based" are based on what
experience?
s/This mapping mechanisms is implementation specific/This
mapping
mechanisms is model- and implementation-specific/

3.3.3
s/, at the most simple level,//
s/RADIUS clients, with the NAS, initiates/A RADIUS client
within the NAS
initiates/

The sentence starting "In some cases" is not
needed.
The sentence starting "In many deployements"
should start a new
paragraph

3.3.4
TACACS needs a citation/reference
s/separate out/separate/

3.4
See general comments about securityname = RADIUS state attr

3.5
Sentence about accounting packets seems out of place; this
should be
dropped or expalined further.

<Kaushik> Will fix these </Kaushik>


3.6 discussion of PAM is not needed. That is purely
implementation-specific and we do not discuss implementation
issues.

<Kaushik> 

I think it is useful to mention PAM since that's prevelant
model of
integrating authentication mechanisms into SSH although it
might be
implementation specific.

</Kaushik>


"there is no equivalent shell" - is an SSH
subsystem comparable to a
shell?

"The ability of the SSH server to use  ..." is
this sentence needed?

s/such as PAM, provide/such as pAM, may provide/

"Transport Mapping Security Model" is no longer
valid.
"It is possible therefore," is inconsistent with
the architecure, since
the ACM does not get passed the tmStateRef.

"make calls to the RADIUS client" - would this be
clearer as "make calls
to the RADIUS server"?

5.
s/amy/may/

7.
[sshsm] is incorrect.

<Kaushik>

Will fix these.

Thanks,
  kaushik

</Kaushik>



David Harrington
dharringtonhuawei.com
dbharringtoncomcast.net
ietfdbhcomcast.net



_______________________________________________
Isms mailing list
Ismslists.ietf.org
https://w
ww1.ietf.org/mailman/listinfo/isms

_______________________________________________
Isms mailing list
Ismslists.ietf.org
https://w
ww1.ietf.org/mailman/listinfo/isms