Subject: [Isms] Draft-narayan-isms-sshsm-radius-01 review

From: "David Harrington" <[email-masked]>
Date: Sat, 17 Mar 2007 16:30:51 -0400

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.

---------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//

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/

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?

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.

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

"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.

David Harrington
[email-masked] 
[email-masked]
[email-masked]

_______________________________________________
Isms mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/isms

---

From: "Kaushik Narayan \(kaushik\)" <[email-masked]>
Date: Mon, 19 Mar 2007 12:38:47 -0700

Hi David,

Thanks for your comments. Please find my reply inline. 

-----Original Message-----
From: David Harrington [mailto:[email-masked]] 
Sent: Saturday, March 17, 2007 1:31 PM
To: [email-masked]
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).

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.

<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.

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

Section 3.1

Section 3.2

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

<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.

name,/ s/mapping of the group name to/mapping of th epolicy name to/

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

3.3.4

3.4

3.5

<Kaushik> Will fix these </Kaushik>

<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>

"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
[email-masked]
[email-masked]
[email-masked]