[Nbd] Get Instant e Approval on Car Loan

[Yes, Its True]

If you are dreaming for a New Car?

We are here to to take care of it

Get Instant e-Approval from Top Banks:

 Apply Now 


--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

[Nbd] Get your Dream Home Now !

[Dream Home]

Yes!! It’s True, Now Get Home Loan

e-Approval Instantly

*Minimum Paper Work Required

Apply Online Now 


--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

[Nbd] [PATCH v2] doc: Add new NBD_REP_INFO reply, for advertising block size

[Eric Blake]

Existing NBD servers often have limitations, such as requiring
actions to be aligned to block sizes or limiting maximum
transactions to avoid denial of service attacks; for example,
qemu's NBD server refuses any transaction larger than 32M.  But
to date, clients have to learn these limitations via out-of-band
means.

This alters NBD_OPT_INFO and NBD_OPT_GO to use a new reply type
NBD_REP_INFO (rather than overloading NBD_REP_SERVER), so that
we have a future-proof way of supplying as much additional
structured information about an export as we want.

Design decision: a client that wants to learn block sizes MUST
use NBD_OPT_GO, rather than the old NBD_OPT_EXPORT_NAME, even
though we could have repurposed some of the reserved zeroes when
NBD_FLAG_C_NO_ZEROES is not in effect, because we don't want to
encourage any further abuse of NBD_OPT_EXPORT_NAME.

Design decision: no new global NBD_FLAG or NBD_OPT are required;
there is nothing for the client to negotiate.  The server merely
provides as much information as it can, and the client then
interprets what information it understands.  The items are
structured so that a client can ignore details from the server
that the client does not know about, and that we can easily add
future items of information.

Signed-off-by: Eric Blake 
---

v2: add new NBD_REP_INFO, ditch NBD_FLAG_BLOCK_SIZE, create new
subsection "Block sizes" rather than repeating information, more
details on sizing constraints

 doc/proto.md | 213 ++-
 1 file changed, 167 insertions(+), 46 deletions(-)

diff --git a/doc/proto.md b/doc/proto.md
index cd3a4cd..4604ae6 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -590,6 +590,67 @@ This functionality has not yet been implemented by the 
reference
 implementation, but was implemented by qemu and subsequently
 by other users, so has been moved out of the "experimental" section.

+## Block sizes
+
+During transmission phase, several operations are constrained by three
+block sizes: minimum, preferred, and maximum; as well as the export
+size sent in reply to `NBD_OPT_EXPORT_NAME` or `NBD_OPT_GO`.  A server
+MAY advertise the three block size contraints during handshake phase
+via the experimental `INFO` extension; see below.  However, not all
+servers advertise block size information; if such information is not
+also made available via out of band means, clients SHOULD assume some
+default sizes for the maximum chance of successful interaction, as
+described herein.
+
+The minimum block size represents the smallest addressable length and
+alignment within the export, although writing to an area that small
+may require the server to use a less-efficient read-modify-write
+action.  If advertised, this value MUST be a power of 2, MUST NOT be
+larger than 65,536, and MAY be as small as 1 for an export backed by a
+regular file, although the values of 512 or 4,096 are more typical for
+an export backed by a block device.  If a server advertises a minimum
+block size, the advertised export size MUST be an integer multiple of
+that block size.  If the server does not advertise a minimum block
+size, the client MAY infer a minimum block size as the smaller of 512
+bytes or the first power of 2 that evenly divides the export size.
+
+The preferred block size represents the minimum size at which aligned
+requests will have efficient I/O, avoiding behaviour such as
+read-modify-write.  If advertised, this MUST be a power of 2 at least
+as large as the minimum block size, and MUST NOT be larger than
+3,355,442 (32M).  The preferred block size MAY be larger than the
+export size, in which case the client is unable to utilize the
+preferred block size for that export.  If the server does not
+advertise a preferred block size, the client MAY assume that a
+preferred block size of 65,536 bytes will have reasonable performance.
+
+The maximum block size represents the maximum length that the server
+is willing to handle in one request.  If advertised, it MUST be either
+an integer multiple of the minimum block size or the value
+4,294,967,295 for no inherent limit, MUST be at least as large as the
+preferred block size, and SHOULD be at least 3,355,442 (32M), but MAY
+be something other than a power of 2.  For convenience, the server MAY
+advertise a maximum block size that is larger than the export size,
+although in that case, the client MUST treat the export size as the
+effectual maximum block size.  If the server does not advertise a
+maximum block size, the client SHOULD NOT send requests with a length
+larger than 3,355,442 (32M), to minimize the chance of the server
+rejecting the request as too large.
+
+Where a transmission request can have a nonzero *offset* and/or
+*length* (such as `NBD_CMD_READ`, `NBD_CMD_WRITE`, or `NBD_CMD_TRIM`),
+the client MUST ensure that *offset* and *length* are integer
+multiples of any advertised minimum block size, and SHOULD use integer
+multiples of 

Re: [Nbd] [PATCH] Docs: improve description of disconnection methods

[Eric Blake]

On 04/12/2016 01:31 PM, Alex Bligh wrote:
> Improve the documentation as per the mailing list discussion.
> Here's what we deciced (broadly).

s/deciced/decided/

> 
> * One side MAY drop the connection if the other end violates a
>  MUST condition.
> 
> * The server MUST drop the connection in the 'no way out' situations
>  during the negotiation phase (error on NBD_OPT_EXPORT_NAME, error
>  in negotiating text).

negotiating text, or negotiating TLS?

> 
> * The server SHOULD NOT otherwise drop the connection. It can wait
>  and error the next command. Clearly there are situations where
>  this is going to happen (e.g. server shutdown).
> 
> * If the server does need to drop the connection, it SHOULD wait
>  until there are no commands in-flight in transmission mode,
>  it possible.
> 
> * If he client is going to drop the the connection, then other

s/he/the/
s/the the/the/

>  than in the event of a protocol violation or a 'no way out'
>  situation (e.g. TLS negotiation fails), it MUST use NBD_CMD_DISC
>  or NBD_OPT_ABORT
> 
> * We should tidy up the semantics and descriptions of NBD_CMD_DISC
>  and NBD_OPT_ABORT, viz replies or not to the latter, shutting
>  down TLS properly etc.
> 
> Other changes:
> 
> * Added a reply to NBD_OPT_ABORT. No harm if the client closes
>   the connection anyway.
> 
> * Said the offset and length fields in NBD_CMD_DISC MUST be zero.
>   Not doing so is a protocol violation and would only lead to ...
>   the connection being closed, so this is a useful tidy up.
> 
> * Introduced consistent terminology for disconnection throughout.
> 
> This patch applies on top of:
>   0001-docs-proto.md-Clarify-SHOULD-MUST-MAY-etc v7
> 
> Signed-off-by: Alex Bligh 
> ---
>  doc/proto.md | 143 
> ---
>  1 file changed, 107 insertions(+), 36 deletions(-)
> 

> + Termination of the session during option haggling
> +
> +There are three possible mechanisms to end option haggling:
> +
> +* Transmission mode can be entered (by the client sending
> +  `NBD_OPT_EXPORT_NAME` or by the server responding to an
> +  `NBD_OPT_GO` with `NBD_REP_ACK`). This is documented

s/ACK/SERVER/

(although I may bite the bullet and create a new NBD_REP_INFO if we want
the name to be optional, since NBD_OPT_[INFO/GO] is still experimental,
as part of my rework on block size information)

> +  elsewhere.
> +
> +* The client can send (and the server can reply to) an
> +  `NBD_OPT_ABORT`. This MUST be followed by the client
> +  shutting down TLS (if it is running), and the client
> +  dropping the connection. This is referred to as
> +  'initiating a soft disconnect'; soft disconnects can
> +  only be initiated by the client.
> +
> +* The client or the server can disconnect the TCP session
> +  without activity at the NBD protocol level. If TLS is
> +  negotiated, the party intitiating the transaction SHOULD

s/intitiating/initiating

> +  shutdown TLS first if it is running. This is referrred

s/referrred/referred/

> +  to as 'initiating a hard disconnect'.
> +
> +This section concerns the second and third of these, together
> +called 'terminating the session', and under which circumstances
> +they are valid.
> +
> +If either the client or the server detects a violation of a
> +mandatory condition ('MUST' etc.) by the other party, it MAY
> +initiate a hard discconect.

s/discconect/disconnect/

> +
> +A client MAY use a soft disconnect to terminate the session
> +whenever it wishes, provided that there are no outstanding
> +replies to options.

Why the disclaimer on no outstanding replies?

> +
> +A party that is mandated by this document to terminate the
> +session MUST initiate a hard disconnect if it is not possible
> +to use a soft disconnect. Such circumstances include: where
> +that party is the server and it cannot return an error
> +(e.g. after an `NBD_OPT_EXPORT_NAME` it cannot satisfy),
> +and where that party is the client following a failed TLS
> +negotiation.
> +
> +A party MUST NOT initiate a hard disconnect save where set out
> +in this section. Therefore, unless a client's situation falls
> +within the provisions of the previous paragraph, it MUST NOT
> +use a hard disconnect, and hence its only option to terminate
> +the session is via a soft disconnect.
> +
>  There is no requirement for the client or server to complete a
>  negotiation if it does not wish to do so. Either end MAY simply
> -close the TCP connection (though see below regarding prior use
> -of NBD_OPT_ABORT). Under certain circumstances either
> -the client or the server may be required by this document to close
> -the TCP connection. In each case, this is referred to as 'terminate
> -the session'.
> -
> -If the client wishes to terminate the session in the negotiation
> -phase, and is not doing so because it is required to do so
> -by this document, it SHOULD send NBD_OPT_ABORT first if the protocol
> -permits. There are instances where this is 

[Nbd] [PATCH] Docs: improve description of disconnection methods

[Alex Bligh]

Improve the documentation as per the mailing list discussion.
Here's what we deciced (broadly).

* One side MAY drop the connection if the other end violates a
 MUST condition.

* The server MUST drop the connection in the 'no way out' situations
 during the negotiation phase (error on NBD_OPT_EXPORT_NAME, error
 in negotiating text).

* The server SHOULD NOT otherwise drop the connection. It can wait
 and error the next command. Clearly there are situations where
 this is going to happen (e.g. server shutdown).

* If the server does need to drop the connection, it SHOULD wait
 until there are no commands in-flight in transmission mode,
 it possible.

* If he client is going to drop the the connection, then other
 than in the event of a protocol violation or a 'no way out'
 situation (e.g. TLS negotiation fails), it MUST use NBD_CMD_DISC
 or NBD_OPT_ABORT

* We should tidy up the semantics and descriptions of NBD_CMD_DISC
 and NBD_OPT_ABORT, viz replies or not to the latter, shutting
 down TLS properly etc.

Other changes:

* Added a reply to NBD_OPT_ABORT. No harm if the client closes
  the connection anyway.

* Said the offset and length fields in NBD_CMD_DISC MUST be zero.
  Not doing so is a protocol violation and would only lead to ...
  the connection being closed, so this is a useful tidy up.

* Introduced consistent terminology for disconnection throughout.

This patch applies on top of:
  0001-docs-proto.md-Clarify-SHOULD-MUST-MAY-etc v7

Signed-off-by: Alex Bligh 
---
 doc/proto.md | 143 ---
 1 file changed, 107 insertions(+), 36 deletions(-)

diff --git a/doc/proto.md b/doc/proto.md
index b88e054..db6b96d 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -122,7 +122,7 @@ C: 32 bits, flags
 This completes the initial phase of negotiation; the client and server
 now both know they understand the first version of the newstyle
 handshake, with no options. The client SHOULD ignore any handshake flags
-it does not recognize, while the server MUST close the connection if
+it does not recognize, while the server MUST close the TCP connection if
 it does not recognize the client's flags.  What follows is a repeating
 group of options. In non-fixed newstyle only one option can be set
 (`NBD_OPT_EXPORT_NAME`), and it is not optional.
@@ -150,8 +150,8 @@ S: 16 bits, transmission flags
 S: 124 bytes, zeroes (reserved) (unless `NBD_FLAG_C_NO_ZEROES` was
negotiated by the client)  
 
-If the server is unwilling to allow the export, it SHOULD close the
-connection.
+If the server is unwilling to allow the export, it MUST terminate
+the session.
 
 The reason that the flags field is 16 bits large and not 32 as in the
 oldstyle negotiation is that there are now 16 bits of transmission flags,
@@ -201,22 +201,60 @@ request before sending the next one of the same type. The 
server MAY
 send replies in the order that the requests were received, but is not
 required to.
 
+ Termination of the session during option haggling
+
+There are three possible mechanisms to end option haggling:
+
+* Transmission mode can be entered (by the client sending
+  `NBD_OPT_EXPORT_NAME` or by the server responding to an
+  `NBD_OPT_GO` with `NBD_REP_ACK`). This is documented
+  elsewhere.
+
+* The client can send (and the server can reply to) an
+  `NBD_OPT_ABORT`. This MUST be followed by the client
+  shutting down TLS (if it is running), and the client
+  dropping the connection. This is referred to as
+  'initiating a soft disconnect'; soft disconnects can
+  only be initiated by the client.
+
+* The client or the server can disconnect the TCP session
+  without activity at the NBD protocol level. If TLS is
+  negotiated, the party intitiating the transaction SHOULD
+  shutdown TLS first if it is running. This is referrred
+  to as 'initiating a hard disconnect'.
+
+This section concerns the second and third of these, together
+called 'terminating the session', and under which circumstances
+they are valid.
+
+If either the client or the server detects a violation of a
+mandatory condition ('MUST' etc.) by the other party, it MAY
+initiate a hard discconect.
+
+A client MAY use a soft disconnect to terminate the session
+whenever it wishes, provided that there are no outstanding
+replies to options.
+
+A party that is mandated by this document to terminate the
+session MUST initiate a hard disconnect if it is not possible
+to use a soft disconnect. Such circumstances include: where
+that party is the server and it cannot return an error
+(e.g. after an `NBD_OPT_EXPORT_NAME` it cannot satisfy),
+and where that party is the client following a failed TLS
+negotiation.
+
+A party MUST NOT initiate a hard disconnect save where set out
+in this section. Therefore, unless a client's situation falls
+within the provisions of the previous paragraph, it MUST NOT
+use a hard disconnect, and hence its only option to terminate
+the session is via a soft disconnect.

[Nbd] [PATCHv7] docs/proto.md: Clarify SHOULD / MUST / MAY etc

[Alex Bligh]

These are changes which possibly have semantic effect

* Clarify that SHOULD / MUST / MAY etc. when in capitals have an
  RFC 2119 meaning using the wording within that RFC.

* Fix some lowercase use of these words which actually were
  meant to be uppercase.

* Fix some lowercase 'should' which clearly meant 'MUST'; where
  it's not obvious, I've made them 'SHOULD' or left them as is.

* Fix wording on transmission flags to be clearer.

Signed-off-by: Alex Bligh 
---
 doc/proto.md | 100 +++
 1 file changed, 59 insertions(+), 41 deletions(-)

Changes since v6:

* Rebased on master

Changes since v5:

* Rebased on top of "Improve documentation for TLS"

* Eric's nit re NBD_OPT_INFO fixed in passing

Changes since v4:

* Uses 'exposes' terminology instead of repetitive text on
NBD_CMD_FLAGS

* Change backwards compatibility references from MUST to SHOULD

Changes since v3:

* Consensus is that bolding is permanently dead

* Fixed the transmission flags section by pretty much rewriting it.

* Fixed provisions for back-compatibility. They are now all a MUST.
The client can always disconnect if it doesn't want to be
back compatible.

* Various other nits from Eric & Wouter

Changes since v2:

* Remove bolding for now (may do separately)

* Remove NUL => `NUL` for now (will do separately) so easier to see
possibly semantic changes

Changes since v1:

* Added an http link to RFC2119 per Eric Blake

* Corrected a couple more MUST entries

* Put NUL in monospace

diff --git a/doc/proto.md b/doc/proto.md
index d12240d..b88e054 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -25,6 +25,12 @@ comments) constant names, `0xdeadbeef` is used for literal 
hex numbers
 (which are always sent in network byte order), and (brackets) are used
 for comments. Anything else is a description of the data that is sent.
 
+The key words "MUST", "MUST NOT", "REQUIRED", "SHALL",
+"SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED",
+"MAY", and "OPTIONAL" in this document are to be interpreted as
+described in [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt).
+The same words in lower case carry their natural meaning.
+
 Where this document refers to a string, then unless otherwise stated,
 that string is a sequence of UTF-8 code points, which is not `NUL`
 terminated, MUST NOT contain `NUL` characters, SHOULD be no longer than
@@ -101,7 +107,7 @@ the IANA-reserved port for NBD, 10809. The server MAY 
listen on other
 ports as well, but it SHOULD use the old style handshake on those. The
 server SHOULD refuse to allow oldstyle negotiations on the newstyle
 port. For debugging purposes, the server MAY change the port on which to
-listen for newstyle negotiation, but this should not happen for
+listen for newstyle negotiation, but this SHOULD NOT happen for
 production purposes.
 
 The initial few exchanges in newstyle negotiation look as follows:
@@ -144,7 +150,7 @@ S: 16 bits, transmission flags
 S: 124 bytes, zeroes (reserved) (unless `NBD_FLAG_C_NO_ZEROES` was
negotiated by the client)  
 
-If the server is unwilling to allow the export, it should close the
+If the server is unwilling to allow the export, it SHOULD close the
 connection.
 
 The reason that the flags field is 16 bits large and not 32 as in the
@@ -169,9 +175,9 @@ To fix these two issues, the following changes were 
implemented:
 
 - The server will set the handshake flag `NBD_FLAG_FIXED_NEWSTYLE`, to
   signal that it supports fixed newstyle negotiation.
-- The client should reply with `NBD_FLAG_C_FIXED_NEWSTYLE` set in its flags
+- The client SHOULD reply with `NBD_FLAG_C_FIXED_NEWSTYLE` set in its flags
   field too, though its side of the protocol does not change incompatibly.
-- The client may now send other options to the server as appropriate, in
+- The client MAY now send other options to the server as appropriate, in
   the generic format for sending an option as described above.
 - The server MUST NOT send a response to `NBD_OPT_EXPORT_NAME` until all
   other pending option requests have had their final reply.
@@ -183,7 +189,7 @@ S: 32 bits, the option as sent by the client to which this 
is a reply
 S: 32 bits, reply type (e.g., `NBD_REP_ACK` for successful completion,
or `NBD_REP_ERR_UNSUP` to mark use of an option not known by this
server  
-S: 32 bits, length of the reply. This may be zero for some replies, in
+S: 32 bits, length of the reply. This MAY be zero for some replies, in
which case the next field is not sent  
 S: any data as required by the reply (e.g., an export name in the case
of `NBD_REP_SERVER`)  
@@ -601,7 +607,7 @@ order.
 This field of 16 bits is sent by the server after the `INIT_PASSWD` and
 the first magic number.
 
-- bit 0, `NBD_FLAG_FIXED_NEWSTYLE`; should be set by servers that
+- bit 0, `NBD_FLAG_FIXED_NEWSTYLE`; MUST be set by servers that
   support the fixed newstyle protocol
 - bit 1, `NBD_FLAG_NO_ZEROES`; if set, and if the client 

[Nbd] [PATCH] Two nits on STARTTLS documentation

[Alex Bligh]

These two nits from Eric Blake got missed as Wouter merged v9
when I was doing v10.

Signed-off-by: Alex Bligh 
---
 doc/proto.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/proto.md b/doc/proto.md
index 255fd11..05fef3c 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -445,7 +445,7 @@ exports are TLS-only. This is permitted in part to make 
programming
 of servers easier. Operation is a little different from FORCEDTLS,
 as the client is not forced to upgrade to TLS prior to any options
 being processed, and the server MAY choose to give information on
-non-existent exports via NBD_OPT_INFO exports prior to an upgrade
+non-existent exports via NBD_OPT_INFO responses prior to an upgrade
 to TLS.
 
 The SELECTIVETLS mode of operation has an implementation problem
@@ -460,7 +460,7 @@ must be initiated between client and server out of band.
 Therefore, if a server uses SELECTIVETLS, it MUST implement
 the INFO extension.
 
-## Client-side requirements
+### Client-side requirements
 
 If the client supports TLS at all, it MUST be prepared
 to deal with servers operating in any of the above modes.
-- 
1.9.1


--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

[Nbd] [PATCHv10] Improve documentation for TLS

[Alex Bligh]

* Call out TLS into a separate section

* Add details of the TLS protocol itself

* Emphasise that actual TLS session initiation (i.e. the TLS handshake) can
  be initiated from either side (as required by the TLS standard I believe
  and as actually works in practice)

* Clarify what is a requirement on servers, and what is a requirement on
  clients, separately, specifying their behaviour in a single place
  in the document.

* Document the three possible modes of operation of a server.

* Add text defining what 'terminate the session' means during
  negotiation, and when it is available.

Signed-off-by: Alex Bligh 
---
 doc/proto.md | 352 +--
 1 file changed, 318 insertions(+), 34 deletions(-)

Changes since v9

* Two further nits from Eric Blake.

Changes since v8:

* Reword section on disconnection, as per mail to list save with Eric Blake's
 change of 're' to 'regarding'.

Changes since v7

* I missed committing the changes re consistent use of 'option' rather than 
'command'
in v7. They are here now.

Changes from v6:

* Introduced language mandating a server to reply with NBD_ERR_INVALID
to NBD_OPT_STARTTLS if TLS is already negotiatied.

* Removed some duplication in SELECTIVETLS over the prohibition on
servers not returning NBD_ERR_TLSREQD to options other than
NBD_OPT_EXPORTNAME, NBD_OPT_INFO and NBD_OPT_GO. The same thing
was said a different way a couple of paragraphs below.

* Consistently refer to 'options' rather than 'commands' in the
negotiation phase.

* Eric Blake's nits

Changes from v5:

* Delete OPTIONALTLS (RIP)

* Add NBD_REP_ERR_POLICY

* s/NBD_ERR_REP/NBD_REP_ERR/ in one place

* Consistently use the phrase 'terminate the session' to mean dropping
the connection, as per Wouter. Note there are other inconsistent
uses of 'dropping the connection', 'disconnecting' etc. elsewhere
which I haven't touched.

* Similarly refer to the connection as a 'session' when it doesn't
explicitly mean the L3 TCP connection (TLS section only).

* Introduce a paragraph under newstyle negotiation emphasising that
terminating the session is legal and sometimes required, and defining
it.

Changes from v4

* Minor grammar nit

Changes from v3:

* Delete confusing text about server omitting entries from NBD_OPT_LIST
if TLS is not negotiated and FORCETLS is used, as that (of course)
requires NBD_REP_ERR_TLS_REQD elsewhere in the text.

* Further nits from Eric Blake

Changes from v2:

* The response to a command is a response, not a NBD_REP_ACK

* Make it clear that the response can be errored

* Nits from Eric Blake

Changes from v1:

* Make a NBD_CMD_CLOSE imply a flush

* Nits from Eric Blake

diff --git a/doc/proto.md b/doc/proto.md
index f117394..05fef3c 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -195,6 +195,23 @@ request before sending the next one of the same type. The 
server MAY
 send replies in the order that the requests were received, but is not
 required to.
 
+There is no requirement for the client or server to complete a
+negotiation if it does not wish to do so. Either end may simply
+close the TCP connection (though see below regarding prior use
+of NBD_OPT_ABORT). Under certain circumstances either
+the client or the server may be required by this document to close
+the TCP connection. In each case, this is referred to as 'terminate
+the session'.
+
+If the client wishes to terminate the session in the negotiation
+phase, and is not doing so because it is required to do so
+by this document, it SHOULD send NBD_OPT_ABORT first if the protocol
+permits. There are instances where this is impossible, such as after
+an NBD_OPT_EXPORTNAME has been issued, or on an unsuccessful
+negotiation of TLS.  For instance, if the client does not find an
+export it is looking for, it may simply send an NBD_OPT_ABORT
+and close the TCP connection.
+
 ### Transmission
 
 There are three message types in the transmission phase: the request,
@@ -286,6 +303,287 @@ S: (*length* bytes of data if the request is of type 
`NBD_CMD_READ`)
 This reply type MUST NOT be used except as documented by the
 experimental `STRUCTURED_REPLY` extension; see below.
 
+## TLS support
+
+The NBD protocol supports Transport Layer Security (TLS) (see
+[RFC5246](https://tools.ietf.org/html/rfc5246)
+as updated by
+[RFC6176](https://tools.ietf.org/html/rfc6176)
+).
+
+TLS is negotiated with the `NBD_OPT_STARTTLS`
+option. This is performed as an in-session upgrade. Below the term
+'negotiation' is used to refer to the sending and receiving of
+NBD options and option replies, and the term 'initiation' of TLS
+is used to refer to the actual upgrade to TLS.
+
+### Certificates, authentication and authorisation
+
+This standard does not specify what encryption, certification
+and signature algorithms are used. This standard does not
+specify authentication and authorisation (for instance
+whether client and/or server certificates are required and
+what they 

[Nbd] [PATCHv4 6/6] Add TLS support to NBD client

[Alex Bligh]

Signed-off-by: Alex Bligh 
---
 nbd-client.c | 119 +--
 1 file changed, 116 insertions(+), 3 deletions(-)

diff --git a/nbd-client.c b/nbd-client.c
index ff79a27..74ba7d9 100644
--- a/nbd-client.c
+++ b/nbd-client.c
@@ -47,6 +47,10 @@
 #define MY_NAME "nbd_client"
 #include "cliserv.h"
 
+#ifdef WITH_GNUTLS
+#include "crypto-gnutls.h"
+#endif
+
 #ifdef WITH_SDP
 #include 
 #endif
@@ -266,13 +270,14 @@ void ask_list(int sock) {
err("Failed writing length");
 }
 
-void negotiate(int sock, u64 *rsize64, uint16_t *flags, char* name, uint32_t 
needed_flags, uint32_t client_flags, uint32_t do_opts) {
+void negotiate(int *sockp, u64 *rsize64, uint16_t *flags, char* name, uint32_t 
needed_flags, uint32_t client_flags, uint32_t do_opts, char *certfile, char 
*keyfile, char *cacertfile, char *tlshostname) {
u64 magic, size64;
uint16_t tmp;
uint16_t global_flags;
char buf[256] = "\0\0\0\0\0\0\0\0\0";
uint32_t opt;
uint32_t namesize;
+   int sock = *sockp;
 
printf("Negotiation: ");
readit(sock, buf, 8);
@@ -304,6 +309,108 @@ void negotiate(int sock, u64 *rsize64, uint16_t *flags, 
char* name, uint32_t nee
if (write(sock, _flags, sizeof(client_flags)) < 0)
err("Failed/2.1: %m");
 
+#ifdef WITH_GNUTLS
+/* TLS */
+if (keyfile) {
+   int plainfd[2]; // [0] is used by the proxy, [1] is used by NBD
+   tlssession_t *s = NULL;
+   int ret;
+   uint32_t tmp32;
+   uint64_t tmp64;
+
+   /* magic */
+   tmp64 = htonll(opts_magic);
+   if (write(sock, , sizeof(tmp64)) < 0)
+   err( "Could not write magic: %m");
+   /* starttls */
+   tmp32 = htonl(NBD_OPT_STARTTLS);
+   if (write(sock, , sizeof(tmp32)) < 0)
+   err("Could not write option: %m");
+   /* length of data */
+   tmp32 = htonl(0);
+   if (write(sock, , sizeof(tmp32)) < 0)
+   err("Could not write option length: %m");
+
+   if (read(sock, , sizeof(tmp64)) < 0)
+   err("Could not read cliserv_magic: %m");
+   tmp64 = ntohll(tmp64);
+   if (tmp64 != NBD_OPT_REPLY_MAGIC) {
+   err("reply magic does not match");
+   }
+   if (read(sock, , sizeof(tmp32)) < 0)
+   err("Could not read option type: %m");
+   tmp32 = ntohl(tmp32);
+   if (tmp32 != NBD_OPT_STARTTLS)
+   err("Reply to wrong option");
+   if (read(sock, , sizeof(tmp32)) < 0)
+   err("Could not read option reply type: %m");
+   tmp32 = ntohl(tmp32);
+   if (tmp32 != NBD_REP_ACK) {
+   err("Option reply type != NBD_REP_ACK");
+   }
+   if (read(sock, , sizeof(tmp32)) < 0) err(
+   "Could not read option data length: %m");
+   tmp32 = ntohl(tmp32);
+   if (tmp32 != 0) {
+   err("Option reply data length != 0");
+   }
+   s = tlssession_new(0,
+  keyfile,
+  certfile,
+  cacertfile,
+  tlshostname,
+  !cacertfile || !tlshostname, // insecure flag
+#ifdef DODBG
+  1, // debug
+#else
+  0, // debug
+#endif
+  NULL, // quitfn
+  NULL, // erroutfn
+  NULL // opaque
+   );
+   if (!s)
+   err("Cannot establish TLS session");
+
+   if (socketpair(AF_UNIX, SOCK_STREAM, 0, plainfd) < 0)
+   err("Cannot get socket pair");
+
+   if (set_nonblocking(plainfd[0], 0) <0 ||
+   set_nonblocking(plainfd[1], 0) <0 ||
+   set_nonblocking(sock, 0) <0) {
+   close(plainfd[0]);
+   close(plainfd[1]);
+   err("Cannot set socket options");
+   }
+
+   ret = fork();
+   if (ret < 0)
+   err("Could not fork");
+   else if (ret == 0) {
+   // we are the child
+   if (daemon(0, 0) < 0) {
+   /* no one will see this */
+   fprintf(stderr, "Can't detach from the 
terminal");
+   exit(1);
+   }
+   signal (SIGPIPE, SIG_IGN);
+   

[Nbd] [PATCHv4 1/6] Add GnuTLS infrastructure

[Alex Bligh]

Add configure.ac section to detect GnuTLS

Add buffer.[ch] and crypto-gnutls.[ch] from
  https://github.com/abligh/tlsproxy

Add Makefile.am changes to link these new files in

Signed-off-by: Alex Bligh 
---
 Makefile.am   |   5 +
 buffer.c  | 225 +++
 buffer.h  |  45 
 configure.ac  |  15 ++
 crypto-gnutls.c   | 610 ++
 crypto-gnutls.h   |  43 
 tests/run/Makefile.am |   3 +
 7 files changed, 946 insertions(+)
 create mode 100644 buffer.c
 create mode 100644 buffer.h
 create mode 100644 crypto-gnutls.c
 create mode 100644 crypto-gnutls.h

diff --git a/Makefile.am b/Makefile.am
index 304db6d..9f7e4f7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -17,3 +17,8 @@ nbd_server_LDADD = @GLIB_LIBS@ libnbdsrv.la libcliserv.la
 nbd_trdump_LDADD = libcliserv.la
 make_integrityhuge_SOURCES = make-integrityhuge.c cliserv.h nbd.h nbd-debug.h
 EXTRA_DIST = maketr CodingStyle autogen.sh README.md
+TLSSRC = crypto-gnutls.c crypto-gnutls.h buffer.c buffer.h
+if GNUTLS
+nbd_client_SOURCES += $(TLSSRC)
+nbd_server_SOURCES += $(TLSSRC)
+endif
diff --git a/buffer.c b/buffer.c
new file mode 100644
index 000..e08efd8
--- /dev/null
+++ b/buffer.c
@@ -0,0 +1,225 @@
+/*
+
+The MIT License (MIT)
+
+Copyright (c) 2016 Wrymouth Innovation Ltd
+
+Permission is hereby granted, free of charge, to any person obtaining a
+copy of this software and associated documentation files (the "Software"),
+to deal in the Software without restriction, including without limitation
+the rights to use, copy, modify, merge, publish, distribute, sublicense,
+and/or sell copies of the Software, and to permit persons to whom the
+Software is furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included
+in all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+OTHER DEALINGS IN THE SOFTWARE.
+
+*/
+
+#include 
+
+#include "buffer.h"
+
+typedef struct buffer
+{
+  char *buf;
+  ssize_t size;
+  ssize_t hwm;
+  ssize_t ridx;
+  ssize_t widx;
+  int empty;
+} buffer_t;
+
+/* the buffer is organised internally as follows:
+ *
+ * * There are b->size bytes in the buffer.
+ *
+ * * Bytes are at offsets 0 to b->size-1
+ * 
+ * * b->ridx points to the first readable byte
+ *
+ * * b->widx points to the first empty space
+ *
+ * * b->ridx < b->widx indicates a non-wrapped buffer:
+ *
+ * 0   ridx widxsize
+ * |   ||   |
+ * V   VV   V
+ * X
+ *
+ * * b->ridx > b->widx indicates a wrapped buffer:
+ *
+ * 0   widx ridxsize
+ * |   ||   |
+ * V   VV   V
+ * .
+ *
+ * * b->ridx == b->widx indicates a FULL buffer:
+ *
+ * * b->ridx == b->widx indicates a wrapped buffer:
+ *
+ * 0   widx == ridxsize
+ * |   |   |
+ * V   V   V
+ * 
+ *
+ * An empty buffer is indicated by empty=1
+ *
+ */
+
+buffer_t *
+bufNew (ssize_t size, ssize_t hwm)
+{
+  buffer_t *b = calloc (1, sizeof (buffer_t));
+  b->buf = calloc (1, size);
+  b->size = size;
+  b->hwm = hwm;
+  b->empty = 1;
+  return b;
+}
+
+
+void
+bufFree (buffer_t * b)
+{
+  free (b->buf);
+  free (b);
+}
+
+/* get a maximal span to read. Returns 0 if buffer
+ * is empty
+ */
+ssize_t
+bufGetReadSpan (buffer_t * b, void **addr)
+{
+  if (b->empty)
+{
+  *addr = NULL;
+  return 0;
+}
+  *addr = &(b->buf[b->ridx]);
+  ssize_t len = b->widx - b->ridx;
+  if (len <= 0)
+len = b->size - b->ridx;
+  return len;
+}
+
+/* get a maximal span to write. Returns 0 id buffer is full
+ */
+ssize_t
+bufGetWriteSpan (buffer_t * b, void **addr)
+{
+  if (b->empty)
+{
+  *addr = b->buf;
+  b->ridx = 0;
+  b->widx = 0;
+  return b->size;
+}
+  if (b->ridx == b->widx)
+{
+  *addr = NULL;
+  return 0;
+}
+  *addr = &(b->buf[b->widx]);
+  ssize_t len = b->ridx - b->widx;
+  if (len <= 0)
+len = b->size - b->widx;
+  return len;
+}
+
+/* mark size bytes as read */
+void
+bufDoneRead (buffer_t * b, ssize_t size)
+{
+  while (!b->empty && (size > 0))
+{
+  /* empty can't occur here, so equal pointers means full */
+  ssize_t len = b->widx - b->ridx;
+  if (len <= 0)
+   len = b->size - b->ridx;

[Nbd] [PATCHv4 3/6] Add TLS support to server

[Alex Bligh]

Known problems / potential issues:

* It now passes a pointer to genconf around so handle_starttls can
  get at the certificates. This is a pity.

* It forks() the TLS proxy child using spawn_child. If we use fork()
  we get complaints about unknown children on SIGCHILD. If we use
  this method, each TLS connection potentially counts as 2 rather than one
  connection as far as maxconnections is concerned. In fact I don't
  *think* this happens because the hashtable itself isn't shared
  across fork(). We need to have a deeper think about how best to
  do this.

* Does not currently set minimum TLS version.

* Minimal testing to date.

Signed-off-by: Alex Bligh 
---
 cliserv.c|  17 +-
 cliserv.h|   2 +
 nbd-server.c | 183 ++-
 3 files changed, 174 insertions(+), 28 deletions(-)

diff --git a/cliserv.c b/cliserv.c
index 4e57cbb..50423c0 100644
--- a/cliserv.c
+++ b/cliserv.c
@@ -1,4 +1,5 @@
-#include 
+#include "config.h"
+#include 
 #include 
 #include 
 #include 
@@ -12,6 +13,20 @@ const u64 cliserv_magic = 0x00420281861253LL;
 const u64 opts_magic = 0x49484156454F5054LL;
 const u64 rep_magic = 0x3e889045565a9LL;
 
+/**
+ * Set a socket to blocking or non-blocking
+ *
+ * @param fd The socket's FD
+ * @param nb non-zero to set to non-blocking, else 0 to set to blocking
+ * @return 0 - OK, -1 failed
+ */
+int set_nonblocking(int fd, int nb) {
+int sf = fcntl (fd, F_GETFL, 0);
+if (sf == -1)
+return -1;
+return fcntl (fd, F_SETFL, nb ? (sf | O_NONBLOCK) : (sf & 
~O_NONBLOCK));
+}
+
 void setmysockopt(int sock) {
int size = 1;
 #if 0
diff --git a/cliserv.h b/cliserv.h
index 9c71cb8..39a657c 100644
--- a/cliserv.h
+++ b/cliserv.h
@@ -72,6 +72,7 @@ extern const u64 rep_magic;
 
 #define INFO(a) do { } while(0)
 
+int set_nonblocking(int fd, int nb);
 void setmysockopt(int sock);
 void err_nonfatal(const char *s);
 
@@ -95,6 +96,7 @@ void readit(int f, void *buf, size_t len);
 #define NBD_OPT_EXPORT_NAME(1) /** Client wants to select a named 
export (is followed by name of export) */
 #define NBD_OPT_ABORT  (2) /** Client wishes to abort negotiation 
*/
 #define NBD_OPT_LIST   (3) /** Client request list of supported 
exports (not followed by data) */
+#define NBD_OPT_STARTTLS   (5) /** Client wishes to start TLS */
 
 /* Replies the server can send during negotiation */
 #define NBD_REP_ACK(1) /** ACK a request. Data: option number 
to be acked */
diff --git a/nbd-server.c b/nbd-server.c
index 729156b..c98e22d 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -129,6 +129,10 @@
 #include 
 #endif
 
+#ifdef WITH_GNUTLS
+#include "crypto-gnutls.h"
+#endif
+
 /** Default position of the config file */
 #ifndef SYSCONFDIR
 #define SYSCONFDIR "/etc"
@@ -246,6 +250,8 @@ struct generic_conf {
gint threads;   /**< maximum number of parallel threads we want 
to run */
 };
 
+static pid_t spawn_child();
+
 /**
  * Translate a command name into human readable form
  *
@@ -1256,7 +1262,7 @@ static void send_reply(uint32_t opt, int net, uint32_t 
reply_type, size_t datasi
}
 }
 
-static CLIENT* handle_export_name(uint32_t opt, int net, GArray* servers, 
uint32_t cflags) {
+static CLIENT* handle_export_name(uint32_t opt, int net, GArray* servers, 
uint32_t cflags, int tlsfd) {
uint32_t namelen;
char* name;
int i;
@@ -1280,6 +1286,11 @@ static CLIENT* handle_export_name(uint32_t opt, int net, 
GArray* servers, uint32
for(i=0; ilen; i++) {
SERVER* serve = &(g_array_index(servers, SERVER, i));
if(!strcmp(serve->servename, name)) {
+   if ((tlsfd < 0) && (serve->flags & F_TLSONLY || 
glob_flags & F_TLSONLY)) {
+   err("Negotiation failed: Attempt to read from 
TLS only export without TLS");
+   free(name);
+   return NULL;
+   }
CLIENT* client = g_new0(CLIENT, 1);
client->server = serve;
client->exportsize = OFFT_MAX;
@@ -1324,16 +1335,125 @@ static void handle_list(uint32_t opt, int net, GArray* 
servers, uint32_t cflags)
send_reply(opt, net, NBD_REP_ACK, 0, NULL);
 }
 
+static int tlserrout (void *opaque, const char *format, va_list ap) {
+   char buf[1024];
+   if (vsnprintf(buf, 1024, format, ap) < 0)
+   return -1;
+   buf[1023] = 0; // in case it overran the buffer
+   err_nonfatal(buf);
+   return 0;
+}
+
+static void handle_starttls(uint32_t opt, int *net, int *tlsfd, GArray* 
servers, uint32_t cflags,
+   struct generic_conf *genconf) {
+   uint32_t len;
+   int i;
+   char buf[1024];
+   char *ptr = buf + sizeof(len);
+   int plainfd[2]; // [0] is used by the proxy, [1] is 

Re: [Nbd] [RFC PATCH] doc: Add new NBD_FLAG_BLOCK_SIZE extension

[Alex Bligh]

On 12 Apr 2016, at 15:22, Eric Blake  wrote:

> In other words, you're proposing that we limit the advertisement of
> block size ONLY via NBD_CMD_INFO/GO, so that gives people a reason to
> upgrade. I can buy that, if Wouter likes it.

Yes

> I think it's probably time to promote NBD_CMD_INFO/GO to stable, since
> we have several implementations coded up (I'm too late for qemu 2.6 to
> contain it, but qemu 2.7 will have the code merged in as soon as 2.6 is
> released) - you already mentioned that you looked at what it would take
> to get the reference implementation to also support it (with the tricky
> part being how to get the export size).

Save that I'm proposing extending it :-) But it should be a future-proof
extension.

>> Perhaps instead of the name (before the other bit that looks
>> like NBD_OPT_EXPORT_NAME's reply), we should *now* specify
>> a structured selection of export information, e.g.
>> 
>> : 32 bits: information element length
>> 0004: 32 bits: information element type
>> 0008 ... length: information element data
> 
> If we're going to send something other than the name, then it is no
> longer NBD_REP_SERVER, and we'd be better off inventing a new NBD_REP_.
> But if we keep it NBD_REP_SERVER (start with name like always, and end
> with transmission flags like NBD_OPT_EXPORT_NAME), we can still make the
> middle as structured as we want, as in this example:
> 
> S: 64 bits magic (0x3e889045565a9)
> S: 32 bits NBD_OPT_INFO (6)
> S: 32 bits NBD_REP_SERVER (2)
> S: 32 bits length
> S: 32 bits name length (3)
> S: name length bytes ("foo")
> S: 16 bits element type NBD_INFO_BLOCK_SIZE (1)
> S: 16 bits element length (12)
> S: 32 bits minimum block size
> S: 32 bits preferred block size
> S: 32 bits maximum block size
> S: 16 bits element type NBD_INFO_...
> S: 16 bits element length ...
> S: length bytes for next element...
> S: 16 bits element type NBD_INFO_END (0)
> S: 64 bits export size
> S: 16 bits transmission flags
> 
> where we can have as many additional information elements defined in the
> middle, and where the client doesn't have to negotiate anything (the
> server just goes ahead and provides everything that it knows, and the
> client is free to use the element types it knows and ignore via the
> length the ones it does not, until reaching NBD_INFO_END).

Yeah I could buy that. I forgot NBD_REP_SERVER is used by NBD_OPT_LIST
too.

>> We can then put what we like in there, and we have a future-proof
>> way of sending mount information.
>> 
>> The name then can be the first of these, i.e.
>> 
>> Element type 0001, Data: length of name (32 bits, followed by name).
>> 
>> We can then transmit the other data as an element type.
> 
> If we reuse NBD_REP_SERVER, then name can't be any different from how
> NBD_OPT_LIST uses it. Otherwise, we want a new NBD_REP_ type (but then,
> we could even make the name optional, as for NBD_OPT_INFO it is only
> useful information if the export has a canonical name that differs from
> the name the user asked about).

Agree.

>>> +
>>> +The minimum block size MUST be a power of 2, and represents the
>>> +smallest addressable length and alignment within the export, even if
>>> +writing a block that small requires the server to use a less-efficient
>>> +read-modify-write action.  This value MAY be as small as 1 if the
>>> +export is backed by a regular file, although the values of 512 or 4096
>>> +are more typical for an export backed by a block device.  When block
>>> +sizes are negotiated, the server MUST return an export size that is an
>>> +integer multiple of the minimum block size.
>> 
>> I think you should make it clear that ALL requests MUST have an
>> offset AND a length which are multiples of the minimum blocksize.
>> Ah, you handle this below, but it might be worth saying here
>> what this means.
>> 
>> +1 on export size.
>> 
>> Should this not be larger than 32M?
> 
> True. In fact, minimum block size should probably not be larger than
> 32k,

Hah yes.

> since the NBD_CMD_FLAG_DF says that we only guarantee that the
> server won't fragment at 32k or smaller, but at the same time, the
> server should not be fragmenting smaller than minimum block size.  If
> the minimum block size is greater than 32k, then the client can't
> usefully set NBD_CMD_FLAG_DF, but also can't usefully guarantee the
> result will be unfragmented.  And while 4k physical blocks are becoming
> more prominent, I seriously doubt we have block devices where minimum
> block size larger than 32k are going to be in heavy use any time soon.
> 
> I still strongly think that export size MUST be an integer multiple of
> minimum block size (otherwise, you have the funky situation of the tail
> of the file being inaccessible via NBD) - perhaps this means we should
> add a statement that if an NBD server will be visiting a file that is
> not a multiple of the minimum block size it is willing to let the client
> use, then the server either has to truncate 

Re: [Nbd] [PATCHv2 6/6] Add TLS testing to nbd-tester-client.c

[Alex Bligh]

On 12 Apr 2016, at 15:15, Wouter Verhelst  wrote:

> On Mon, Apr 11, 2016 at 06:15:39PM +0100, Alex Bligh wrote:
>> This commit adds TLS testing to nbd-tester-client and 'make check'.
>> If TLS is not compiled in, then the test is skipped.
> 
> Alternatively, it could check that nbd-server produces an error in that
> case.

That's quite difficult to do per se as simpletest would need to
read config.h or similar. However, in practice it *is* doing this
as nbd-tester-client.c only exits with the magic code 77 if
one of those options is specified. If it didn't exit with that
code, it would either produce a failure from an immediate exit with
-1, or it would fail as TLS isn't enabled. Unless the failure is
accidentally building in entirely functional TLS support of course,
but simpletest's reading of config.h could be fooled the same way.

>> Signed-off-by: Alex Bligh 
>> ---
>> nbd.h   |   2 +
>> tests/run/Makefile.am   |  13 +++-
>> tests/run/certs/ca-cert.pem |  20 +
>> tests/run/certs/ca-key.pem  |  32 
>> tests/run/certs/ca.info |   3 +
>> tests/run/certs/client-cert.pem |  23 ++
>> tests/run/certs/client-key.pem  |  32 
>> tests/run/certs/client.info |   8 ++
>> tests/run/certs/server-cert.pem |  22 ++
>> tests/run/certs/server-key.pem  |  32 
>> tests/run/certs/server.info |   5 ++
> 
> Might be good to add a little README there to explain what each of the
> files does.

Will do for v4.

>>  * This is the packet used for communication between client and
>>  * server. All data are in network byte order.
>> diff --git a/tests/run/Makefile.am b/tests/run/Makefile.am
>> index d1e28ed..050b51d 100644
>> --- a/tests/run/Makefile.am
>> +++ b/tests/run/Makefile.am
>> @@ -1,11 +1,16 @@
>> +if GNUTLS
>> +TLSSRC = $(top_srcdir)/crypto-gnutls.c $(top_srcdir)/crypto-gnutls.h 
>> $(top_srcdir)/buffer.c $(top_srcdir)/buffer.h
>> +else
>> +TLSSRC =
>> +endif
> 
> Same here as in the main Makefile.am, obviously.

Done already in v3.

>> 
>> +/**
>> + * Set a socket to blocking or non-blocking
>> + *
>> + * @param fd The socket's FD
>> + * @param nb non-zero to set to non-blocking, else 0 to set to blocking
>> + * @return 0 - OK, -1 failed
>> + */
>> +int set_nonblocking(int fd, int nb) {
>> +int sf = fcntl (fd, F_GETFL, 0);
>> +if (sf == -1)
>> +return -1;
>> +return fcntl (fd, F_SETFL, nb ? (sf | O_NONBLOCK) : (sf & 
>> ~O_NONBLOCK));
>> +}
> 
> Maybe this should be moved to cliserv.[ch]?

Will do for v4.

> [...]
>> diff --git a/tests/run/simple_test b/tests/run/simple_test
>> index 0c05ea1..80b99dc 100755
>> --- a/tests/run/simple_test
>> +++ b/tests/run/simple_test
>> @@ -284,6 +284,51 @@ EOF
>>  ./nbd-tester-client -N export1 -u ${tmpdir}/unix.sock
>>  retval=$?
>>  ;;
>> +*/tls)
>> +# TLS test
>> +certdir=`pwd`/certs
> 
> I prefer $()-style command substitution over backticks.

Will do for v4.

> [...]
>> +cat >${conffile} <> +[generic]
>> +certfile = $certdir/server-cert.pem
>> +keyfile = $certdir/server-key.pem
>> +cacertfile = $certdir/ca-cert.pem
> 
> More whitespace mishaps (and that happens again for tlshuge, too)

Will so. Not sure what the standard for that file is, but whatever it is
it's not emacs' default.

-- 
Alex Bligh





--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] [PATCHv2 5/6] Add TLS support to server

[Wouter Verhelst]

On Tue, Apr 12, 2016 at 03:15:27PM +0100, Alex Bligh wrote:
> Wouter,
> 
> On 12 Apr 2016, at 15:04, Wouter Verhelst  wrote:
> 
> > On Mon, Apr 11, 2016 at 06:15:38PM +0100, Alex Bligh wrote:
> > [...]
> >> +#ifdef WITH_GNUTLS
> > [...]
> >> +#else
> >> +
> >> +  send_reply(opt, *net, NBD_REP_ERR_UNSUP, 0, NULL);
> > 
> > NBD_REP_ERR_POLICY (or NBD_REP_ERR_PLATFORM, perhaps).
> 
> But this is *without* TLS support compiled in. Surely the correct error
> is NBD_REP_ERR_UNSUP, i.e. the same error as it gives now without
> the TLS code? IE it can NEVER work against this server, unless
> the server's code is changed. It's not a local policy thing.

PLATFORM is "this option is not supported on the platform where it was
compiled". If that platform doesn't have GnuTLS, then you disable
STARTTLS, so it can't work.

Maybe the names were wrong, but the plan was:

INVALID -- client sent something obviously wrong
POLICY -- server admin did something wrong or disabled the option
UNSUP -- server does not have code to handle request
PLATFORM -- server does have code to handle request, but it's not
  compiled in for whatever reason (e.g., something required on the
  platform is not available)

Obviously the last applies here.

(and just as obviously POLICY doesn't, either -- the "perhaps" line
above is the wrong way round, sorry)

The solution to an INVALID error is "fix the damn client"
The solution to a POLICY error is "fix the damn server config"
The solution to a PLATFORM error is "recompile the server and/or run it
on the right system"
The solution to an UNSUP error is "implement missing functionality"

There's a clear difference between the latter two.

> > You should think of NBD_REP_ERR_INVALID as 4xx errors (i.e., "you're
> > doing it wrong"), and of NBD_REP_ERR_POLICY (or NBD_REP_ERR_PLATFORM) as
> > 5xx errors ("something went wrong on my end").
> 
> So nothing went wrong server end. He's running a server without
> TLS built in.

It's a detail, I realize, but that's not what this was meant to do.

> > NBD_REP_ERR_UNSUP really is only meant for "I don't know what the 
> > you're talking about". It should only be referenced just once in a
> > server (in a "default:" case of a switch statement)
> 
> He doesn't know what you're talking about - no TLS!

He does, you've gone into a separate function.

> > [...]
> >> -sock_flags_old = fcntl(net, F_GETFL, 0);
> >> -if (sock_flags_old == -1) {
> >> -msg(LOG_ERR, "Failed to get socket flags");
> >> -goto handler_err;
> >> -}
> >> -
> >> -sock_flags_new = sock_flags_old & ~O_NONBLOCK;
> >> -if (sock_flags_new != sock_flags_old &&
> >> -fcntl(net, F_SETFL, sock_flags_new) == -1) {
> >> -msg(LOG_ERR, "Failed to set socket to blocking mode");
> >> -goto handler_err;
> >> -}
> >> +  if (set_nonblocking(client->net, 0) < 0) {
> >> +  msg(LOG_ERR, "Failed to set socket to blocking mode");
> >> +  goto handler_err;
> >> +  }
> > 
> > Some whitespace errors there.
> 
> Couldn't see them, but reindented it for v4.

:)

> >> if (set_peername(net, client)) {
> >> msg(LOG_ERR, "Failed to set peername");
> >> @@ -2241,11 +2375,15 @@ handle_modern_connection(GArray *const servers, 
> >> const int sock)
> >> 
> >> msg(LOG_INFO, "Starting to serve");
> >> serveconnection(client);
> >> +  close (net);
> >> +  if (client->net != net)
> >> +  close (client->net);
> > 
> > Probably safer to have a block here, rather than a single line?
> 
> You mean around "close (client->net)"? Sure, will do for v4. Is there
> some guideline you use? (CodingStyle is remarkably tolerant).

CodingStyle is also remarkably ancient ;-)

I should probably update or ditch it. Some other time.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
   people in the world who think they really understand all of its rules,
   and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] NBD_CMD_DISC

[Eric Blake]

On 04/12/2016 03:48 AM, Daniel P. Berrange wrote:

> gnutls_bye should return GNUTLS_E_AGAIN on blocking and
> require that you call it again.
> 
> QEMU should absolutely ensure all pending buffers are flushed
> before it drops the connection, but I'm not sure it is needed
> to actually call gnutls_bye in order to achieve this, because
> I don't believe gnutls should be caching any outgoing buffers
> we're sending, since we don't use gnutls_cork.

I don't know if we _should_ be using gnutls_cork - we have
qio_channel_set_cork, and some of the coroutines definitely play with
the TCP cork; it seems like layering-wise, if we have situations where a
block device wants to set the TCP cork when running plaintext, it would
also want to set the TLS cork when running TLS.  But that's a question
for the qemu folks.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] [RFC PATCH] doc: Add new NBD_FLAG_BLOCK_SIZE extension

[Eric Blake]

On 04/12/2016 04:34 AM, Alex Bligh wrote:
> 
> On 12 Apr 2016, at 05:04, Eric Blake  wrote:
> 
>> Existing NBD servers often have limitations, such as requiring
>> actions to be aligned to block sizes or limiting maximum
>> transactions to avoid denial of service attacks; for example,
>> qemu's NBD server refuses any transaction larger than 32M.  But
>> to date, clients have to learn these limitations via out-of-band
>> means.
> 
> I fully support solving this.
> 
>> This adds a new negotiation flag to let client and server agree
>> to alter the response to NBD_OPT_EXPORT_NAME and NBD_OPT_GO to
>> advertise the server's limitations, as well as documenting sane
>> defaults that a client should follow if the information was not
>> documented.
>>
>> This is done as a global flag rather than a new NBD_OPT, for a
>> couple of reasons.  First, block size information can be
>> implemented even without support for NBD_FLAG_FIXED_NEWSTYLE,
>> because of the reserved block of zeroes sent in response to
>> NBD_CMD_EXPORT_NAME.
> 
> This is a bad reason. Clients & servers without support
> for NBD_FLAG_FIXED_NEWSTYLE should be burned with fire.
> We should do nothing to help them beyond help them upgrade
> to sanity (I'm serious).
> 
>>  Second, negotiating the option changes
>> the NBD_REP_SERVER message size sent by the server for
>> NBD_OPT_INFO; although this reply is structured and the length
>> will be reliable, it would still be awkward for out-of-order
>> option replies from the server to ping-pong in size within a
>> single session.  Making the negotiation part of the initial
>> global flag handshake means the rest of the session has
>> consistent message sizing.
> 
> I don't think this is a great reason either. The INFO
> extension is still experimental. I would prefer to fix the
> INFO statement to transmit information about the export.

In other words, you're proposing that we limit the advertisement of
block size ONLY via NBD_CMD_INFO/GO, so that gives people a reason to
upgrade. I can buy that, if Wouter likes it.

I think it's probably time to promote NBD_CMD_INFO/GO to stable, since
we have several implementations coded up (I'm too late for qemu 2.6 to
contain it, but qemu 2.7 will have the code merged in as soon as 2.6 is
released) - you already mentioned that you looked at what it would take
to get the reference implementation to also support it (with the tricky
part being how to get the export size).

> 
> Perhaps instead of the name (before the other bit that looks
> like NBD_OPT_EXPORT_NAME's reply), we should *now* specify
> a structured selection of export information, e.g.
> 
> : 32 bits: information element length
> 0004: 32 bits: information element type
> 0008 ... length: information element data

If we're going to send something other than the name, then it is no
longer NBD_REP_SERVER, and we'd be better off inventing a new NBD_REP_.
 But if we keep it NBD_REP_SERVER (start with name like always, and end
with transmission flags like NBD_OPT_EXPORT_NAME), we can still make the
middle as structured as we want, as in this example:

S: 64 bits magic (0x3e889045565a9)
S: 32 bits NBD_OPT_INFO (6)
S: 32 bits NBD_REP_SERVER (2)
S: 32 bits length
S: 32 bits name length (3)
S: name length bytes ("foo")
S: 16 bits element type NBD_INFO_BLOCK_SIZE (1)
S: 16 bits element length (12)
S: 32 bits minimum block size
S: 32 bits preferred block size
S: 32 bits maximum block size
S: 16 bits element type NBD_INFO_...
S: 16 bits element length ...
S: length bytes for next element...
S: 16 bits element type NBD_INFO_END (0)
S: 64 bits export size
S: 16 bits transmission flags

where we can have as many additional information elements defined in the
middle, and where the client doesn't have to negotiate anything (the
server just goes ahead and provides everything that it knows, and the
client is free to use the element types it knows and ignore via the
length the ones it does not, until reaching NBD_INFO_END).

> 
> We can then put what we like in there, and we have a future-proof
> way of sending mount information.
> 
> The name then can be the first of these, i.e.
> 
> Element type 0001, Data: length of name (32 bits, followed by name).
> 
> We can then transmit the other data as an element type.

If we reuse NBD_REP_SERVER, then name can't be any different from how
NBD_OPT_LIST uses it. Otherwise, we want a new NBD_REP_ type (but then,
we could even make the name optional, as for NBD_OPT_INFO it is only
useful information if the export has a canonical name that differs from
the name the user asked about).


> 
>> +The three block size fields represent constraints that the client
>> +SHOULD obey during transmission phase, as further detailed in
>> +individual transmission requests.  If `NBD_FLAG_C_BLOCK_SIZE` was not
>> +negotiated, the server MUST use zero for all three fields (or omit
>> +them, when `NBD_FLAG_C_NO_ZEROES` is negotiated), and the client
>> +SHOULD assume a minimum block 

Re: [Nbd] [PATCHv2 5/6] Add TLS support to server

[Alex Bligh]

Wouter,

On 12 Apr 2016, at 15:04, Wouter Verhelst  wrote:

> On Mon, Apr 11, 2016 at 06:15:38PM +0100, Alex Bligh wrote:
> [...]
>> +#ifdef WITH_GNUTLS
> [...]
>> +#else
>> +
>> +send_reply(opt, *net, NBD_REP_ERR_UNSUP, 0, NULL);
> 
> NBD_REP_ERR_POLICY (or NBD_REP_ERR_PLATFORM, perhaps).

But this is *without* TLS support compiled in. Surely the correct error
is NBD_REP_ERR_UNSUP, i.e. the same error as it gives now without
the TLS code? IE it can NEVER work against this server, unless
the server's code is changed. It's not a local policy thing.

> You should think of NBD_REP_ERR_INVALID as 4xx errors (i.e., "you're
> doing it wrong"), and of NBD_REP_ERR_POLICY (or NBD_REP_ERR_PLATFORM) as
> 5xx errors ("something went wrong on my end").

So nothing went wrong server end. He's running a server without
TLS built in.

> NBD_REP_ERR_UNSUP really is only meant for "I don't know what the 
> you're talking about". It should only be referenced just once in a
> server (in a "default:" case of a switch statement)

He doesn't know what you're talking about - no TLS!

> [...]
>> -sock_flags_old = fcntl(net, F_GETFL, 0);
>> -if (sock_flags_old == -1) {
>> -msg(LOG_ERR, "Failed to get socket flags");
>> -goto handler_err;
>> -}
>> -
>> -sock_flags_new = sock_flags_old & ~O_NONBLOCK;
>> -if (sock_flags_new != sock_flags_old &&
>> -fcntl(net, F_SETFL, sock_flags_new) == -1) {
>> -msg(LOG_ERR, "Failed to set socket to blocking mode");
>> -goto handler_err;
>> -}
>> +if (set_nonblocking(client->net, 0) < 0) {
>> +msg(LOG_ERR, "Failed to set socket to blocking mode");
>> +goto handler_err;
>> +}
> 
> Some whitespace errors there.

Couldn't see them, but reindented it for v4.

>> if (set_peername(net, client)) {
>> msg(LOG_ERR, "Failed to set peername");
>> @@ -2241,11 +2375,15 @@ handle_modern_connection(GArray *const servers, 
>> const int sock)
>> 
>> msg(LOG_INFO, "Starting to serve");
>> serveconnection(client);
>> +close (net);
>> +if (client->net != net)
>> +close (client->net);
> 
> Probably safer to have a block here, rather than a single line?

You mean around "close (client->net)"? Sure, will do for v4. Is there
some guideline you use? (CodingStyle is remarkably tolerant).

-- 
Alex Bligh





--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] [PATCHv2 6/6] Add TLS testing to nbd-tester-client.c

[Wouter Verhelst]

On Mon, Apr 11, 2016 at 06:15:39PM +0100, Alex Bligh wrote:
> This commit adds TLS testing to nbd-tester-client and 'make check'.
> If TLS is not compiled in, then the test is skipped.

Alternatively, it could check that nbd-server produces an error in that
case.

> Signed-off-by: Alex Bligh 
> ---
>  nbd.h   |   2 +
>  tests/run/Makefile.am   |  13 +++-
>  tests/run/certs/ca-cert.pem |  20 +
>  tests/run/certs/ca-key.pem  |  32 
>  tests/run/certs/ca.info |   3 +
>  tests/run/certs/client-cert.pem |  23 ++
>  tests/run/certs/client-key.pem  |  32 
>  tests/run/certs/client.info |   8 ++
>  tests/run/certs/server-cert.pem |  22 ++
>  tests/run/certs/server-key.pem  |  32 
>  tests/run/certs/server.info |   5 ++

Might be good to add a little README there to explain what each of the
files does.

>  tests/run/nbd-tester-client.c   | 169 
> +++-
>  tests/run/simple_test   |  45 +++
>  13 files changed, 402 insertions(+), 4 deletions(-)
>  create mode 100644 tests/run/certs/ca-cert.pem
>  create mode 100644 tests/run/certs/ca-key.pem
>  create mode 100644 tests/run/certs/ca.info
>  create mode 100644 tests/run/certs/client-cert.pem
>  create mode 100644 tests/run/certs/client-key.pem
>  create mode 100644 tests/run/certs/client.info
>  create mode 100644 tests/run/certs/server-cert.pem
>  create mode 100644 tests/run/certs/server-key.pem
>  create mode 100644 tests/run/certs/server.info
> 
> diff --git a/nbd.h b/nbd.h
> index 732c605..90c97a6 100644
> --- a/nbd.h
> +++ b/nbd.h
> @@ -59,6 +59,8 @@ enum {
>  #define NBD_REPLY_MAGIC 0x67446698
>  /* Do *not* use magics: 0x12560953 0x96744668. */
>  
> +#define NBD_OPT_REPLY_MAGIC 0x3e889045565a9LL
> +
>  /*
>   * This is the packet used for communication between client and
>   * server. All data are in network byte order.
> diff --git a/tests/run/Makefile.am b/tests/run/Makefile.am
> index d1e28ed..050b51d 100644
> --- a/tests/run/Makefile.am
> +++ b/tests/run/Makefile.am
> @@ -1,11 +1,16 @@
> +if GNUTLS
> +TLSSRC = $(top_srcdir)/crypto-gnutls.c $(top_srcdir)/crypto-gnutls.h 
> $(top_srcdir)/buffer.c $(top_srcdir)/buffer.h
> +else
> +TLSSRC =
> +endif

Same here as in the main Makefile.am, obviously.

[...]
> diff --git a/tests/run/nbd-tester-client.c b/tests/run/nbd-tester-client.c
> index ed4d03b..c80204d 100644
> --- a/tests/run/nbd-tester-client.c
> +++ b/tests/run/nbd-tester-client.c
> @@ -42,6 +42,10 @@
>  #define MY_NAME "nbd-tester-client"
>  #include "cliserv.h"
>  
> +#ifdef WITH_GNUTLS
> +#include "crypto-gnutls.h"
> +#endif
> +
>  static gchar errstr[1024];
>  const static int errstr_len = 1023;
>  
> @@ -50,6 +54,10 @@ static uint64_t size;
>  static int looseordering = 0;
>  
>  static gchar *transactionlog = "nbd-tester-client.tr";
> +static gchar *certfile = NULL;
> +static gchar *keyfile = NULL;
> +static gchar *cacertfile = NULL;
> +static gchar *tlshostname = NULL;
>  
>  typedef enum {
>   CONNECTION_TYPE_NONE,
> @@ -341,6 +349,24 @@ static inline int write_all(int f, void *buf, size_t len)
>   return retval;
>  }
>  
> +/**
> + * Set a socket to blocking or non-blocking
> + *
> + * @param fd The socket's FD
> + * @param nb non-zero to set to non-blocking, else 0 to set to blocking
> + * @return 0 - OK, -1 failed
> + */
> +int set_nonblocking(int fd, int nb) {
> +int sf = fcntl (fd, F_GETFL, 0);
> +if (sf == -1)
> +return -1;
> +return fcntl (fd, F_SETFL, nb ? (sf | O_NONBLOCK) : (sf & 
> ~O_NONBLOCK));
> +}

Maybe this should be moved to cliserv.[ch]?

[...]
> diff --git a/tests/run/simple_test b/tests/run/simple_test
> index 0c05ea1..80b99dc 100755
> --- a/tests/run/simple_test
> +++ b/tests/run/simple_test
> @@ -284,6 +284,51 @@ EOF
>   ./nbd-tester-client -N export1 -u ${tmpdir}/unix.sock
>   retval=$?
>   ;;
> + */tls)
> + # TLS test
> + certdir=`pwd`/certs

I prefer $()-style command substitution over backticks.

[...]
> + cat >${conffile} < +[generic]
> + certfile = $certdir/server-cert.pem
> +keyfile = $certdir/server-key.pem
> +cacertfile = $certdir/ca-cert.pem

More whitespace mishaps (and that happens again for tlshuge, too)

> +[export1]
> + exportname = $tmpnam
> + flush = true
> + fua = true
> + rotational = true
> + filesize = 52428800
> + temporary = true
> +EOF
> + ../../nbd-server -C ${conffile} -p ${pidfile} &
> + PID=$!
> + sleep 1
> + ./nbd-tester-client -N export1 -i -t 
> "${mydir}/integrity-test.tr" -C "${certdir}/client-cert.pem" -K 
> "${certdir}/client-key.pem" -A "${certdir}/ca-cert.pem" -H 127.0.0.1 localhost
> + retval=$?
> + ;;
> + */tlshuge)
> + # TLS test with big operations
> + # takes a while
> 

[Nbd] [PATCHv3 4/6] Add TLS testing to nbd-tester-client.c

[Alex Bligh]

This commit adds TLS testing to nbd-tester-client and 'make check'.
If TLS is not compiled in, then the test is skipped.

Signed-off-by: Alex Bligh 
---
 nbd.h   |   2 +
 tests/run/Makefile.am   |  11 ++-
 tests/run/certs/ca-cert.pem |  20 +
 tests/run/certs/ca-key.pem  |  32 
 tests/run/certs/ca.info |   3 +
 tests/run/certs/client-cert.pem |  23 ++
 tests/run/certs/client-key.pem  |  32 
 tests/run/certs/client.info |   8 ++
 tests/run/certs/server-cert.pem |  22 ++
 tests/run/certs/server-key.pem  |  32 
 tests/run/certs/server.info |   5 ++
 tests/run/nbd-tester-client.c   | 169 +++-
 tests/run/simple_test   |  45 +++
 13 files changed, 401 insertions(+), 3 deletions(-)
 create mode 100644 tests/run/certs/ca-cert.pem
 create mode 100644 tests/run/certs/ca-key.pem
 create mode 100644 tests/run/certs/ca.info
 create mode 100644 tests/run/certs/client-cert.pem
 create mode 100644 tests/run/certs/client-key.pem
 create mode 100644 tests/run/certs/client.info
 create mode 100644 tests/run/certs/server-cert.pem
 create mode 100644 tests/run/certs/server-key.pem
 create mode 100644 tests/run/certs/server.info

diff --git a/nbd.h b/nbd.h
index 732c605..90c97a6 100644
--- a/nbd.h
+++ b/nbd.h
@@ -59,6 +59,8 @@ enum {
 #define NBD_REPLY_MAGIC 0x67446698
 /* Do *not* use magics: 0x12560953 0x96744668. */
 
+#define NBD_OPT_REPLY_MAGIC 0x3e889045565a9LL
+
 /*
  * This is the packet used for communication between client and
  * server. All data are in network byte order.
diff --git a/tests/run/Makefile.am b/tests/run/Makefile.am
index 29e4f7f..60fdb25 100644
--- a/tests/run/Makefile.am
+++ b/tests/run/Makefile.am
@@ -1,5 +1,10 @@
+if GNUTLS
+TLSSRC = $(top_srcdir)/crypto-gnutls.c $(top_srcdir)/crypto-gnutls.h 
$(top_srcdir)/buffer.c $(top_srcdir)/buffer.h
+else
+TLSSRC =
+endif
 TESTS_ENVIRONMENT=$(srcdir)/simple_test
-TESTS = cfg1 cfgmulti cfgnew cfgsize write flush integrity dirconfig list 
rowrite tree rotree unix #integrityhuge
+TESTS = cfg1 cfgmulti cfgnew cfgsize write flush integrity dirconfig list 
rowrite tree rotree unix tls #integrityhuge tlshuge
 check_PROGRAMS = nbd-tester-client
 nbd_tester_client_SOURCES = nbd-tester-client.c $(top_srcdir)/cliserv.h 
$(top_srcdir)/netdb-compat.h $(top_srcdir)/cliserv.c
 if GNUTLS
@@ -8,7 +13,7 @@ endif
 nbd_tester_client_CFLAGS = @CFLAGS@ @GLIB_CFLAGS@
 nbd_tester_client_CPPFLAGS = -I$(top_srcdir)
 nbd_tester_client_LDADD = @GLIB_LIBS@
-EXTRA_DIST = integrity-test.tr integrityhuge-test.tr simple_test
+EXTRA_DIST = integrity-test.tr integrityhuge-test.tr simple_test 
certs/client-key.pem certs/client-cert.pem certs/server-cert.pem 
certs/ca-cert.pem certs/ca.info certs/client.info certs/server-key.pem 
certs/ca-key.pem certs/server.info
 cfg1:
 cfgmulti:
 cfgnew:
@@ -23,3 +28,5 @@ rowrite:
 tree:
 rotree:
 unix:
+tls:
+tlshuge:
diff --git a/tests/run/certs/ca-cert.pem b/tests/run/certs/ca-cert.pem
new file mode 100644
index 000..a3b8ba0
--- /dev/null
+++ b/tests/run/certs/ca-cert.pem
@@ -0,0 +1,20 @@
+-BEGIN CERTIFICATE-
+MIIDSzCCAgOgAwIBAgIEVwQNzDANBgkqhkiG9w0BAQsFADAVMRMwEQYDVQQDEwpB
+bGV4IEJsaWdoMB4XDTE2MDQwNTE5MTEwOFoXDTE3MDQwNTE5MTEwOFowFTETMBEG
+A1UEAxMKQWxleCBCbGlnaDCCAVIwDQYJKoZIhvcNAQEBBQADggE/ADCCAToCggEx
+AMTtsWhYOU8iEFlGfkCb+RbqeyansOHjVS90jLNXSmd8GLS9vpfLogR3b4Vc9jCc
+aJuQqJhSvhP5JDOuEycEN3u8Yhemi1AEPiF6ZIRczTxw4cWgR6km0g4AoaSFTWD7
+baQkqKFygawYY8rDS0Q7Op+POqpCUz7irRSGbig3FVA3QLoGGBkiY8baB795XP6r
+SBmyURWnPNVpsmFf0c5GbLb+CriUkmaR3Hf9cUj/Q2fowRJ3zBSukl4Xiw20Aj6T
+PL/k6yFJvqX5j4BtWUNG6aji6ckbtg1gnEW65wPYw1Mzw5wGFA73u+1lT5/vVhmA
+CZaCKuu5wKGJe7fHhdvE23BPcJZ699/miRBERLnOVQnO6t3SAgMOS3yQ5TlPczXd
+P9GBfnk6FaVjvY98nPgQeAkCAwEAAaNDMEEwDwYDVR0TAQH/BAUwAwEB/zAPBgNV
+HQ8BAf8EBQMDBwQAMB0GA1UdDgQWBBS+f9HxvzeLlZ1ubwEC1PC7ZREDnjANBgkq
+hkiG9w0BAQsFAAOCATEAvayiGrM7ouetVPZkxKF0qIWMsXh29gNuB7L4iHUTsIQn
+3uB/EdrtvWCZw9S++y87XEoxbyBjX4GRJiB2/6+098YGA5QBa2+f2rtZeMbeGsDz
+pZmvwXNBJOyZM7GY7c+yvsrPnUdd25cWXFslQAWvvNuRyW/oTbVVlAT36UJaMBDh
+uteuQT33AH+RU49ZHZrSEaMeM9mXOPquLkHPsXiG4XHLTBZnVj6Y90iz7SXRRg6s
+u2q4kiej8Jy9KlcPKgpbvij3tKmYuBpadQrVGG8U2mRFWuc2561cIWQiLWiRvPM+
+GjlfmyIwkUoLjo54qaCOE6sIKAX1bw/JVZeHrVvvjeEnqCfnnJyfOicdG7eW0BjI
+3016pFiSL7Eqiei9ltMFYoag6plz1mAAOudFZUQKhg==
+-END CERTIFICATE-
diff --git a/tests/run/certs/ca-key.pem b/tests/run/certs/ca-key.pem
new file mode 100644
index 000..ed76fd8
--- /dev/null
+++ b/tests/run/certs/ca-key.pem
@@ -0,0 +1,32 @@
+-BEGIN RSA PRIVATE KEY-
+MIIFewIBAAKCATEAxO2xaFg5TyIQWUZ+QJv5Fup7Jqew4eNVL3SMs1dKZ3wYtL2+
+l8uiBHdvhVz2MJxom5ComFK+E/kkM64TJwQ3e7xiF6aLUAQ+IXpkhFzNPHDhxaBH
+qSbSDgChpIVNYPttpCSooXKBrBhjysNLRDs6n486qkJTPuKtFIZuKDcVUDdAugYY
+GSJjxtoHv3lc/qtIGbJRFac81WmyYV/RzkZstv4KuJSSZpHcd/1xSP9DZ+jBEnfM
+FK6SXheLDbQCPpM8v+TrIUm+pfmPgG1ZQ0bpqOLpyRu2DWCcRbrnA9jDUzPDnAYU
+Dve77WVPn+9WGYAJloIq67nAoYl7t8eF28TbcE9wlnr33+aJEEREuc5VCc7q3dIC

Re: [Nbd] [PATCHv2 5/6] Add TLS support to server

[Wouter Verhelst]

On Mon, Apr 11, 2016 at 06:15:38PM +0100, Alex Bligh wrote:
[...]
> +#ifdef WITH_GNUTLS
[...]
> +#else
> +
> + send_reply(opt, *net, NBD_REP_ERR_UNSUP, 0, NULL);

NBD_REP_ERR_POLICY (or NBD_REP_ERR_PLATFORM, perhaps).

You should think of NBD_REP_ERR_INVALID as 4xx errors (i.e., "you're
doing it wrong"), and of NBD_REP_ERR_POLICY (or NBD_REP_ERR_PLATFORM) as
5xx errors ("something went wrong on my end").

NBD_REP_ERR_UNSUP really is only meant for "I don't know what the 
you're talking about". It should only be referenced just once in a
server (in a "default:" case of a switch statement)

[...]
> -sock_flags_old = fcntl(net, F_GETFL, 0);
> -if (sock_flags_old == -1) {
> -msg(LOG_ERR, "Failed to get socket flags");
> -goto handler_err;
> -}
> -
> -sock_flags_new = sock_flags_old & ~O_NONBLOCK;
> -if (sock_flags_new != sock_flags_old &&
> -fcntl(net, F_SETFL, sock_flags_new) == -1) {
> -msg(LOG_ERR, "Failed to set socket to blocking mode");
> -goto handler_err;
> -}
> + if (set_nonblocking(client->net, 0) < 0) {
> + msg(LOG_ERR, "Failed to set socket to blocking mode");
> + goto handler_err;
> + }

Some whitespace errors there.

>  if (set_peername(net, client)) {
>  msg(LOG_ERR, "Failed to set peername");
> @@ -2241,11 +2375,15 @@ handle_modern_connection(GArray *const servers, const 
> int sock)
>  
>  msg(LOG_INFO, "Starting to serve");
>  serveconnection(client);
> + close (net);
> + if (client->net != net)
> + close (client->net);

Probably safer to have a block here, rather than a single line?

[...]

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
   people in the world who think they really understand all of its rules,
   and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

[Nbd] [PATCHv3 5/6] Add options to nbd-client for TLS support

[Alex Bligh]

Signed-off-by: Alex Bligh 
---
 nbd-client.c | 60 +---
 1 file changed, 57 insertions(+), 3 deletions(-)

diff --git a/nbd-client.c b/nbd-client.c
index d9cdb19..ff79a27 100644
--- a/nbd-client.c
+++ b/nbd-client.c
@@ -344,7 +344,7 @@ void negotiate(int sock, u64 *rsize64, uint16_t *flags, 
char* name, uint32_t nee
*rsize64 = size64;
 }
 
-bool get_from_config(char* cfgname, char** name_ptr, char** dev_ptr, char** 
hostn_ptr, int* bs, int* timeout, int* persist, int* swap, int* sdp, int* 
b_unix, char**port) {
+bool get_from_config(char* cfgname, char** name_ptr, char** dev_ptr, char** 
hostn_ptr, int* bs, int* timeout, int* persist, int* swap, int* sdp, int* 
b_unix, char**port, char **certfile, char **keyfile, char **cacertfile, char 
**tlshostname) {
int fd = open(SYSCONFDIR "/nbdtab", O_RDONLY);
bool retval = false;
if(fd < 0) {
@@ -433,6 +433,22 @@ bool get_from_config(char* cfgname, char** name_ptr, 
char** dev_ptr, char** host
loc += 4;
goto next;
}
+   if(!strncmp(loc, "certfile=", 9)) {
+   *certfile = strndup(loc+9, strcspn(loc+9, ","));
+   goto next;
+   }
+   if(!strncmp(loc, "keyfile=", 8)) {
+   *keyfile = strndup(loc+8, strcspn(loc+8, ","));
+   goto next;
+   }
+   if(!strncmp(loc, "cacertfile=", 11)) {
+   *cacertfile = strndup(loc+11, strcspn(loc+11, ","));
+   goto next;
+   }
+   if(!strncmp(loc, "hostname=", 9)) {
+   *tlshostname = strndup(loc+9, strcspn(loc+9, ","));
+   goto next;
+   }
// skip unknown options, with a warning unless they start with 
a '_'
l = strcspn(loc, ",");
if(*loc != '_') {
@@ -545,6 +561,9 @@ void usage(char* errmsg, ...) {
fprintf(stderr, "Or   : nbd-client -c nbd_device\n");
fprintf(stderr, "Or   : nbd-client -h|--help\n");
fprintf(stderr, "Or   : nbd-client -l|--list host\n");
+#ifdef WITH_GNUTLS
+   fprintf(stderr, "All commands that connect to a host also take: 
[-C|-certfile certfile] [-K|-keyfile keyfile] [-A|-cacertfile cacertfile] 
[-H|-hostname hostname]\n");
+#endif
fprintf(stderr, "Default value for blocksize is 1024 (recommended for 
ethernet)\n");
fprintf(stderr, "Allowed values for blocksize are 
512,1024,2048,4096\n"); /* will be checked in kernel :) */
fprintf(stderr, "Note, that kernel 2.4.2 and older ones do not work 
correctly with\n");
@@ -588,6 +607,10 @@ int main(int argc, char *argv[]) {
uint32_t cflags=NBD_FLAG_C_FIXED_NEWSTYLE;
uint32_t opts=0;
sigset_t block, old;
+   char *certfile = NULL;
+   char *keyfile = NULL;
+   char *cacertfile = NULL;
+   char *tlshostname = NULL;
struct sigaction sa;
struct option long_options[] = {
{ "block-size", required_argument, NULL, 'b' },
@@ -603,12 +626,16 @@ int main(int argc, char *argv[]) {
{ "systemd-mark", no_argument, NULL, 'm' },
{ "timeout", required_argument, NULL, 't' },
{ "unix", no_argument, NULL, 'u' },
+   { "certfile", required_argument, NULL, 'C' },
+   { "keyfile", required_argument, NULL, 'K' },
+   { "cacertfile", required_argument, NULL, 'A' },
+   { "hostname", required_argument, NULL, 'H' },
{ 0, 0, 0, 0 }, 
};
 
logging(MY_NAME);
 
-   while((c=getopt_long_only(argc, argv, "-b:c:d:hlnN:pSst:u", 
long_options, NULL))>=0) {
+   while((c=getopt_long_only(argc, argv, "-b:c:d:hlnN:pSst:uC:K:A:H:", 
long_options, NULL))>=0) {
switch(c) {
case 1:
// non-option argument
@@ -693,6 +720,27 @@ int main(int argc, char *argv[]) {
case 'u':
b_unix = 1;
break;
+#ifdef WITH_GNUTLS
+case 'C':
+certfile=strdup(optarg);
+break;
+case 'K':
+keyfile=strdup(optarg);
+break;
+case 'A':
+cacertfile=strdup(optarg);
+break;
+case 'H':
+tlshostname=strdup(optarg);
+break;
+#else
+case 'C':
+case 'K':
+case 'H':
+case 'A':
+   fprintf(stderr, "E: TLS support not compiled in\n");
+exit(EXIT_FAILURE);
+#endif
default:
fprintf(stderr, "E: option eaten by 42 mice\n");
   

[Nbd] [PATCHv3 3/6] Add TLS support to server

[Alex Bligh]

Known problems / potential issues:

* It now passes a pointer to genconf around so handle_starttls can
  get at the certificates. This is a pity.

* It forks() the TLS proxy child using spawn_child. If we use fork()
  we get complaints about unknown children on SIGCHILD. If we use
  this method, each TLS connection potentially counts as 2 rather than one
  connection as far as maxconnections is concerned. In fact I don't
  *think* this happens because the hashtable itself isn't shared
  across fork(). We need to have a deeper think about how best to
  do this.

* Does not currently set minimum TLS version.

* Minimal testing to date.

Signed-off-by: Alex Bligh 
---
 cliserv.h|   1 +
 nbd-server.c | 196 +++
 2 files changed, 170 insertions(+), 27 deletions(-)

diff --git a/cliserv.h b/cliserv.h
index 9c71cb8..2039cf5 100644
--- a/cliserv.h
+++ b/cliserv.h
@@ -95,6 +95,7 @@ void readit(int f, void *buf, size_t len);
 #define NBD_OPT_EXPORT_NAME(1) /** Client wants to select a named 
export (is followed by name of export) */
 #define NBD_OPT_ABORT  (2) /** Client wishes to abort negotiation 
*/
 #define NBD_OPT_LIST   (3) /** Client request list of supported 
exports (not followed by data) */
+#define NBD_OPT_STARTTLS   (5) /** Client wishes to start TLS */
 
 /* Replies the server can send during negotiation */
 #define NBD_REP_ACK(1) /** ACK a request. Data: option number 
to be acked */
diff --git a/nbd-server.c b/nbd-server.c
index 729156b..cb5756d 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -129,6 +129,10 @@
 #include 
 #endif
 
+#ifdef WITH_GNUTLS
+#include "crypto-gnutls.h"
+#endif
+
 /** Default position of the config file */
 #ifndef SYSCONFDIR
 #define SYSCONFDIR "/etc"
@@ -246,6 +250,8 @@ struct generic_conf {
gint threads;   /**< maximum number of parallel threads we want 
to run */
 };
 
+static pid_t spawn_child();
+
 /**
  * Translate a command name into human readable form
  *
@@ -888,6 +894,20 @@ static void sighup_handler(const int s G_GNUC_UNUSED) {
 }
 
 /**
+ * Set a socket to blocking or non-blocking
+ *
+ * @param fd The socket's FD
+ * @param nb non-zero to set to non-blocking, else 0 to set to blocking
+ * @return 0 - OK, -1 failed
+ */
+int set_nonblocking(int fd, int nb) {
+   int sf = fcntl (fd, F_GETFL, 0);
+   if (sf == -1)
+   return -1;
+   return fcntl (fd, F_SETFL, nb ? (sf | O_NONBLOCK) : (sf & ~O_NONBLOCK));
+}
+
+/**
  * Get the file handle and offset, given an export offset.
  *
  * @param client The client we're serving for
@@ -1256,7 +1276,7 @@ static void send_reply(uint32_t opt, int net, uint32_t 
reply_type, size_t datasi
}
 }
 
-static CLIENT* handle_export_name(uint32_t opt, int net, GArray* servers, 
uint32_t cflags) {
+static CLIENT* handle_export_name(uint32_t opt, int net, GArray* servers, 
uint32_t cflags, int tlsfd) {
uint32_t namelen;
char* name;
int i;
@@ -1280,6 +1300,11 @@ static CLIENT* handle_export_name(uint32_t opt, int net, 
GArray* servers, uint32
for(i=0; ilen; i++) {
SERVER* serve = &(g_array_index(servers, SERVER, i));
if(!strcmp(serve->servename, name)) {
+   if ((tlsfd < 0) && (serve->flags & F_TLSONLY || 
glob_flags & F_TLSONLY)) {
+   err("Negotiation failed: Attempt to read from 
TLS only export without TLS");
+   free(name);
+   return NULL;
+   }
CLIENT* client = g_new0(CLIENT, 1);
client->server = serve;
client->exportsize = OFFT_MAX;
@@ -1324,16 +1349,125 @@ static void handle_list(uint32_t opt, int net, GArray* 
servers, uint32_t cflags)
send_reply(opt, net, NBD_REP_ACK, 0, NULL);
 }
 
+static int tlserrout (void *opaque, const char *format, va_list ap) {
+   char buf[1024];
+   if (vsnprintf(buf, 1024, format, ap) < 0)
+   return -1;
+   buf[1023] = 0; // in case it overran the buffer
+   err_nonfatal(buf);
+   return 0;
+}
+
+static void handle_starttls(uint32_t opt, int *net, int *tlsfd, GArray* 
servers, uint32_t cflags,
+   struct generic_conf *genconf) {
+   uint32_t len;
+   int i;
+   char buf[1024];
+   char *ptr = buf + sizeof(len);
+   int plainfd[2]; // [0] is used by the proxy, [1] is used by NBD
+#ifdef WITH_GNUTLS
+   tlssession_t *s = NULL;
+   int ret;
+#endif
+   if (read(*net, , sizeof(len)) < 0)
+   err("Negotiation failed/8: %m");
+   len = ntohl(len);
+   if (len || (*tlsfd >= 0)) {
+   send_reply(opt, *net, NBD_REP_ERR_INVALID, 0, NULL);
+   }
+
+#ifdef WITH_GNUTLS
+   if (socketpair(AF_UNIX, SOCK_STREAM, 0, plainfd) < 0) {
+   

[Nbd] [PATCHv3 0/6] Introduce TLS support on nbdserver & nbdclient

[Alex Bligh]

This is an RFC patch to introduce TLS support on nbdserver & nbdclient.

This is *NOT* production ready by any means, and is submitted for comment.

I have added crypto-gnutls.[ch] from:
github.com/abligh/tlsproxy
which is my attempt at an MIT licenced GnuTLS proxy. The proxy element
is standalone, and is incorporated here. Whilst it's not GPL licensed,
MIT is compatible. Also it uses GNU format indentation (sorry about that).
However, it (together with buffer.[ch]) is almost entirely self-contained.

The same approach is taken for nbdclient. As the proxy runs as an independent
process, nbdclient can launch it, then call the DOIT ioctl() on one end of
the created socketpair(). The proxy process then drops into the background
and closes after either the kernel closes the socket or the other end closes
the socket.

I have tested this to a minimal extent against qemu-img (i.e. qemu
acting as a client). The problem (see nbdgeneral ad nauseam) we have
with NBD_CMD_DISC means that we see false reports of 'magic number mismatch'.
This appears to be because read() returns 0 in negotiate(), and nbdserver
does not check for this. It then reverses (again) the *previous* magic
number, using ntohll(), and this causes the 'magic number mismatch' issue.
This isn't really a problem but causes confusing errors.

I have also tested nbd-client against nbd-server with TLS, and
the test suite tests nbd-tester-client against nbd-server with TLS.

The first two patches are preparatory work, and the third patch actually
adds NBD_OPT_STARTTLS. The comment in that patch shows what is left to
figure out.

Changes from v2:

* Add support for nbd-client

* Wouter's changes to Makefile.am and conditional compilation.

Changes from v1:

* Added support for TLS to nbd-tester-client. Weirdly it passed first
 time.

* In doing so, folded into this series the two patches to
 ndb-tester-client.c into this series. Note the FIXED_NEWSTYLE patch
 earlier missed host/network ordering.

* Per Wouter, use AM_CONDITIONAL rather than #ifdef'ing out files

* Per Wouter, do the test for GnuTLS later.

Alex Bligh (6):
  Add GnuTLS infrastructure
  Add options for TLS support for server
  Add TLS support to server
  Add TLS testing to nbd-tester-client.c
  Add options to nbd-client for TLS support
  Add TLS support to NBD client

 Makefile.am |   5 +
 buffer.c| 225 +++
 buffer.h|  45 +++
 cliserv.h   |   1 +
 configure.ac|  15 +
 crypto-gnutls.c | 610 
 crypto-gnutls.h |  43 +++
 man/nbd-server.5.in.sgml|  65 +
 nbd-client.c| 193 -
 nbd-server.c| 204 --
 nbd.h   |   2 +
 nbdsrv.h|   1 +
 tests/run/Makefile.am   |  14 +-
 tests/run/certs/ca-cert.pem |  20 ++
 tests/run/certs/ca-key.pem  |  32 +++
 tests/run/certs/ca.info |   3 +
 tests/run/certs/client-cert.pem |  23 ++
 tests/run/certs/client-key.pem  |  32 +++
 tests/run/certs/client.info |   8 +
 tests/run/certs/server-cert.pem |  22 ++
 tests/run/certs/server-key.pem  |  32 +++
 tests/run/certs/server.info |   5 +
 tests/run/nbd-tester-client.c   | 169 ++-
 tests/run/simple_test   |  45 +++
 24 files changed, 1778 insertions(+), 36 deletions(-)
 create mode 100644 buffer.c
 create mode 100644 buffer.h
 create mode 100644 crypto-gnutls.c
 create mode 100644 crypto-gnutls.h
 create mode 100644 tests/run/certs/ca-cert.pem
 create mode 100644 tests/run/certs/ca-key.pem
 create mode 100644 tests/run/certs/ca.info
 create mode 100644 tests/run/certs/client-cert.pem
 create mode 100644 tests/run/certs/client-key.pem
 create mode 100644 tests/run/certs/client.info
 create mode 100644 tests/run/certs/server-cert.pem
 create mode 100644 tests/run/certs/server-key.pem
 create mode 100644 tests/run/certs/server.info

-- 
1.9.1


--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

[Nbd] [PATCHv3 2/6] Add options for TLS support for server

[Alex Bligh]

Signed-off-by: Alex Bligh 
---
 man/nbd-server.5.in.sgml | 65 
 nbd-server.c |  8 ++
 nbdsrv.h |  1 +
 3 files changed, 74 insertions(+)

diff --git a/man/nbd-server.5.in.sgml b/man/nbd-server.5.in.sgml
index 41235e0..0249eec 100644
--- a/man/nbd-server.5.in.sgml
+++ b/man/nbd-server.5.in.sgml
@@ -264,6 +264,57 @@ manpage.1: manpage.sgml
  

   
+  
+   keyfile
+   
+ 
+   Optional; string
+ 
+ If this option is set, it should contain a path to
+  a PEM format X.509 private key used for TLS negotiation
+  with the client. This option must be set to enable TLS.
+   
+  
+  
+   certfile
+   
+ 
+   Optional; string
+ 
+ If this option is set, it should contain a path to
+  a PEM format X.509 public certificate used for TLS negotiation
+  with the client. If keyfile is set but
+  certfile is not set, then the server will
+  attempt to read the certificate from the path specified
+  by keyfile.
+   
+  
+  
+   cacertfile
+   
+ 
+   Optional; string
+ 
+ If this option is set, it should contain a path to
+  a PEM format X.509 CA certificate used for validating client
+  certificates supplied by the client. If this option is not
+  set then client certificates will not be checked.
+   
+  
+  
+tlsonly
+   
+ Optional; boolean.
+ When this option is enabled,
+   nbd-server will only serve exports
+   using the TLS extension. If this option is not supplied,
+   TLS is optional, unless tlsonly is set
+in the section corresponding to the specific export.
+In order for TLS to work at all, the keyfile
+option must be specified in the generic section.
+ 
+   
+  
 
   
   
@@ -831,6 +882,20 @@ manpage.1: manpage.sgml
  

   
+  
+tlsonly
+   
+ Optional; boolean.
+ When this option is enabled,
+   nbd-server will only serve the export
+   using the TLS extension. If this option is not supplied,
+   TLS is optional, unless tlsonly is set
+in the generic section. In order for TLS to work at all,
+the keyfile option must be specified in
+the generic section.
+ 
+   
+  
 
 
   
diff --git a/nbd-server.c b/nbd-server.c
index 4edb883..729156b 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -239,6 +239,9 @@ struct generic_conf {
 gchar *modernaddr;  /**< address of the modern socket */
 gchar *modernport;  /**< port of the modern socket*/
 gchar *unixsock;   /**< file name of the unix domain socket */
+   gchar *certfile;/**< certificate file */
+   gchar *keyfile; /**< key file */
+   gchar *cacertfile;  /**< CA certificate file  */
 gint flags; /**< global flags */
gint threads;   /**< maximum number of parallel threads we want 
to run */
 };
@@ -626,6 +629,7 @@ GArray* parse_cfile(gchar* f, struct generic_conf *const 
genconf, bool expect_ge
{ "trim",   FALSE,  PARAM_BOOL, &(s.flags), 
F_TRIM },
{ "listenaddr", FALSE,  PARAM_STRING,   &(s.listenaddr),
0 },
{ "maxconnections", FALSE, PARAM_INT,   &(s.max_connections),   
0 },
+   { "tlsonly",FALSE,  PARAM_BOOL, &(s.flags), 
F_TLSONLY },
};
const int lp_size=sizeof(lp)/sizeof(PARAM);
 struct generic_conf genconftmp;
@@ -639,6 +643,10 @@ GArray* parse_cfile(gchar* f, struct generic_conf *const 
genconf, bool expect_ge
{ "allowlist",  FALSE, PARAM_BOOL,  &(genconftmp.flags),
  F_LIST },
{ "unixsock",   FALSE, PARAM_STRING,&(genconftmp.unixsock), 
  0 },
{ "max_threads", FALSE, PARAM_INT,  &(genconftmp.threads),  
  0 },
+   { "certfile",   FALSE, PARAM_STRING,&(genconftmp.certfile), 
  0 },
+   { "keyfile",FALSE, PARAM_STRING,&(genconftmp.keyfile),  
  0 },
+   { "cacertfile", FALSE, PARAM_STRING,
&(genconftmp.cacertfile), 0 },
+   { "tlsonly",FALSE, PARAM_BOOL,  &(genconftmp.flags),
  F_TLSONLY },
};
PARAM* p=gp;
int p_size=sizeof(gp)/sizeof(PARAM);
diff --git a/nbdsrv.h b/nbdsrv.h
index f3be738..fefd063 100644
--- a/nbdsrv.h
+++ b/nbdsrv.h
@@ -141,6 +141,7 @@ typedef enum {
 #define F_TRIM 2048   /**< Whether server wants TRIM (discard) to be sent 
by the client */
 #define F_FIXED 4096 /**< Client supports fixed 

[Nbd] [PATCHv9] Improve documentation for TLS

[Alex Bligh]

* Call out TLS into a separate section

* Add details of the TLS protocol itself

* Emphasise that actual TLS session initiation (i.e. the TLS handshake) can
  be initiated from either side (as required by the TLS standard I believe
  and as actually works in practice)

* Clarify what is a requirement on servers, and what is a requirement on
  clients, separately, specifying their behaviour in a single place
  in the document.

* Document the three possible modes of operation of a server.

* Add text defining what 'terminate the session' means during
  negotiation, and when it is available.

Signed-off-by: Alex Bligh 
---
 doc/proto.md | 352 +--
 1 file changed, 318 insertions(+), 34 deletions(-)

Changes since v8:

* Reword section on disconnection, as per mail to list save with Eric Blake's
  change of 're' to 'regarding'.

Changes since v7

* I missed committing the changes re consistent use of 'option' rather than 
'command'
 in v7. They are here now.

Changes from v6:

* Introduced language mandating a server to reply with NBD_ERR_INVALID
to NBD_OPT_STARTTLS if TLS is already negotiatied.

* Removed some duplication in SELECTIVETLS over the prohibition on
servers not returning NBD_ERR_TLSREQD to options other than
NBD_OPT_EXPORTNAME, NBD_OPT_INFO and NBD_OPT_GO. The same thing
was said a different way a couple of paragraphs below.

* Consistently refer to 'options' rather than 'commands' in the
negotiation phase.

* Eric Blake's nits

Changes from v5:

* Delete OPTIONALTLS (RIP)

* Add NBD_REP_ERR_POLICY

* s/NBD_ERR_REP/NBD_REP_ERR/ in one place

* Consistently use the phrase 'terminate the session' to mean dropping
the connection, as per Wouter. Note there are other inconsistent
uses of 'dropping the connection', 'disconnecting' etc. elsewhere
which I haven't touched.

* Similarly refer to the connection as a 'session' when it doesn't
explicitly mean the L3 TCP connection (TLS section only).

* Introduce a paragraph under newstyle negotiation emphasising that
terminating the session is legal and sometimes required, and defining
it.

Changes from v4

* Minor grammar nit

Changes from v3:

* Delete confusing text about server omitting entries from NBD_OPT_LIST
if TLS is not negotiated and FORCETLS is used, as that (of course)
requires NBD_REP_ERR_TLS_REQD elsewhere in the text.

* Further nits from Eric Blake

Changes from v2:

* The response to a command is a response, not a NBD_REP_ACK

* Make it clear that the response can be errored

* Nits from Eric Blake

Changes from v1:

* Make a NBD_CMD_CLOSE imply a flush

* Nits from Eric Blake

diff --git a/doc/proto.md b/doc/proto.md
index f117394..255fd11 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -195,6 +195,23 @@ request before sending the next one of the same type. The 
server MAY
 send replies in the order that the requests were received, but is not
 required to.
 
+There is no requirement for the client or server to complete a
+negotiation if it does not wish to do so. Either end may simply
+close the TCP connection (though see below regarding prior use
+of NBD_OPT_ABORT). Under certain circumstances either
+the client or the server may be required by this document to close
+the TCP connection. In each case, this is referred to as 'terminate
+the session'.
+
+If the client wishes to terminate the session in the negotiation
+phase, and is not doing so because it is required to do so
+by this document, it SHOULD send NBD_OPT_ABORT first if the protocol
+permits. There are instances where this is impossible, such as after
+an NBD_OPT_EXPORTNAME has been issued, or on an unsuccessful
+negotiation of TLS.  For instance, if the client does not find an
+export it is looking for, it may simply send an NBD_OPT_ABORT
+and close the TCP connection.
+
 ### Transmission
 
 There are three message types in the transmission phase: the request,
@@ -286,6 +303,287 @@ S: (*length* bytes of data if the request is of type 
`NBD_CMD_READ`)
 This reply type MUST NOT be used except as documented by the
 experimental `STRUCTURED_REPLY` extension; see below.
 
+## TLS support
+
+The NBD protocol supports Transport Layer Security (TLS) (see
+[RFC5246](https://tools.ietf.org/html/rfc5246)
+as updated by
+[RFC6176](https://tools.ietf.org/html/rfc6176)
+).
+
+TLS is negotiated with the `NBD_OPT_STARTTLS`
+option. This is performed as an in-session upgrade. Below the term
+'negotiation' is used to refer to the sending and receiving of
+NBD options and option replies, and the term 'initiation' of TLS
+is used to refer to the actual upgrade to TLS.
+
+### Certificates, authentication and authorisation
+
+This standard does not specify what encryption, certification
+and signature algorithms are used. This standard does not
+specify authentication and authorisation (for instance
+whether client and/or server certificates are required and
+what they should contain); this is implementation dependent.
+

Re: [Nbd] [PATCHv2 3/6] Add GnuTLS infrastructure

[Wouter Verhelst]

Alex,

On Mon, Apr 11, 2016 at 06:15:36PM +0100, Alex Bligh wrote:
> diff --git a/Makefile.am b/Makefile.am
> index 304db6d..554860e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1,3 +1,10 @@
> +if GNUTLS
> +TLSSRC = crypto-gnutls.c crypto-gnutls.h buffer.c buffer.h
> +TLSEXTRA =
> +else
> +TLSSRC =
> +TLSEXTRA = crypto-gnutls.c crypto-gnutls.h buffer.c buffer.h
> +endif
>  SUBDIRS = . man doc tests gznbd
>  bin_PROGRAMS = nbd-server nbd-trdump
>  sbin_PROGRAMS = @NBD_CLIENT_NAME@
> @@ -5,8 +12,8 @@ EXTRA_PROGRAMS = nbd-client make-integrityhuge
>  noinst_LTLIBRARIES = libnbdsrv.la libcliserv.la
>  libcliserv_la_SOURCES = cliserv.h cliserv.c
>  libcliserv_la_CFLAGS = @CFLAGS@
> -nbd_client_SOURCES = nbd-client.c cliserv.h
> -nbd_server_SOURCES = nbd-server.c cliserv.h lfs.h nbd.h nbdsrv.h backend.h
> +nbd_client_SOURCES = nbd-client.c cliserv.h $(TLSSRC)
> +nbd_server_SOURCES = nbd-server.c cliserv.h lfs.h nbd.h nbdsrv.h backend.h 
> $(TLSSRC)
>  nbd_trdump_SOURCES = nbd-trdump.c cliserv.h nbd.h
>  nbd_server_CFLAGS = @CFLAGS@ @GLIB_CFLAGS@
>  nbd_trdump_CFLAGS = @CFLAGS@ @GLIB_CFLAGS@
> @@ -16,4 +23,4 @@ nbd_client_LDADD = libcliserv.la
>  nbd_server_LDADD = @GLIB_LIBS@ libnbdsrv.la libcliserv.la
>  nbd_trdump_LDADD = libcliserv.la
>  make_integrityhuge_SOURCES = make-integrityhuge.c cliserv.h nbd.h nbd-debug.h
> -EXTRA_DIST = maketr CodingStyle autogen.sh README.md
> +EXTRA_DIST = maketr CodingStyle autogen.sh README.md $(TLSEXTRA)

This can be done more easily like so:


nbd_client_SOURCES = nbd-client.c cliserv.h
nbd_server_SOURCES = nbd-server.c cliserv.h lfs.h nbd.h nbdsrv.h backend.h
TLSSRC = crypto-gnutls.c crypto-gnutls.h
if GNUTLS
nbd_client_SOURCES += $(TLSSRC)
nbd_server_SOURCES += $(TLSSRC)
endif

Automake is smart enough about conditionals to realize that they might
add some extra files which need to be distributed, and so fiddling with
EXTRA_DIST isn't required (as per
https://www.gnu.org/software/automake/manual/html_node/Conditional-Sources.html#Conditional-Compilation-using-Automake-Conditionals)

[...]
-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
   people in the world who think they really understand all of its rules,
   and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] NBD_CMD_DISC

[Alex Bligh]

On 12 Apr 2016, at 11:34, Daniel P. Berrange  wrote:

> On Tue, Apr 12, 2016 at 10:48:20AM +0100, Daniel P. Berrange wrote:
>> On Sun, Apr 10, 2016 at 10:49:00AM +0100, Alex Bligh wrote:
>>> (Daniel: if you want to replicate the issue, just run qemu-img info
>>> against my gonbdserver with TLS. Every fifth NBD_CMD_DISC doesn't
>>> get through before the TCP session closes).
>> 
>> Hmm, I'll have a look at this - I'm not sure its caused by
>> the lack of gnutls_bye, as opposed to incorrect handling
>> of EAGAIN when the block layer sends CMD_DISC
> 
> I have tried to reproduce with your server yet, but I did look at the
> code and identified one potential flaw that might cause the behaviour
> you mention. Can you try testing with the following change applied
> to QEMU:
> 
> diff --git a/nbd/common.c b/nbd/common.c
> index 8ddb2dd..47757b6 100644
> --- a/nbd/common.c
> +++ b/nbd/common.c
> @@ -50,7 +50,7 @@ ssize_t nbd_wr_syncv(QIOChannel *ioc,
>  * qio_channel_yield() that works with AIO contexts
>  * and consider using that in this branch */
> qemu_coroutine_yield();
> -} else if (done) {
> +} else if (done || !do_read) {
> /* XXX this is needed by nbd_reply_ready.  */
> qio_channel_wait(ioc,
>  do_read ? G_IO_IN : G_IO_OUT);
> 
> 
> The current code will cause it to silently not send CMD_DISC if the socket
> returns EAGAIN on send(). This change will cause it to do a poll to wait
> and retry the send(). That said I'd be pretty surprised if we do actually
> get EAGAIN in this scenario, as I'd expect the outgoing TCP buffers to be
> empty at the point in which we close the client connection, but this fix
> is worth a try.

Well, what with pulling to the latest qemu master I now can't reproduce
the original problem against my server :-/ However, I have verified that
against my server it works with your patch (as well as without). I have
verified each time that I am receiving NBD_CMD_DISC.

It's also now working (with and without your patch) against nbd-server.c
(reference implementation) with my GnuTLS patches. This should be exactly
the same as before.

I suspect the issue may be timing related. Unless there's something else
fixed in master, the main change I've made is rebooted my MBP, which
was running very slowly.

I think you should probably still do a gnutls_bye() though.

-- 
Alex Bligh





--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] NBD_CMD_DISC

[Daniel P. Berrange]

On Tue, Apr 12, 2016 at 10:48:20AM +0100, Daniel P. Berrange wrote:
> On Sun, Apr 10, 2016 at 10:49:00AM +0100, Alex Bligh wrote:
> > (Daniel: if you want to replicate the issue, just run qemu-img info
> > against my gonbdserver with TLS. Every fifth NBD_CMD_DISC doesn't
> > get through before the TCP session closes).
> 
> Hmm, I'll have a look at this - I'm not sure its caused by
> the lack of gnutls_bye, as opposed to incorrect handling
> of EAGAIN when the block layer sends CMD_DISC

I have tried to reproduce with your server yet, but I did look at the
code and identified one potential flaw that might cause the behaviour
you mention. Can you try testing with the following change applied
to QEMU:

diff --git a/nbd/common.c b/nbd/common.c
index 8ddb2dd..47757b6 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -50,7 +50,7 @@ ssize_t nbd_wr_syncv(QIOChannel *ioc,
  * qio_channel_yield() that works with AIO contexts
  * and consider using that in this branch */
 qemu_coroutine_yield();
-} else if (done) {
+} else if (done || !do_read) {
 /* XXX this is needed by nbd_reply_ready.  */
 qio_channel_wait(ioc,
  do_read ? G_IO_IN : G_IO_OUT);


The current code will cause it to silently not send CMD_DISC if the socket
returns EAGAIN on send(). This change will cause it to do a poll to wait
and retry the send(). That said I'd be pretty surprised if we do actually
get EAGAIN in this scenario, as I'd expect the outgoing TCP buffers to be
empty at the point in which we close the client connection, but this fix
is worth a try.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] [PATCH v2] doc: In STRUCTURED_REPLY, make error types easy to recognize

[Alex Bligh]

On 11 Apr 2016, at 22:29, Eric Blake  wrote:

> We may add future structured error replies; making it easy
> for older clients to properly treat such new reply types as
> an error gives us a bit more flexibility on introducing new
> errors to existing commands.  Of course, good design practice
> says we should try hard to prevent the server from sending
> something the client isn't expecting, but covering the
> situation from both sides never hurts.  Also, marking error
> types resembles how NBD_REP_ERR_* has a common layout.
> 
> Signed-off-by: Eric Blake 

Reviewed-by: Alex Bligh 

> ---
> 
> v2: make the message length field mandatory and only 16 bits
> 
> Not done: one comment was that NBD_REPLY_TYPE_ is awkward, but
> a rename to use NBD_REPLY_CHUNK_ or any other name is best done
> separately.
> 
> Requires these patches to be applied first:
> [AB] [PATCHv8] Improve documentation for TLS
> [AB*] [PATCHv6] docs/proto.md: Clarify SHOULD / MUST / MAY etc
> 
> (*Note that this is Alex's v6 posted today based on the TLS
> patch, and not my v6 posted a couple days earlier. Proof that
> we really need to get the conflict magnet out of the way...)
> 
> doc/proto.md | 93 +++-
> 1 file changed, 54 insertions(+), 39 deletions(-)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index deb22a5..06a275d 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -1209,8 +1209,7 @@ error, and alters the reply to the `NBD_CMD_READ` 
> request.
> These values are used in the "type" field of a structured reply.
> Some chunk types can additionally be categorized by role, such as
> *error chunks* or *content chunks*.  Each type determines how to
> -interpret the "length" bytes of payload.  If the client receives
> -an unknown or unexpected type, it MUST close the connection.
> +interpret the "length" bytes of payload.
> 
> - `NBD_REPLY_TYPE_NONE` (0)
> 
> @@ -1221,42 +1220,7 @@ error, and alters the reply to the `NBD_CMD_READ` 
> request.
>   chunks were sent, then this type implies that the overall client
>   request is successful.  Valid as a reply to any request.
> 
> -- `NBD_REPLY_TYPE_ERROR` (1)
> -
> -  This chunk type is in the error chunk category.  *length* MUST
> -  be at least 4.  This chunk represents that an error occurred,
> -  and the client MAY NOT make any assumptions about partial
> -  success. This type SHOULD NOT be used more than once in a
> -  structured reply.  Valid as a reply to any request.
> -
> -  The payload is structured as:
> -
> -  32 bits: error (MUST be nonzero)  
> -  *length - 4* bytes: optional string suitable for
> - direct display to a human being
> -
> -- `NBD_REPLY_TYPE_ERROR_OFFSET` (2)
> -
> -  This chunk type is in the error chunk category.  *length* MUST
> -  be at least 12.  This reply represents that an error occurred at
> -  a given offset, which MUST lie within the original offset and
> -  length of the request; the client can use this offset to
> -  determine if request had any partial success.  This chunk type
> -  MAY appear multiple times in a structured reply, although the
> -  same offset SHOULD NOT be repeated.  Likewise, if content chunks
> -  were sent earlier in the structured reply, the server SHOULD NOT
> -  send multiple distinct offsets that lie within the bounds of a
> -  single content chunk.  Valid as a reply to `NBD_CMD_READ`,
> -  `NBD_CMD_WRITE`, `NBD_CMD_WRITE_ZEROES`, and `NBD_CMD_TRIM`.
> -
> -  The payload is structured as:
> -
> -  32 bits: error (MUST be non-zero)  
> -  64 bits: offset (unsigned)  
> -  *length - 12* bytes: optional string suitable for
> - direct display to a human being
> -
> -- `NBD_REPLY_TYPE_OFFSET_DATA` (3)
> +- `NBD_REPLY_TYPE_OFFSET_DATA` (1)
> 
>   This chunk type is in the content chunk category.  *length* MUST
>   be at least 9.  It represents the contents of *length - 8* bytes
> @@ -1272,7 +1236,7 @@ error, and alters the reply to the `NBD_CMD_READ` 
> request.
>   64 bits: offset (unsigned)  
>   *length - 8* bytes: data  
> 
> -- `NBD_REPLY_TYPE_OFFSET_HOLE` (4)
> +- `NBD_REPLY_TYPE_OFFSET_HOLE` (2)
> 
>   This chunk type is in the content chunk category.  *length* MUST
>   be exactly 12.  It represents that the contents of *hole size*
> @@ -1289,6 +1253,57 @@ error, and alters the reply to the `NBD_CMD_READ` 
> request.
>   64 bits: offset (unsigned)  
>   32 bits: hole size (unsigned, MUST be nonzero)  
> 
> +All error chunk types have bit 15 set, and begin with the same
> +*error*, *message length*, and optional *message* fields as
> +`NBD_REPLY_TYPE_ERROR`.  If non-zero, *message length* indicates
> +that an optional error string message appears next, suitable for
> +display to a human user.  

Re: [Nbd] [PATCHv8] Improve documentation for TLS

[Alex Bligh]

Wouter,

On 12 Apr 2016, at 10:20, Wouter Verhelst  wrote:

> To summarize, there are three ways for the connection to end:
> 
> - The client wishes to end the session, and sends the appropriate
>  termination message (OPT_ABORT or CMD_DISC). This is a normal
>  disconnect.
> - Either peer violates a MUST in the spec, and the other side doesn't
>  know how to handle the resulting inconsistency. The only proper
>  solution at that point is to drop the connection, but that's only
>  because there's really nothing else we *can* do. This is an abnormal
>  disconnect.
> - The server wishes to terminate the session. There isn't actually a
>  message for this, so it also results in an abnormal disconnect.

The last case includes (e.g.) 'NBD_OPT_EXPORT_NAME' issued to
a non-existing mount)

> Perhaps we could state that the server can send a message (offset 0,
> length 0, handle 0, error EINTR) when it wants to terminate the session
> for whatever reason (e.g., because it's being restarted).

I think that will make clients' life harder. Most clients don't
expect messages from the server which aren't replies, so I can see
them treating it as a reply to the next message they issue, or
getting into some horrible blocking situation.

(Also please don't use EINTR - that implies you can retry. ETERM?)

> Originally, there were a number of termination points where we could
> drop the connection without further explanation. It was a mess, because
> it resulted in confusing messages (e.g., the server would produce error
> messages in system logs for every disconnect because it couldn't
> distinguish between clean disconnects and unclean disconnects).
> 
> I don't want to go there again.

I think what we should probably say is this (wording needs
tweaking I know):

* One side MAY drop the connection if the other end violates a
  MUST condition.
* The server MUST drop the connection in the 'no way out' situations
  during the negotiation phase (error on NBD_OPT_EXPORT_NAME, error
  in negotiating text).
* As protocol authors we should minimise the number of 'no way out'
  situations.
* The server SHOULD NOT otherwise drop the connection. It can wait
  and error the next command. Clearly there are situations where
  this is going to happen (e.g. server shutdown).
* If the server does need to drop the connection, it SHOULD wait
  until there are no commands in-flight in transmission mode,
  it possible.
* If he client is going to drop the the connection, then other
  than in the event of a protocol violation or a 'no way out'
  situation (e.g. TLS negotiation fails), it MUST use NBD_CMD_DISC
  or NBD_OPT_ABORT
* We should tidy up the semantics and descriptions of NBD_CMD_DISC
  and NBD_OPT_ABORT, viz replies or not to the latter, shutting
  down TLS properly etc.


-- 
Alex Bligh





--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

[Nbd] Get protection for your car at a price you can afford.

[ICICI Lombard Car Insurance]

 


 


3350+

Cashless

Network Garages*

Roadside Assistance

Add On Cover**

Zero Depreciation

& Garage

Cash Cover***

Transfer complete

No Claim

Bonus^

 

 

 

 

 


#Above mentioned discounts has been calculated on the basis of the
rates prescribed under erstwhile Indian Motor Tariff.

*As on 1st December 2015

**Road Side Assistance is add-on cover with additional premium. Terms
and Conditions for Road Side
Assistance


***Zero Depreciation Cover is add-on cover with additional premium for
more details on terms, conditions and exclusions please refer to the
Branch Executive or call at our toll free number 1800 2666. Garage
cash coveris add-on cover with additional premium For Terms and
Condition


^NCB (No Claims Bonus) will only be allowed provided the policy is
renewed within 90 days of the expiry date of the previous policy. The
NCB will be available; provided you show
evidence that you are entitled to NCB from your previous motor
insurance company. Evidence can be in form of a written declaration or
renewal notice or a letter confirming the NCB entitlement from the
previous insurer.

The advertisement contains only an indication of the covered offered.
For more details on risk factors, terms, conditions and exclusions,
please read the sales brochure carefully before concluding a sale.
ICICI Lombard General Insurance Company Limited, ICICI Lombard House,
414, Veer Savarkar Marg, Prabhadevi, Mumbai - 400025. IRDA Reg. No.
115. Toll Free 1800 2666. Fax no
- 022 61961323.

Comprehensive Insurance ( Private Car) "B" Policy (Motor 01), UIN
2920, CIN (U67200MH2000PLC129408).

customersupp...@icicilombard.com

 www.icicilombard.com 
 Unsubscribe 

--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] [PATCHv8] Improve documentation for TLS

[Wouter Verhelst]

On Tue, Apr 12, 2016 at 08:47:49AM +0100, Alex Bligh wrote:
> 
> On 12 Apr 2016, at 07:01, Wouter Verhelst  wrote:
> 
> > hat doesn't mean OPT_ABORT not having a reply is necessarily a good
> > idea. Since it's only used by reference nbd-client in just one use case
> > at this point, I don't think it's particularly bad to change the
> > definition to say that the server SHOULD send a reply (NBD_REP_ACK),
> > upon which the server drops the connection.
> > 
> > The client should probably wait for that too, and not close its socket
> > until either it gets a zero read (indicating that the server closed it
> > already) or it gets an NBD_REP_ACK from the NBD_OPT_ABORT message.
> 
> Yeah. That way would be a safe change (as the worst that can
> happen is the client thinks the server has rudely dropped
> the connection).

Right.

To summarize, there are three ways for the connection to end:

- The client wishes to end the session, and sends the appropriate
  termination message (OPT_ABORT or CMD_DISC). This is a normal
  disconnect.
- Either peer violates a MUST in the spec, and the other side doesn't
  know how to handle the resulting inconsistency. The only proper
  solution at that point is to drop the connection, but that's only
  because there's really nothing else we *can* do. This is an abnormal
  disconnect.
- The server wishes to terminate the session. There isn't actually a
  message for this, so it also results in an abnormal disconnect.

Perhaps we could state that the server can send a message (offset 0,
length 0, handle 0, error EINTR) when it wants to terminate the session
for whatever reason (e.g., because it's being restarted).

Originally, there were a number of termination points where we could
drop the connection without further explanation. It was a mess, because
it resulted in confusing messages (e.g., the server would produce error
messages in system logs for every disconnect because it couldn't
distinguish between clean disconnects and unclean disconnects).

I don't want to go there again.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
   people in the world who think they really understand all of its rules,
   and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] [PATCH/RFC 0/3] Introduce TLS on nbdserver

[Wouter Verhelst]

On Mon, Apr 11, 2016 at 11:47:00PM +0100, Alex Bligh wrote:
> Wouter,
> 
> >> just found ANOTHER reason for that - see below for the bad news.
> >> 
> >> I think I'd quite like to get this in as is and get the refactoring
> >> done later (given SSL is in the standard now :-) )
> > 
> > Sure. Like I said, I'm not opposed to what you're suggesting. It's just
> > that it adds more work for me later (experience tells me that
> > "refactoring" is mostly my job), and I'd like to avoid that if we can.
> 
> :-)
> 
> >> I think I understood about 70% of that.
> > 
> > Can you be a bit more specific than that? Which parts aren't entirely
> > clear?
> 
> Let me look at that again in the cold light of day tomorrow. I suspect
> (given your comment below) I may have missed bits.
> 
> >> 1. The individual functions in nbdserver.c do their own bitbanging.
> >>   (i.e. sending things to the wire).
> > 
> > Yes, that's what my brain dump above is trying to fix, too.
> 
> OK
> 
> >>   This is partly a result of
> >>   history where there was not much commonality and structure between
> >>   nbd options. This isn't entirely true (see e.g. send_reply()).
> >>   Another approach would be to have the entire option consumed
> >>   or not in one place, and the entire reply sent or not in one place.
> > 
> > While that would be an improvement, it will retain the status quo where
> > function X has a hard coded call to function Y at the end. I'd like to
> > get rid of that.
> > 
> > My brain dump of above, if implemented, would result in something along
> > the following lines (pseudo code):
> > 
> > int copyonwrite_write(CLIENT* client, METHODS* next, int active_fd, off_t 
> > offset, size_t len, char* databuf) {
> > assert(next->next != NULL);
> > next = next->next;
> > if(offset is in copyonwrite map) {
> > active_fd = copyonwrite_difffile;
> > }
> > return next->write(client, next, active_fd, offset, min(len, length of 
> > current extent in copyonwrite map), databuf);
> > }
> 
> Ah, so something like a layered set of backend drivers?

That's the idea, yeah.

> This is pretty much what qemu's block layer does, except without the
> scary coroutines.

Heh.

> I was actually talking about bitbanging the network protocol side.

I know.

It's just that this approach would work equally well for the network
side of things, but in that case would be simpler (because it needs less
handler functions), and would thus be a good testbed, so it's what I had
been working on for implementing TLS.

[...]
> >>> I now notice that PKG_PROG_PKG_CONFIG is in the wrong place (it needs
> >>> to be before all those AC_CHECK_* calls), but other than that...
> >> 
> >>  oh, it's already there?
> > 
> > No, my (local, not pushed anywhere) starttls branch from which this
> > patch was pulled puts it in the wrong location.
> 
> I mean "the dependency on PKG_CONFIG is already there?"

Yes, I know that's what you meant. No, it isn't. The patch I sent added
the PKG_PROG_PKG_CONFIG macro in the wrong place (and that's what the
above refers to), but there isn't pkg-config in any released version of
nbd (or of nbd master, for that matter)

> >>> The requirement to have gnutls >= 3.3.0 (released in 2014, so old enough
> >>> that most distributions should have it by now) allows to skip
> >>> gnutls_global_init() at startup -- always nice to not to have to track
> >>> that.
> >> 
> >> Nope. I'm running Ubuntu 14.04 (which is a reasonable OS I think) and
> >> you *definitely* need gnutls_global_init() there. There's no harm in
> >> callign it.
> > 
> > 14.04 isn't going to get a new nbd-server with TLS support. I don't
> > think supporting ages-old versions of GnuTLS because "people might want
> > to compile it on ubuntu 4.04" is a particularly bright idea.
> > 
> > Then again, if you're writing the support, who am I to complain?
> 
> No sure, but's it's a reasonable target to compile on. 16.04
> is either not out yet or only just out,

Not for another week or so :-)

https://wiki.ubuntu.com/XenialXerus/ReleaseSchedule

But my point is that I think it's fine if I depend on "whatever's in
most current development releases of distributions". People use
nbd-server *mostly* through their distribution, not by downloading it
from sourceforge and compiling themselves. If they're doing that anyway,
they can compile dependencies, too.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
   people in the world who think they really understand all of its rules,
   and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!

Re: [Nbd] [PATCHv8] Improve documentation for TLS

[Alex Bligh]

On 12 Apr 2016, at 07:01, Wouter Verhelst  wrote:

> hat doesn't mean OPT_ABORT not having a reply is necessarily a good
> idea. Since it's only used by reference nbd-client in just one use case
> at this point, I don't think it's particularly bad to change the
> definition to say that the server SHOULD send a reply (NBD_REP_ACK),
> upon which the server drops the connection.
> 
> The client should probably wait for that too, and not close its socket
> until either it gets a zero read (indicating that the server closed it
> already) or it gets an NBD_REP_ACK from the NBD_OPT_ABORT message.

Yeah. That way would be a safe change (as the worst that can
happen is the client thinks the server has rudely dropped
the connection).

-- 
Alex Bligh





--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Re: [Nbd] [PATCHv8] Improve documentation for TLS

[Wouter Verhelst]

On Mon, Apr 11, 2016 at 09:34:44PM +0100, Alex Bligh wrote:
> Eric,
> 
> On 11 Apr 2016, at 21:14, Eric Blake  wrote:
> > Current qemu NBD server implementation does NOT send a reply to
> > NBD_OPT_ABORT, but immediately closes the connection. I don't know if
> > that is a bug in qemu (especially given the discussion on NBD_CMD_DISC),
> > but it is an independent issue from TLS documentation, so may be better
> > discussed on that thread.
> 
> Ha, neither does mine, despite my reading of the protocol being
> that it should.
> 
> Reference nbd-server.c doesn't either.

Indeed. That may have been a bad idea.

> > Likewise, current qemu NBD client implementation does NOT send
> > NBD_OPT_ABORT at all, so it's hard to say whether waiting around for a
> > reply is worthwhile.
> 
> :-)
> 
> nbd-client.c only appears to send it after asking for a list,
> and not in any error conditions.

Well, it was added together with NBD_OPT_LIST, and with fixed newstyle (it was
my test case for implementing fixed newstyle ;-)

> >> Obviously NBD_OPT_ABORT and aborting the connection needs
> >> more clearing up, but I'm loathe to do it in the TLS patch.
> >> 
> >> In order not to make things worse, how about:
> >> 
> >>> There is no requirement for the client or server to complete a
> >>> negotiation if it does not wish to do so. Either end may simply
> >>> close the TCP connection (though see below re prior use
> > 
> > Not sure if the use of "re" is ideal (are you abbreviating for "regarding")?
> 
> OK will fix that if Wouter likes the words.
> 
> >>> of NBD_OPT_ABORT). Under certain circumstances either
> >>> the client or the server may be required by this document to close
> >>> the TCP connection. In each case, this is referred to as 'terminate
> >>> the session'.
> >>> 
> >>> If the client wishes to terminate the session in the negotiation
> >>> phase, and is not doing so because it is required to do so
> >>> by this document, it SHOULD send NBD_OPT_ABORT first if the protocol
> >>> permits. There are instances where this is impossible, such as after
> >>> an NBD_OPT_EXPORTNAME has been issued, or on an unsuccessful
> >>> negotiation of TLS.  For instance, if the client does not find an
> >>> export it is looking for, it may simply send an NBD_OPT_ABORT
> >>> and close the TCP connection.
> > 
> > Otherwise, this seems reasonable, other than the fact that qemu needs
> > patches to actually start sending NBD_OPT_ABORT where possible.
> 
> I'd suggest waiting for a definitive answer on whether it's meant
> to have a reply.

Clearly from reading the code it wasn't meant to, at the time.

That doesn't mean OPT_ABORT not having a reply is necessarily a good
idea. Since it's only used by reference nbd-client in just one use case
at this point, I don't think it's particularly bad to change the
definition to say that the server SHOULD send a reply (NBD_REP_ACK),
upon which the server drops the connection.

The client should probably wait for that too, and not close its socket
until either it gets a zero read (indicating that the server closed it
already) or it gets an NBD_REP_ACK from the NBD_OPT_ABORT message.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
   people in the world who think they really understand all of its rules,
   and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

--
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
___
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general